HyukjinKwon commented on PR #59403:
URL: https://github.com/apache/spark/pull/59403#issuecomment-6177314601
Thanks for the detailed follow-up. I went through each commit, and the six
points and the stacked-aggregate question are addressed at this head, with one
gap in the pandas case below. On 3f543f3, I'd keep it here: with the normalized
filter pushed below the DISTINCT, removing that DISTINCT in
RemoveRedundantAggregates returns rows that fail the predicate, so the guard is
part of this fix. Linking SPARK-60093 to this PR should be enough to resolve
both. On the pandas UDF skip: `outputsNormalizedKeys` is checked when the
filter is pushed, but ColumnPruning runs later in the same batch and can drop
aggregate expressions. For `df.groupBy("x").agg(pudf(col("v")).as("p"),
count("*").as("c")).where(col("x").cast("string") === "-0.0").select("x", "p")`
over -0.0 keys, the filter is pushed normalized because the functions are
mixed. Then `c` is pruned and the Aggregate is planned as
ArrowAggregatePythonExec on the raw keys, so the -0.0 group is dropped while
master returns it. One follow-u
p on the cost you mention: for LIMIT 1 the rewrite is now skipped entirely, so
LocalLimit(1) is no longer pushed below the Aggregate, and `SELECT DISTINCT d
FROM t LIMIT 1` or `df.distinct().first()` on a DOUBLE column aggregates the
whole input. Could that case keep the Aggregate but still push LocalLimit(1)
under it? Similarly, RemoveRedundantAggregates now keeps the lower aggregate
even when the upper one only groups by the bare key, which the planner
normalizes itself. Also, the comment saying array/map keys are normalized again
on a stacked push doesn't hold: semanticEquals canonicalizes lambda variables,
so the short-circuit keeps them.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"conversation:6173745357","thread_id":"conversation:6173745357","verdict_sha256":"d72a1e87e57537b5aad44c8a6ba3d4a2a08989ae27ec3a312aee8421cf79d32c"}
-->
--
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]
