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): support Having clause and more flexible OrderBy #651

Merged
merged 2 commits into from
May 27, 2022

Conversation

lokax
Copy link
Member

@lokax lokax commented May 17, 2022

Signed-off-by: lokax [email protected]

Having clause

Support having clause and alias

select count(x) as a, y as b from test group by b having a > 1

select 42 having 42 > 18;

Forbid to generate:

select count(x) from test group by count(x)

Since the above of Agg is not necessarily a Project, a more general way to generate binding is needed.
I also push AggCall Expression into bingding vector in the InputRefResolver

The search process:
Step 1. Find binding for y + 1 expression

select y + 1 from test group by y + 1

Step 2. If not found: Find binding for y expression

select y + 1 from test group by y

Problems with aliases

select x as i from test group by x having i + 1

Because the return type of child needs to be known when the BinaryOp(i + 1) expression is bound.

pub struct BoundAlias {
    pub alias: String,
    pub expr: Box<BoundExpr>, // I add a new field
}

AliasRewriter will rewrite alias expressions when generating the logical plan


More flexible OrderBy

select x from test order by y

If the expression of OrderBy does not exist in the select list, an expression copy will be added to the select list to generate the correct binding.

Change: select list: [column(x)] -> [column(x), column(y)]

If the select list has changed:
Also add an new Project above the OrderBy to produce the correct output: [column(x), column(y)] -> [column(x)]

Now OrderBy can support sort using expressions

select x from test order by -(x + 1)
select y from test group by y order by count(x);

@skyzh skyzh requested review from xxchan and skyzh May 17, 2022 13:59
Copy link
Member

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

LGTM! For unused code, maybe we can simply remove them?

@skyzh skyzh enabled auto-merge (squash) May 27, 2022 13:59
@skyzh skyzh merged commit d09e71c into risinglightdb:main May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants