maropu commented on a change in pull request #29170:
URL: https://github.com/apache/spark/pull/29170#discussion_r519511395



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
##########
@@ -116,7 +116,8 @@ abstract class Optimizer(catalogManager: CatalogManager)
         operatorOptimizationRuleSet.filterNot(_ == InferFiltersFromConstraints)
       Batch("Operator Optimization before Inferring Filters", fixedPoint,
         rulesWithoutInferFiltersFromConstraints: _*) ::
-      Batch("Infer Filters", Once,
+      Batch("Infer Filters", fixedPoint,
+        PushDownPredicates,

Review comment:
       > I think if we combine InferFiltersFromConstraints with other rules 
that doesn't reduce constraints (like PushDownPredicates) then we are still 
good. But I might be wrong so please share your thoughts on this.
   
   Yea, I don't have a specific query to deny your thought and I also think it 
is true, but, in my current feeling, I a bit hesitate to change `Once` -> 
`fixedPoint ` for this batch without adding no logic to avoid the infinite loop 
case (and without any evidence that it will not affect users queries...). WDTY, 
@cloud-fan @viirya ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to