pan3793 commented on code in PR #58735:
URL: https://github.com/apache/spark/pull/58735#discussion_r3988410722
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PushdownPredicatesAndPruneColumnsForCTEDef.scala:
##########
@@ -57,8 +57,8 @@ object PushdownPredicatesAndPruneColumnsForCTEDef extends
Rule[LogicalPlan] with
/**
* Gather all the predicates and referenced attributes on different points
of CTE references
- * using pattern `ScanOperation` (which takes care of determinism) and
combine those predicates
- * and attributes that belong to the same CTE definition.
+ * using pattern `PhysicalOperation` and combine those predicates and
attributes that belong
+ * to the same CTE definition.
Review Comment:
Confirmed, and the description was wrong. At `175e429cca2` `ScanOperation`
ran with `legacyMode = false`, whose `filters.isEmpty ||
(filters.forall(_.deterministic) && condition.deterministic)` admits the first
filter whatever it is, so a lone non-deterministic filter came back pushable
there too. The rule shipped with this bug in SPARK-37670 (3.4.0); SPARK-39764
changed nothing here and "which takes care of determinism" was never true. Took
your scaladoc wording. PR description and commit message updated to name
SPARK-37670 as the origin. Affected versions unchanged.
--
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]
