comphead commented on code in PR #2429:
URL: https://github.com/apache/datafusion-comet/pull/2429#discussion_r2366390841
##########
spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:
##########
@@ -558,12 +558,6 @@ object QueryPlanSerde extends Logging with CometExprShim {
binding: Boolean,
conf: SQLConf): Option[AggExpr] = {
- if (aggExpr.isDistinct) {
Review Comment:
this is a good point, I was thinking the same but IMO Spark doesn't call the
count distinct on partial phase.
```
+-----------------------------+
| Driver |
| COUNT(DISTINCT name) |
+-------------+---------------+
|
v
+-------------------+ +-------------------+
| Executor 1 | | Executor 2 |
| Partitions P0,P1 | | Partitions P2,P3 |
| Local distinct: | | Local distinct: |
| {Alice,Bob,Eve} | | {Mallory,Eve,Bob,Trent}|
+---------+---------+ +---------+---------+
| |
| Shuffle |
v v
+--------------+ +--------------+
| Reducer R0 | | Reducer R1 |
| {Alice,Bob,Eve} | {Mallory,Trent} |
+--------------+ +--------------+
\ /
\ /
\ /
+-----------+-------------+
v
Driver Final Merge
DISTINCT = 5
```
The local distinct is made by `HashAggregate` so when count distinct get
called as `aggExpr` it might not be needing the flag as data already deduped on
reducers. Checking the Final stage though
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
