Skip to content
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(binder): add const case-when evaluation optimization during binding #14965

Merged
merged 15 commits into from
Feb 23, 2024

Conversation

xzhseh
Copy link
Contributor

@xzhseh xzhseh commented Feb 3, 2024

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

As titled, the optimization goes to case-when expression in below format, e.g.,

select case <constant> 
    when <constant> then <expr_1>
    when <constant> then <expr_2>
    ... (potentially lots of other arms with constant condition clause)
    else <expr_fallback> end;

related: #14586 (comment).

And since I'm new to optimizer, feel free to point out any part I may have missed and any suggestion is welcomed. 😄

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added test labels as necessary. See details.
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)
  • My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

@@ -469,6 +469,60 @@ impl Binder {
Ok(func_call.into())
}

/// The optimization check for the following case-when expression pattern
/// e.g., select case 1 when (...) then (...) else (...) end;
fn check_constant_case_when_optimization(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I create another check function is that we can't check all the optimizable case-when patterns within only one pass, and they indeed have subtle but significant difference, which may not be generalized.

Comment on lines 610 to 617
// Here we reuse the `ConstantLookup` as the `FunctionCall`
// to avoid creating a dummy `ConstCaseWhenEval` expression type
// since we do not need to go through backend
return Ok(FunctionCall::new(
ExprType::ConstantLookup,
constant_case_when_eval_inputs,
)?
.into());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just return constant_case_when_eval_inputs[0] here? Then we don't even need to use a rule to convert it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then what type should we return, I doubt FunctionCall would work 🤔, also Literal may not be the appropriate one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method returns an ExprImpl, an enum that could be any expression type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it!
Then I think return a bound expression should work, and I'll remove the relevant rules. 😄

Copy link
Contributor

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xzhseh xzhseh changed the title feat(optimizer): add const case-when evaluation rewrite rule feat(binder): add const case-when evaluation optimization to binder Feb 21, 2024
@xzhseh xzhseh changed the title feat(binder): add const case-when evaluation optimization to binder feat(binder): add const case-when evaluation optimization during binding Feb 21, 2024
@xzhseh xzhseh added this pull request to the merge queue Feb 23, 2024
Merged via the queue into main with commit b0a9000 Feb 23, 2024
27 of 28 checks passed
@xzhseh xzhseh deleted the xzhseh/case-when-optimizer-rewrite-rule branch February 23, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants