[CALCITE-7534] Support GROUPING SETS, CUBE and ROLLUP in materialized view aggregate rule rewriting - #4949
[CALCITE-7534] Support GROUPING SETS, CUBE and ROLLUP in materialized view aggregate rule rewriting#4949xuzifu666 wants to merge 1 commit into
Conversation
a54971b to
472b7c6
Compare
|
@mihaibudiu Could you please help to review this pr when you have a time? As far as I know, materialized view scenes are used quite frequently in production, very thanks! |
|
|
||
| /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7534">[CALCITE-7534] | ||
| * Support GROUPING SETS, CUBE and ROLLUP in materialized view aggregate rule rewriting</a>. */ | ||
| @Test void testAggregateGroupSets1() { |
There was a problem hiding this comment.
By reading this test I cannot tell why it is related to materialized view aggregate rewriting.
Could the test contain two versions of the expected program: with and without the rewriting?
There was a problem hiding this comment.
The original tests only showed the rewritten plan (after materialized view matching), which made it hard to understand what optimization was actually being tested.So I added Javadoc showing the changes in the plan before and after. Does this achieve the desired result? @mihaibudiu
|
I had restructure tests to assert both the non-rewritten and rewritten plans. Does this meet your expectations? Could you please help to review it again~ @mihaibudiu |
|
mihaibudiu
left a comment
mihaibudiu
left a comment
There was a problem hiding this comment.
I actually don't know anything about this part of Calcite.
Can you try to get a review from some of the people who wrote this code in the first place?
Unfortunately this looks like it's 6-7 years old by now.
If you can't get anyone I can try, but I will have to get familiar with this part.
OK, As @mihaibudiu mentioned, this part was written by you several years ago. Could you please review it? @jcamachor very thanks! |
|
Can you try to find out other people who have worked on materialized view reuse? Maybe they can supply a review |
Currently, it seems only @jcamachor is related to this modification; I need to look for the others. |
… view aggregate rule rewriting
19cee8b to
8749363
Compare
|
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |
2 participants
jira: https://issues.apache.org/jira/browse/CALCITE-7534