Skip to content

Add property spring.data.jdbc.dialect #39941

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

Closed

Conversation

schauder
Copy link
Contributor

The class valued property allows to configure a dialect to use for Spring Data JDBC, without relying on a database connection to determine it.

This is useful especially for CDS.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 14, 2024
@mhalbritter mhalbritter added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 14, 2024
@mhalbritter mhalbritter added this to the 3.3.x milestone Mar 14, 2024
@mhalbritter mhalbritter self-assigned this Mar 15, 2024
mhalbritter pushed a commit that referenced this pull request Mar 15, 2024
The class valued property allows to configure a dialect, without relying
on a database connection to determine it.

See gh-39941
mhalbritter added a commit that referenced this pull request Mar 15, 2024
@mhalbritter
Copy link
Contributor

Thanks Jens!

@mhalbritter mhalbritter modified the milestones: 3.3.x, 3.3.0-M3 Mar 15, 2024
mhalbritter added a commit that referenced this pull request Mar 15, 2024
This reverts commit ab5dee8, reversing
changes made to 2422307.
@mhalbritter mhalbritter reopened this Mar 15, 2024
@mhalbritter
Copy link
Contributor

Sorry, after some discussion I had to revert the merge.

First, because we don't really like having Class configuration properties, it's not very user-friendly having to configure a fully-qualified class name.

Second, this doesn't work in a native image without reflection hints. Sometimes they're unavoidable, but we'd like to explore if there's a way around that.

@mhalbritter mhalbritter modified the milestones: 3.3.0-M3, 3.3.x Mar 15, 2024
@wilkinsona
Copy link
Member

I wonder if we can do something similar to our spring.jpa.database property that's backed by the org.springframework.orm.jpa.vendor.Database enum? Perhaps a somewhat similar enum, either in Boot or Spring Data Relational, that provides mappings of database name to dialect. This mapping could be similar to what's already done in org.springframework.data.jdbc.repository.config.DialectResolver.DefaultDialectProvider.getDialect(Connection), but instead of using the database product name from the connection as the input, it'd be the value from the new enum.

@schauder
Copy link
Contributor Author

That sounds very reasonable. I'll give that a try.

The class valued property allows to configure a dialect, without relying on a database connection to determine it.
@schauder schauder force-pushed the issue/dialect-property branch from eafe1a5 to fb896a8 Compare March 18, 2024 15:15
@schauder
Copy link
Contributor Author

I used an enum utilising the standard enum conversion and left everything in Boot.

Please have another look.

@wilkinsona
Copy link
Member

Thanks for the update, @schauder. Did you consider adding the enum to Spring Data Relational? To me, that feels like a more natural home for it and it would make it easier to keep it in sync with the dialect implementations.

@schauder
Copy link
Contributor Author

Yes, I did consider moving it to Spring Data JDBC.
While I agree that it might be easier to sync it to changes in Spring Data, I do think it belongs more on the Boot side of things, since Boot is responsible for handling application.properties and similar and the Enum exists only for that purpose.

That said: If you want it out of Boot, I don't have a problem with moving it into Spring Data.

@wilkinsona
Copy link
Member

I don't feel strongly about this, just trying to reduce duplication overall.

I wondered if the enum could also support the mapping that's currently done by DefaultDialectProvider.getDialect through a fromMetadata(DatabaseMetaData) static method. That would leave us with a single place in both codebases that knows about all of the different dialects.

If you think that's likely to be more trouble than it's worth, we can keep the enum in Boot.

@schauder
Copy link
Contributor Author

schauder commented Mar 19, 2024

If you think that's likely to be more trouble than it's worth, we can keep the enum in Boot.

Then let us keep it in Boot.

mhalbritter pushed a commit that referenced this pull request Mar 22, 2024
mhalbritter added a commit that referenced this pull request Mar 22, 2024
@mhalbritter
Copy link
Contributor

Thanks Jens!

@mhalbritter mhalbritter modified the milestones: 3.3.x, 3.3.0-RC1 Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants