Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed InvalidDB error when creating a new connection with PrepareStmt #4214

Closed
wants to merge 1 commit into from

Conversation

jccit
Copy link

@jccit jccit commented Mar 24, 2021

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

I was running into an issue using Goyave with the MySQL connector, it was always returning invalid db upon connection.

Here's a snippet from the Goyave database library

dsn := dialect.buildDSN()
db, err := gorm.Open(dialect.initializer(dsn), &gorm.Config{
    PrepareStmt: true,
    Logger:      logger.Default.LogMode(logLevel),
})
if err != nil {
    panic(err)
}

sql, err := db.DB()
if err != nil {
    panic(err)
}

This bug occurs on the call to db.DB() which should return a *sql.DB object.

The bug is caused by DB() trying to check if ConnPool implements the GetDBConnector interface. Goyave makes a call to gorm.Open and enables PrepareStmt. This set ConnPool to be a PrepareStmtDB. PrepareStmtDB does have a function for returning the repsonse that DB() is expecting, however it's named GetDB instead of GetDBConn so the check fails and the code returns invalid db which causes the Goyave app to panic.

I've added a wrapper around GetDB called GetDBConn so the check passes and returns the expected result. Since I'm not renaming the function, it remains backwards compatible with any code that uses the GetDB method.

User Case Description

Bug fix

@jinzhu jinzhu closed this in a8b7254 Mar 25, 2021
mittwillson pushed a commit to itering/gorm that referenced this pull request Sep 27, 2021
cgxxv pushed a commit to cgxxv/gorm that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant