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): convert to constant lookup when case when expression is in simple form #15083

Merged
merged 29 commits into from
Mar 1, 2024

Conversation

xzhseh
Copy link
Contributor

@xzhseh xzhseh commented Feb 12, 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, related: #14317, specifically the following pattern.

In addition, if users write a case-when expression that can be converted to the simple form, we can also normalize it in optimizer easily:

SELECT a,
       CASE WHEN a=1 THEN 'one'
            WHEN a=2 THEN 'two'
            ELSE 'other'
       END
    FROM test;

Also, base branch is set to #15077 to avoid any potential merge conflict, should merge after merging base branch.

Planner tests & e2e_test will be added later.

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.

@BugenZhao
Copy link
Member

BugenZhao commented Feb 21, 2024

Can you elaborate on the relationship between this PR and #14965? BTW, would you like to have a try of https://graphite.dev for the stacking workflow? 😄

Copy link
Contributor Author

xzhseh commented Feb 21, 2024

Can you elaborate on the relationship between this PR and #14965?

#14965 aims to optimize the pattern like below, e.g.,

select case 1 when 1 then 114514 (...) else 1919810 end;

to a simple constant (i.e., 114514 in the above example) during frontend binding phase, while this PR is particularly for detecting some pattern that can be transformed to optimizable case-when expression (e.g., constant lookup), an example would be the pattern in the above description.

BTW, would you like to have a try of https://graphite.dev for the stacking workflow? 😄

I heard of graphite before (but never actually try it) and I just realize how powerful it is, will start using it today. 😄
Thanks for the recommendation!

Base automatically changed from xzhseh/const-case-when-planner-test to main February 27, 2024 16:24
@TennyZhuang
Copy link
Contributor

Can you add some planner test? to cover most branches of your function.

@xzhseh xzhseh force-pushed the xzhseh/case-when-convert-to-simple-form branch from 0aa38fd to 2e852c9 Compare February 29, 2024 19:25
@xzhseh xzhseh force-pushed the xzhseh/case-when-convert-to-simple-form branch from 2e852c9 to 88a2aa0 Compare February 29, 2024 19:26
end
from t1;
logical_plan: |-
LogicalProject { exprs: [ConstantLookup(t1.c1, 1:Int32, 'one':Varchar, 2:Int32, 'two':Varchar, 3:Int32, 'three':Varchar, 4:Int32, 'four':Varchar, 5:Int32, 'five':Varchar, 6:Int32, 'six':Varchar, 7:Int32, 'seven':Varchar, 8:Int32, 'eight':Varchar, 9:Int32, 'nine':Varchar, 10:Int32, 'ten':Varchar, 11:Int32, 'eleven':Varchar, 12:Int32, 'twelve':Varchar, 13:Int32, 'thirteen':Varchar, 14:Int32, 'fourteen':Varchar, 15:Int32, 'fifteen':Varchar, 16:Int32, 'sixteen':Varchar, 17:Int32, 'seventeen':Varchar, 18:Int32, 'eighteen':Varchar, 19:Int32, 'nineteen':Varchar, 20:Int32, 'twenty':Varchar, 21:Int32, 'twenty-one':Varchar, 22:Int32, 'twenty-two':Varchar, 23:Int32, 'twenty-three':Varchar, 24:Int32, 'twenty-four':Varchar, 25:Int32, 'twenty-five':Varchar, 26:Int32, 'twenty-six':Varchar, 27:Int32, 'twenty-seven':Varchar, 28:Int32, 'twenty-eight':Varchar, 29:Int32, 'twenty-nine':Varchar, 30:Int32, 'thirty':Varchar, 31:Int32, 'thirty-one':Varchar, 'other':Varchar) as $expr1] }
Copy link
Contributor Author

@xzhseh xzhseh Feb 29, 2024

Choose a reason for hiding this comment

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

Note here, only if the amount of arms exceeds the pre-set limit (i.e., 30), we will try the transformation. In other words, the case-when expression as below will operate normally. (only 2 arms excluding the fallback arm)

select
    case 
        when c1 = 1 then (...)
        when c1 = 2 then (...)
        else (...)
    end
from t1;

Copy link
Contributor

@TennyZhuang TennyZhuang 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 enabled auto-merge March 1, 2024 04:14
@xzhseh xzhseh added this pull request to the merge queue Mar 1, 2024
Merged via the queue into main with commit 7f6ab49 Mar 1, 2024
26 of 27 checks passed
@xzhseh xzhseh deleted the xzhseh/case-when-convert-to-simple-form branch March 1, 2024 04:36
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.

3 participants