-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: Optimize CASE expression for usage where then and else values are literals #11553
Conversation
.unwrap_or_else(|_| Arc::clone(e)); | ||
let else_ = Scalar::new(expr.evaluate(batch)?.into_array(1)?); | ||
|
||
Ok(ColumnarValue::Array(zip(&when_value, &then_value, &else_)?)) |
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.
if the input is ColumnarValue::Scalar
shouldn't the output also be a ColumnarValue::Scalar
(rather than a ColumnarValue::Array
?)
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.
No, the output will be an array containing values based on two scalar arguments.
SELECT CASE WHEN a > 2 THEN 'even' ELSE 'odd' END FROM foo
----
odd
even
even
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 wonder if it would make sense (in a separate PR) to produce a dictionary array in this case since it will only even contain two distinct values? 🤔
@@ -50,3 +50,19 @@ SELECT CASE WHEN a > 2 THEN b END FROM foo | |||
NULL | |||
4 | |||
6 | |||
|
|||
# scalar or scalar (string) |
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.
Could you also add a test where both arguments are scalars (like CASE WHEN 1 > 2 THEN 'true' ELSE 'false')
?
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.
Added
|
||
# scalar or scalar (string) | ||
query T | ||
SELECT CASE WHEN a > 2 THEN 'even' ELSE 'odd' END FROM foo |
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.
Looks like NULL
handling in this specialized implementation is not tested, we can add a (NULL, NULL) row into foo table
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.
Good point. I have added this.
…re literals (apache#11553) * Optimize CASE expression for usage where then and else values are literals * add slt test * add more test cases
Which issue does this PR close?
N/A
Rationale for this change
What changes are included in this PR?
Add a fast path for a specific usage of CASE expression
Are these changes tested?
Are there any user-facing changes?