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

Error retrieving database id results in invalid bound statement when statements are executed #3040

Closed
p91paul opened this issue Dec 11, 2023 · 6 comments
Assignees
Labels
Milestone

Comments

@p91paul
Copy link

p91paul commented Dec 11, 2023

Here's what happened to me:

  1. During application startup, connecting to database fails
  2. SqlSessionFactoryBean configures mappers and tries to determine the databaseId to parse mappers xml https://github.com/mybatis/spring/blob/d081a644e12eb036652e010037fd1bf84e398d7c/src/main/java/org/mybatis/spring/SqlSessionFactoryBean.java#L664
  3. Finding databaseId fails: in case of failure, VendorDatabaseIdProvider returns null, not a SqlException
  4. VendorDatabaseIdProvider logs the error, then SqlSessionFactoryBean continues and does not bind all statements with a databaseId="xxx" attribute
  5. The application starts up
  6. Database connection is resumed
  7. Several hours, or days, later, the application runs the rarely-used database-specific statement that could not be bound (ok this is oddly specific, but very important: this specific fault happened several times in production, but it was very difficult debugging it because the revealing log from VendorDatabaseIdProvider was much earlier than the runtime bug)
  8. The statement cannot be executed due to org.apache.ibatis.binding.BindingException: Invalid bound statement (not found):

Proposed solutions:

  • When databaseId is set on a statement, resolve the correct statement when the query is executed instead of during application startup, using the currently live connection instead of the possibly different one used during startup. This would defer binding and avoid situations where the application starts up despite the database being down but then is unable to execute queries.
  • If VendorDatabaseIdProvider returns null, fail (or fix VendorDatabaseIdProvider to not return null on exception)
  • Probably the safest and easiest: if all candidates for binding a statement declared in a mapper interface require a specific databaseId, but no databaseId is available, fail immediately instead of silently not binding the statement

Current workaround: instead of the default VendorDatabaseIdProvider, we decided to inject a subclass where we override getDatabaseId; if super.getDatabaseId() returns null, we throw an exception. This way the application fails and our application orchestrator restarts the application (until the database becomes available again).

Steps to reproduce in attached demo project:

  1. Run the application with the spring boot "h2" profile -> "X" is logged by the DemoCommand
  2. Run the application with the spring boot "oracle" profile -> Invalid bound statement is thrown (the jdbc url for oracle is willingly bogus, to simulate a connection failure)

demo-mybatis-invalidbound.zip

@harawata
Copy link
Member

Hello @p91paul ,

Thank you for the report and the demo!

I only took a quick look, but it seems reasonable to throw BuilderException in case SQLException is thrown in VendorDatabaseIdProvider.
That resolves your problem, right?

@p91paul
Copy link
Author

p91paul commented Dec 13, 2023

yes, that would be enough for me. Just to be clear, the main issue is that VendorDatabaseIdProvider currently catches the exception and returns null; it never throws a SqlException.

@harawata harawata transferred this issue from mybatis/spring Dec 14, 2023
harawata added a commit to harawata/mybatis-3 that referenced this issue Dec 14, 2023
As reported in mybatis#3040, swallowing the SQLException could lead to unexpected statement resolution.

Theoretically, this change might expose a problem in an existing solution that was previously hidden.
@hazendaz
Copy link
Member

fixed via #3041

@hazendaz
Copy link
Member

@p91paul Please try our snapshot now and see if this helps. We typically don't release constantly but if it soles the issue, I'm willing to cut another release as that seems how most projects are these days, bug fix, correct, confirm, release.

@harawata harawata added this to the 3.5.16 milestone Dec 16, 2023
@p91paul
Copy link
Author

p91paul commented Dec 16, 2023

Thanks for fixing it so fast!
We extended VendorDatabaseIdProvider in our application and overrode the getDatabaseId method, so there is no need for a strict timeline for us, you can release when you are ready.

@hazendaz
Copy link
Member

ok thanks @p91paul , glad you have a work around. Its likely we release again before year end and we are fighting with mavens 'site' build and even with little changes we likely will wan to try again but thank you for letting us know its not a super rush wince we have already released twice very quickly and I know many that use bots like Renovate/Dependabot are overwhelmed with releases so we try to be mindful of that.

@harawata harawata added the bug label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants