-
Notifications
You must be signed in to change notification settings - Fork 751
fix!: EXPOSED-825 Case().When(...).Else(...) with QueryParameter(...,… #2565
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
1981119 to
e36db22
Compare
e5l
left a comment
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.
please check if the breaking change document should be adjusted as well
e36db22 to
df29b8f
Compare
bog-walk
left a comment
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.
Please check comments, particularly about When(literal, literal) and breaking changes.
exposed-core/src/main/kotlin/org/jetbrains/exposed/v1/core/Function.kt
Outdated
Show resolved
Hide resolved
| /** | ||
| * Adds a WHEN clause that compares the case value against a literal condition with a literal result. | ||
| * | ||
| * @param R The return type of the result value | ||
| * @param cond The literal value to compare against | ||
| * @param result The literal value to return if the condition matches | ||
| * @param resultType Optional column type for the result value | ||
| * @return A ValueCaseWhen instance for method chaining | ||
| */ | ||
| fun <R> When(cond: T, result: R, resultType: IColumnType<R & Any>? = null): ValueCaseWhen<T, R> { | ||
| return ValueCaseWhen<T, R>(value).When(cond, result, resultType) | ||
| } |
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.
This is the only new method that gives me pause, as I thought making resultType optional would always throw at runtime. But now I assume it must be for chained WHEN clauses, but it seems to be order-based?
Is the following expected?
// successful
case(tester.key)
.When(0, 100, IntegerColumnType())
.When(1, 200)
.Else(10)
.let { expr ->
tester.select(expr).where { tester.key eq 0 }.first()
}
// fails with -> No column type has been found
case(tester.key)
.When(0, 100)
.When(1, 200, IntegerColumnType())
.Else(10)
.let { expr ->
tester.select(expr).where { tester.key eq 0 }.first()
}Seems a little odd.
But if it is expected (or unavoidable), can it at least be mentioned in the KDocs? That the first chained use of this method must always have a provided type?
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 updated the KDoc to mention that column type should be provided in the beginning of the chain.
But I also imrpoved it a bit with resolveColumnType(), so primitives could be used in When without explicit providing of column type.
...tests/src/test/kotlin/org/jetbrains/exposed/v1/r2dbc/sql/tests/shared/dml/ConditionsTests.kt
Show resolved
Hide resolved
… EnumerationColumnType) throws “No column type has been found”
fa184ce to
91ad46c
Compare
91ad46c to
d6df146
Compare
We've got a list of example where
case()could be working, but it's not:With this PR
Casewas spit toCaseandValueCaseclasses. With this change it's getting possible to allow pass kotlin values instead of expressions intoWhen/Elseparts of thecase()statement.Also new variant allows to create statement without
Else()part, what was not possible before.Type of Change
Please mark the relevant options with an "X":
Updates/remove existing public API methods:
Affected databases:
Related Issues
EXPOSED-825 Case().When(...).Else(...) with QueryParameter(..., EnumerationColumnType) throws “No column type has been found”