fix!: keep explicit dialects across withConfig and fail fast on ambiguous dialect resolution - #438
Merged
Merged
Conversation
…uous dialect resolution withConfig re-resolved the dialect from the classpath, silently discarding a dialect set via withDialect. The template now tracks the explicitly set dialect and retains it across withConfig; a classpath-resolved dialect is re-resolved under the new configuration, as dialects capture their configuration at construction. Providers.getSqlDialect(StormConfig) now selects its provider through the same unique-selection path as the connection and transaction providers, so an ambiguous classpath fails with an error naming the candidates instead of picking by classpath order. Ambient dialect resolution is deferred to first use: SqlTemplate.PS/JPA no longer resolve a dialect during class initialization, and database-bound templates never trigger classpath resolution at all. The JPA classpath fallback is deferred the same way so a provider filter can still be applied on an ambiguous classpath.
LazySupplier.get() performed a compare-and-set and a volatile store on every call, including after the value was resolved. Suppliers shared across threads on per-query paths, such as the lazily resolved dialect and template preparation, turned that into contended cache-line writes. A resolved value is now returned after a single volatile read.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #404.
Problem
Two related resolution defects:
SqlTemplateImpl.withConfigre-resolved the dialect from the classpath, soSqlTemplate.PS.withDialect(x).withConfig(c)silently lostx.Providers.getSqlDialect(StormConfig)picked the first enabled dialect provider with no ambiguity guard, unlike the unique selection used for connection and transaction providers, so the winner on a classpath with several dialect modules depended on classpath order.Changes
Providers.getSqlDialect(StormConfig)now routes through the sameselectUniquepath as the connection and transaction providers: an ambiguous resolution throws aPersistenceExceptionnaming the candidates, withwithDialect(...)or database binding as the remedy. Ordered classpaths resolve exactly as before:DefaultSqlDialectProviderImplis@AfterAnyand MariaDB is@Before(MySQL), so the default provider plus a single dialect module still resolves uniquely to the module's dialect.SqlTemplateImpltracks the explicitly set dialect separately from the resolved one.withConfigretains an explicit dialect and re-resolves a classpath-resolved one under the new configuration (dialects capture their configuration at construction). The otherwith*methods pass the explicit dialect through, so derived templates keep their resolution mode.Ambient resolution is lazy.
SqlTemplate.PS/JPAresolved a dialect in their static initializers, and every database-bound template derives from them viawithConfig(...)before applying the dialect resolved for its database. With an eager fail-fast guard, class initialization would have failed on multi-dialect classpaths that are perfectly valid under the per-database resolution rules of #359. The dialect, the dialect-keyed template cache, and the template preparation are now initialized on first use, so the ambiguity error surfaces only when an ambient dialect is actually used, at query time, consistent with #359.JpaTemplateImpldefers its classpath fallback the same way: it previously resolved in the constructor, which would have made thewithProviderFilter(...)remedy unreachable on an ambiguous classpath. The fallback dialect and the derivedSqlTemplateare now lazy; the data-source path still resolves by database product at construction.Tests
SqlDialectProviderResolutionTest(new): unordered peers fail fast naming both candidates; single and ordered provider sets resolve; ambient resolution is lazy (an explicit dialect works on an ambiguous classpath whiledialect()on an ambient template fails fast). Uses a context class loader that substitutes the dialect service registrations.SqlTemplateImplTest: explicit dialect surviveswithConfigand resolver customization; classpath-resolved dialects are re-resolved under the new configuration, asserted in both ANSI escaping directions since the surefire run sets-Dstorm.ansi_escaping=true.Full reactor green (30 modules, zero failures).