-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix: support NULL return values from CASE statements #3531
fix: support NULL return values from CASE statements #3531
Conversation
This commit enhances the processing of `CASE` statements so that both `THEN` and the default return types can be `NULL`. At least one branch must be non-null so that KSQL can determine the result type of the statement. All non-null branches must have the same result schema, i.e. we don't (yet) do any implicit casting of numeric types. The commit also improves some error messages by using KSQL types rather than connect schema types so that error messages use, for example, `BIGINT` rather than `Schema{INT64}`.
*/ | ||
public boolean canUpCast(final SqlBaseType to) { | ||
public boolean canImplicitlyCast(final SqlBaseType to) { |
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.
Renamed to make the intent more clear.
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.
nice
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.
LGTM. Thanks @big-andy-coates !
ksql-execution/src/main/java/io/confluent/ksql/execution/util/ExpressionTypeManager.java
Outdated
Show resolved
Hide resolved
ksql-common/src/main/java/io/confluent/ksql/schema/ksql/SqlBaseType.java
Outdated
Show resolved
Hide resolved
…eType.java Co-Authored-By: Victoria Xia <victoria.f.xia281@gmail.com>
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.
LGTM just some nits
*/ | ||
public boolean canUpCast(final SqlBaseType to) { | ||
public boolean canImplicitlyCast(final SqlBaseType to) { |
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.
nice
Description
Fixes: #3405, #3344
This commit enhances the processing of
CASE
statements so that bothTHEN
and the default return types can beNULL
.At least one branch must be non-null so that KSQL can determine the result type of the statement.
All non-null branches must have the same result schema, i.e. we don't (yet) do any implicit casting of numeric types.
The commit also improves some error messages by using KSQL types rather than connect schema types so that error messages use, for example,
BIGINT
rather thanSchema{INT64}
.Testing done
Usual
Reviewer checklist