-
Notifications
You must be signed in to change notification settings - Fork 750
chore!: Phase out support for H2 version 1.x #2573
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
| is H2Dialect -> { | ||
| when (dialect.isSecondVersion) { | ||
| false -> append("BITAND(", expr1, ", ", expr2, ")") | ||
| true -> { | ||
| +"BITAND(" | ||
| castToExpressionTypeForH2BitWiseIps(expr1, this) | ||
| +", " | ||
| castToExpressionTypeForH2BitWiseIps(expr2, this) | ||
| +")" | ||
| } | ||
| } | ||
| +"BITAND(" | ||
| castToExpressionTypeForH2BitWiseIps(expr1, this) | ||
| +", " | ||
| castToExpressionTypeForH2BitWiseIps(expr2, this) | ||
| +")" |
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.
isSecondVersion() either always returns true or throws an exception now.
More details in H2.kt
| val isSecondVersion get() = majorVersion == H2MajorVersion.Two | ||
| // Adding a note to discuss keeping, as this will only ever be true now (otherwise throws) | ||
| val isSecondVersion: Boolean get() = majorVersion == H2MajorVersion.Two |
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.
Based on the 2 changes above, this will only ever have a value of true, or it will throw an exception.
That's why all usages have been changed in the source code.
I considered also deprecating this function, but I could understand leaving it in. Maybe H2 will eventually reach V3 and there will be a case for needing this distinction. But we could then add a more pertinent isThirdVersion, so I'm willing to keep or leave.
Let me know if anyone has strong opinions about this.
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.
I will update breaking changes accordingly once the deprecation details, etc are agreed on.
- Remove dependencies for H2V1 - Remove TestDB.H2_V1 from exposed-tests - Deprecate H2MajorVersion.One - Replace usage of H2Dialect.isSecondVersion and will always be true now
- Fix incorrectly excluded tests
- Add breaking changes note - Fix broken rebased test
5e0573b to
26a41dc
Compare
Description
Summary of the change: Drop support for H2 V1 in tests and where applicable in source-code
Detailed description:
Why: Phased-out support for H2 version 1 has been documented since Exposed's initial release 0.42.0. Most new features implemented since then have not included support for or testing on this version. As a technical and db-support breaking change, 1.0.0 is a good candidate to officially drop support. Full details in EXPOSED-30.
How:
h2_v1andTestDB.H2_V1H2MajorVersion.OneinH2DialectH2Dialect.majorVersionso that it only returnsTwo; any other detected version throwsH2Dialect.isSecondVersion()Type of Change
Please mark the relevant options with an "X":
Updates/remove existing public API methods:
Affected databases:
Checklist
Related Issues
EXPOSED-30