You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "jayzhan211 (via GitHub)" <gi...@apache.org> on 2024/07/09 14:21:02 UTC
[I] Improve `SingleDistinctToGroupBy` to get the same plan as the `group by` query [datafusion]
jayzhan211 opened a new issue, #11360:
URL: https://github.com/apache/datafusion/issues/11360
### Is your feature request related to a problem or challenge?
While working on #11299 , I meet the issue that the `single distinct plan` is different from `group by` plan. https://github.com/apache/datafusion/pull/11299/commits/fb8a6782e094ed20923875342de79593fc45ef30#r1667248774
I solve the issue by handling different values I got in `update_batach`. But, I think this is not the root cause of the problem.
`SingleDistinctToGroupBy` is converting `distinct` to `group by` expression. Ideally the optimized plan should be the same as the `group by` version, but the following plan is not what I expect.
```
statement ok
create table t(a int) as values (1);
query TT
explain select array_agg(distinct a) from t where a > 3;
----
logical_plan
01)Projection: ARRAY_AGG(alias1) AS ARRAY_AGG(DISTINCT t.a)
02)--Aggregate: groupBy=[[]], aggr=[[ARRAY_AGG(alias1)]]
03)----Aggregate: groupBy=[[t.a AS alias1]], aggr=[[]]
04)------Filter: t.a > Int32(3)
05)--------TableScan: t projection=[a]
physical_plan
01)ProjectionExec: expr=[ARRAY_AGG(alias1)@0 as ARRAY_AGG(DISTINCT t.a)]
02)--AggregateExec: mode=Final, gby=[], aggr=[ARRAY_AGG(alias1)]
03)----CoalescePartitionsExec
04)------AggregateExec: mode=Partial, gby=[], aggr=[ARRAY_AGG(alias1)]
05)--------AggregateExec: mode=FinalPartitioned, gby=[alias1@0 as alias1], aggr=[]
06)----------CoalesceBatchesExec: target_batch_size=8192
07)------------RepartitionExec: partitioning=Hash([alias1@0], 4), input_partitions=4
08)--------------AggregateExec: mode=Partial, gby=[a@0 as alias1], aggr=[]
09)----------------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1
10)------------------CoalesceBatchesExec: target_batch_size=8192
11)--------------------FilterExec: a@0 > 3
12)----------------------MemoryExec: partitions=1, partition_sizes=[1]
query TT
explain select array_agg(a) from t where a > 3 group by a;
----
logical_plan
01)Projection: ARRAY_AGG(t.a)
02)--Aggregate: groupBy=[[t.a]], aggr=[[ARRAY_AGG(t.a)]]
03)----Filter: t.a > Int32(3)
04)------TableScan: t projection=[a]
physical_plan
01)ProjectionExec: expr=[ARRAY_AGG(t.a)@1 as ARRAY_AGG(t.a)]
02)--AggregateExec: mode=FinalPartitioned, gby=[a@0 as a], aggr=[ARRAY_AGG(t.a)]
03)----CoalesceBatchesExec: target_batch_size=8192
04)------RepartitionExec: partitioning=Hash([a@0], 4), input_partitions=4
05)--------AggregateExec: mode=Partial, gby=[a@0 as a], aggr=[ARRAY_AGG(t.a)]
06)----------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1
07)------------CoalesceBatchesExec: target_batch_size=8192
08)--------------FilterExec: a@0 > 3
09)----------------MemoryExec: partitions=1, partition_sizes=[1]
```
### Describe the solution you'd like
Rewrite `SingleDistinctToGroupBy` so the optimized plan should be the same like the `group by` version.
### Describe alternatives you've considered
Do nothing but add the docs about the reason of why we can't
### Additional context
_No response_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Improve `SingleDistinctToGroupBy` to get the same plan as the `group by` query [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #11360:
URL: https://github.com/apache/datafusion/issues/11360#issuecomment-2243812261
> What is the rationale of single distinct to group by? Take count for example, I think distinct accumulator could be way more efficient than normal accumulator with group by ๐ค
Before we had specialized and optimized accumulators (e.g the groups accumulators) I think the hash group by was more efficient than using special aggregators. Now that we have special accumulators for distinct aggregates, it may not be needed.
we could run some benchmarks to check
It also lets you do stuff like reuse the grouping if the same argument is shared between accumlators (`min(distinct x)` and `max(distinct x)` can be rewritten to deduplicate `x` once
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Improve `SingleDistinctToGroupBy` to get the same plan as the `group by` query [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11360:
URL: https://github.com/apache/datafusion/issues/11360#issuecomment-2249149122
```
--------------------
Benchmark clickbench_1.json
--------------------
โโโโโโโโโโโโโโโโณโโโโโโโโโโโโโณโโโโโโโโโโโโโณโโโโโโโโโโโโโโโโ
โ Query โ main โ rm-group โ Change โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ QQuery 0 โ 0.51ms โ 0.47ms โ +1.09x faster โ
โ QQuery 1 โ 39.48ms โ 40.42ms โ no change โ
โ QQuery 2 โ 72.55ms โ 74.66ms โ no change โ
โ QQuery 3 โ 60.83ms โ 64.15ms โ 1.05x slower โ
โ QQuery 4 โ 411.03ms โ 780.34ms โ 1.90x slower โ
โ QQuery 5 โ 689.68ms โ 848.49ms โ 1.23x slower โ
โ QQuery 6 โ 36.17ms โ 39.55ms โ 1.09x slower โ
โ QQuery 7 โ 38.18ms โ 36.60ms โ no change โ
โ QQuery 8 โ 752.03ms โ 520.39ms โ +1.45x faster โ
โ QQuery 9 โ 646.74ms โ 650.58ms โ no change โ
โ QQuery 10 โ 192.66ms โ 194.48ms โ no change โ
โ QQuery 11 โ 217.84ms โ 222.11ms โ no change โ
โ QQuery 12 โ 721.83ms โ 725.29ms โ no change โ
โ QQuery 13 โ 1531.53ms โ 2302.53ms โ 1.50x slower โ
โ QQuery 14 โ 998.00ms โ 997.10ms โ no change โ
โ QQuery 15 โ 487.51ms โ 494.70ms โ no change โ
โ QQuery 16 โ 2073.38ms โ 1678.31ms โ +1.24x faster โ
โ QQuery 17 โ 2035.91ms โ 1731.46ms โ +1.18x faster โ
โ QQuery 18 โ 5263.26ms โ 4561.70ms โ +1.15x faster โ
โ QQuery 19 โ 58.90ms โ 55.16ms โ +1.07x faster โ
โ QQuery 20 โ 1520.68ms โ 1473.57ms โ no change โ
โ QQuery 21 โ 1732.55ms โ 1742.90ms โ no change โ
โ QQuery 22 โ 4368.40ms โ 4373.69ms โ no change โ
โ QQuery 23 โ 8494.32ms โ 8264.09ms โ no change โ
โ QQuery 24 โ 489.16ms โ 491.06ms โ no change โ
โ QQuery 25 โ 496.44ms โ 493.40ms โ no change โ
โ QQuery 26 โ 548.20ms โ 554.44ms โ no change โ
โ QQuery 27 โ 1300.33ms โ 1310.05ms โ no change โ
โ QQuery 28 โ 10281.79ms โ 10086.87ms โ no change โ
โ QQuery 29 โ 420.42ms โ 419.32ms โ no change โ
โ QQuery 30 โ 877.19ms โ 850.86ms โ no change โ
โ QQuery 31 โ 1010.85ms โ 991.67ms โ no change โ
โ QQuery 32 โ 9338.74ms โ 8808.60ms โ +1.06x faster โ
โ QQuery 33 โ 3646.16ms โ 4057.37ms โ 1.11x slower โ
โ QQuery 34 โ 3540.59ms โ 4003.55ms โ 1.13x slower โ
โ QQuery 35 โ 1050.15ms โ 1151.70ms โ 1.10x slower โ
โ QQuery 36 โ 144.86ms โ 146.93ms โ no change โ
โ QQuery 37 โ 102.85ms โ 100.53ms โ no change โ
โ QQuery 38 โ 107.28ms โ 107.49ms โ no change โ
โ QQuery 39 โ 389.38ms โ 396.20ms โ no change โ
โ QQuery 40 โ 35.23ms โ 36.68ms โ no change โ
โ QQuery 41 โ 33.02ms โ 33.52ms โ no change โ
โ QQuery 42 โ 41.73ms โ 45.64ms โ 1.09x slower โ
โโโโโโโโโโโโโโโโดโโโโโโโโโโโโโดโโโโโโโโโโโโโดโโโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโณโโโโโโโโโโโโโ
โ Benchmark Summary โ โ
โกโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโฉ
โ Total Time (main) โ 66298.33ms โ
โ Total Time (rm-group) โ 65958.65ms โ
โ Average Time (main) โ 1541.82ms โ
โ Average Time (rm-group) โ 1533.92ms โ
โ Queries Faster โ 7 โ
โ Queries Slower โ 9 โ
โ Queries with No Change โ 27 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโดโโโโโโโโโโโโโ
```
No clear advantage of whether convert to group or not
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Improve `SingleDistinctToGroupBy` to get the same plan as the `group by` query [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11360:
URL: https://github.com/apache/datafusion/issues/11360#issuecomment-2222082311
I found we accidentally convert non-single expression to group by too, we should not convert `select bit_xor(distinct a % 2) from t;` to group by expression.
```
// datafusion
statement ok
create table t(a int) as values (1), (1), (2), (2), (2), (3);
query I
select bit_xor(distinct a % 2) from t;
----
1
query TT
explain select bit_xor(distinct a % 2) from t;
----
logical_plan
01)Projection: bit_xor(alias1) AS bit_xor(DISTINCT t.a % Int64(2))
02)--Aggregate: groupBy=[[]], aggr=[[bit_xor(alias1)]]
03)----Aggregate: groupBy=[[CAST(t.a AS Int64) % Int64(2) AS alias1]], aggr=[[]]
04)------TableScan: t projection=[a]
physical_plan
01)ProjectionExec: expr=[bit_xor(alias1)@0 as bit_xor(DISTINCT t.a % Int64(2))]
02)--AggregateExec: mode=Final, gby=[], aggr=[bit_xor(alias1)]
03)----CoalescePartitionsExec
04)------AggregateExec: mode=Partial, gby=[], aggr=[bit_xor(alias1)]
05)--------AggregateExec: mode=FinalPartitioned, gby=[alias1@0 as alias1], aggr=[]
06)----------CoalesceBatchesExec: target_batch_size=8192
07)------------RepartitionExec: partitioning=Hash([alias1@0], 4), input_partitions=4
08)--------------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1
09)----------------AggregateExec: mode=Partial, gby=[CAST(a@0 AS Int64) % 2 as alias1], aggr=[]
10)------------------MemoryExec: partitions=1, partition_sizes=[1]
query I
select bit_xor(a % 2) from t group by a;
----
0
0
1
query I
select bit_xor(a % 2) from t group by (a % 2);
----
1
0
```
```
D create table t(a int);
D insert into t values (1);
D insert into t values (1);
D insert into t values (2);
D insert into t values (2);
D insert into t values (2);
D insert into t values (3);
D select bit_xor(distinct a % 2) from t;
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โ bit_xor(DISTINCT (a % 2)) โ
โ int32 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโค
โ 1 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
D select bit_xor(a % 2) from t group by a;
โโโโโโโโโโโโโโโโโโโโ
โ bit_xor((a % 2)) โ
โ int32 โ
โโโโโโโโโโโโโโโโโโโโค
โ 0 โ
โ 0 โ
โ 1 โ
โโโโโโโโโโโโโโโโโโโโ
D select bit_xor(a % 2) from t group by (a % 2);
โโโโโโโโโโโโโโโโโโโโ
โ bit_xor((a % 2)) โ
โ int32 โ
โโโโโโโโโโโโโโโโโโโโค
โ 1 โ
โ 0 โ
โโโโโโโโโโโโโโโโโโโโ
D explain select bit_xor(distinct a % 2) from t;
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โโ Physical Plan โโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
โ UNGROUPED_AGGREGATE โ
โ โ โ โ โ โ โ โ โ โ โ โ โ
โ bit_xor(DISTINCT #0) โ
โโโโโโโโโโโโโโโฌโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโดโโโโโโโโโโโโโโ
โ PROJECTION โ
โ โ โ โ โ โ โ โ โ โ โ โ โ
โ (a % 2) โ
โโโโโโโโโโโโโโโฌโโโโโโโโโโโโโโ
โโโโโโโโโโโโโโโดโโโโโโโโโโโโโโ
โ SEQ_SCAN โ
โ โ โ โ โ โ โ โ โ โ โ โ โ
โ t โ
โ โ โ โ โ โ โ โ โ โ โ โ โ
โ a โ
โ โ โ โ โ โ โ โ โ โ โ โ โ
โ EC: 6 โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Improve `SingleDistinctToGroupBy` to get the same plan as the `group by` query [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11360:
URL: https://github.com/apache/datafusion/issues/11360#issuecomment-2222812983
What is the rationale of introduce single distinct to group by? Take `count` for example, I think distinct accumulator could be way more efficient than normal accumulator with group by ๐ค
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org