Uh oh!
There was an error while loading. Please reload this page.
Support no distinct aggregate sum/min/max in single_distinct_to_group_by rule - #8124
Support no distinct aggregate sum/min/max in single_distinct_to_group_by rule#8124haohuaijin wants to merge 9 commits into
single_distinct_to_group_by rule#8124Conversation
Uh oh!
There was an error while loading. Please reload this page.
| for e in args { | ||
| fields_set.insert(e.canonical_name()); | ||
| match filter { | ||
| Some(_) => return Ok(false), |
There was a problem hiding this comment.
Before this pr, we also don't support filter in single_distinct_to_group_by rule. But we forget check it.
The behaviors of I test on this branch: DataFusion CLI v33.0.0
❯ create table t(a int, b int);
0 rows in set. Query took 0.016 seconds.
❯ selectcount(distinct a), count(b) from t;
+---------------------+------------+
| COUNT(DISTINCT t.a) | COUNT(t.b) |
+---------------------+------------+
| 0 ||
+---------------------+------------+
1 row in set. Query took 0.029 seconds.
Maybe we need a special |
@jonahgao thanks for your provider this case. I didn't read the source code of calcite carefully.
I have no preference between the two methods, what do you think @alamb ? |
alamb
commented
Nov 13, 2023
I hope to review this PR later today |
single_distinct_to_group_by rulesingle_distinct_to_group_by rulealamb
commented
Nov 14, 2023
I am sorry -- I did not have a chance to get to this one. Note that this PR seems to have non trivial conflicts now (perhaps due to #8176 |
haohuaijin
commented
Nov 15, 2023
Due to #8176, I close this pr, and then reorganize the code and submit a new pr. |
Which issue does this PR close?
Closes#8123
Rationale for this change
In this pr
in main 43cc870
What changes are included in this PR?
add no-distinct sum/min/max aggregate support in
single_distinct_to_group_byruleAre these changes tested?
yes, add soem tests
Are there any user-facing changes?