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(planner): implement aggregate operator in new planner framework #5027

Merged
merged 15 commits into from
Apr 28, 2022

Conversation

xudong963
Copy link
Member

@xudong963 xudong963 commented Apr 24, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

  • implement aggregate operator in the new planner framework

Changelog

  • New Feature

Related Issues

Fixes #3749 #5066

@vercel
Copy link

vercel bot commented Apr 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Apr 28, 2022 at 11:09AM (UTC)

@xudong963 xudong963 marked this pull request as draft April 24, 2022 08:18
@mergify
Copy link
Contributor

mergify bot commented Apr 24, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify bot added the pr-feature this PR introduces a new feature to the codebase label Apr 24, 2022
@xudong963
Copy link
Member Author

Currently, the ticket can't produce results due to two known reasons:

  1. bug: select can't work in new parser #5044
  2. The name style for anonymous select item #5053

But, it's time to review, ensure me on the right road. And if the ticket is ok, I think we can merge it and make it work in next tickets.

@xudong963 xudong963 marked this pull request as ready for review April 26, 2022 02:33
@xudong963 xudong963 requested a review from leiysky April 26, 2022 02:45
query/src/sql/exec/mod.rs Outdated Show resolved Hide resolved
@xudong963 xudong963 requested a review from andylokandy April 26, 2022 08:09
@BohuTANG
Copy link
Member

Conflicts need resolved :/

query/src/sql/exec/mod.rs Outdated Show resolved Hide resolved
@xudong963 xudong963 requested a review from leiysky April 27, 2022 06:47
scalar_exprs.push(
scalar_binder
.bind_expr(arg, bind_context)?
.as_any()
Copy link
Member

Choose a reason for hiding this comment

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

maybe move .as_any().downcast_ref.ok_or_else into ScalarExprRef is better?

impl ScalarExpr {
    pub fn safe_cast<To>(&self) -> Result<To> {
        self.as_any().downcast_ref::<To>().ok_or_else(|| {...})
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice idea!

col_binding
.scalar
.as_ref()
.unwrap()
Copy link
Member

Choose a reason for hiding this comment

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

unwrap.

@xudong963
Copy link
Member Author

xudong963 commented Apr 28, 2022

Wow, it works in my local !

mysql> set enable_planner_v2=1;
Query OK, 0 rows affected (0.02 sec)
Read 0 rows, 0 B in 0.002 sec., 0 rows/sec., 0 B/sec.

mysql> select sum(a)  from t group by a;
+----------+
| sum(a_0) |
+----------+
|        1 |
|        2 |
+----------+
2 rows in set (0.04 sec)
Read 2 rows, 8 B in 0.008 sec., 262.05 rows/sec., 1.05 KB/sec.

Will update PR after sorting out code 😄

@BohuTANG
Copy link
Member

Some conflicts that must be resolved :D

@xudong963
Copy link
Member Author

Some conflicts that must be resolved :D

Ok, now start to solve conflicts and use enum_dispatch 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-review pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement aggregate operator in new planner framework
7 participants