-
Notifications
You must be signed in to change notification settings - Fork 369
Introduced new JDBC dialect counterparts #2036
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
Conversation
5ffed08 to
79b3900
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, here I am not sure what is the best solution. Let me clarify. The JdbcDialectProvider interface is defined like this:
public interface JdbcDialectProvider {
Optional<Dialect> getDialect(JdbcOperations operations);
}
The common sense dictates, that since it is a DialectProvider for JDBC, then the returned dialect is supposed to be an instance of JdbcDialect. However, such type migration change would not be backward compatible.
The solution I can think of is to deprecate this method in the interface, and maybe even make JdbcDialectProvider a top-level interface, instead of being an inner one. And in this case, we can direct users to a new interface to be used, where getDialect method returns the Optional<JdbcDialect>
If we just deprecate the JdbcDialectProvider#getDialect method, we will not be able to say at the moment what is the replacement.
Maybe I'm just overcomplicating things... I need your option, Mark @mp911de
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would split the change into two parts. Updating JdbcDialectProvider to return JdbcDialect in the signature would be a change shipped with the next major release.
For the 3.5 release, it should be fine to have an adapter that translates a generic Dialect into JdbcDialect and from DialectResolver.getDialect(...) we can directly return JdbcDialect.
Signed-off-by: mipo256 <mikhailpolivakha@gmail.com>
79b3900 to
413de0a
Compare
Deprecate original DialectResolver and JdbcArrayColumns as they've been in the wrong package and introduce replacements in the dialect package. Let deprecated types extend from their replacements to retain compatibility. Make instance holders final, fix Javadoc typos, update reference docs.
@schauder can you have a look after a round of polishing? Going forward, we would drop the deprecated elements in 4.0 and update the configuration so that we exclusively require JdbcDialect.
Sure, I would take a look into your changes @mp911de, I'll reply shortly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can add a javadoc @deprecated tag here? Just in case to specify the replacement like in other parts of the old DialectResolver...
@mp911de I think it is great overall. I only have the minor comment above.
As a side note, I can provide a PR for 4.x milestone to got rid of those classes once this is merged.
Deprecate original DialectResolver and JdbcArrayColumns as they've been in the wrong package and introduce replacements in the dialect package. Let deprecated types extend from their replacements to retain compatibility. Make instance holders final, fix Javadoc typos, update reference docs. Original pull request #2036 See #2031
Thanks everyone. That's merged.
Deprecate original DialectResolver and JdbcArrayColumns as they've been in the wrong package and introduce replacements in the dialect package. Let deprecated types extend from their replacements to retain compatibility. Make instance holders final, fix Javadoc typos, update reference docs. Original pull request #2036 See #2031
Uh oh!
There was an error while loading. Please reload this page.
Closes #2031
The idea is to introduce for each relational
Dialectits jdbc counterpart. All the Jdbc dialects that were already in place (there were a couple of those) and added ones now extend theJdbcDialectand their corresponding relational counterpart.CC: @mp911de @schauder