Feature: Support DISTINCT in new planner #5410
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
mergify
bot
added
the
pr-feature
query/src/sql/exec/mod.rs
Outdated
| @@ -362,6 +363,12 @@ impl PipelineBuilder { | |||
| })?; | |||
| } | |||
|
|
|||
| // Since transform has added or did, making group expressions as column expr is safe. | |||
| group_expressions = group_expressions | |||
There was a problem hiding this comment.
The group expression may have been a column, so there may be duplicate conversions here.
There was a problem hiding this comment.
Yes, if a group expression is a column, it is unnecessary here.
The desitnation of adding this code is to make distinct with group-by work.
Two ways to make it better:
- We do this only when it is necessary.
- Move this when bind distinct.
|
@mergify update |
✅ Branch has been successfully updated |
| group_expressions | ||
| .iter_mut() | ||
| .filter(|expr| matches!(expr, Expression::Column(_)).not()) | ||
| .for_each(|expr| *expr = Expression::Column(expr.column_name())); | ||
|
|
There was a problem hiding this comment.
Maybe we can use a iter().filter().map().collect() instead?
There was a problem hiding this comment.
If the original group expression is a column, then it will not be in the collect result.
So the group_expressions count will not be the same as before.
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Support base
DISTINCTandDISTINCTwith group by.Changelog
Related Issues
Fixes #5343