Github user srinathshankar commented on a diff in the pull request: https://github.com/apache/spark/pull/14912#discussion_r77748668 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FilterPushdownSuite.scala --- @@ -171,6 +172,27 @@ class FilterPushdownSuite extends PlanTest { comparePlans(optimized, correctAnswer) } + test("push down filters that are combined") { + // The following predicate ('a === 2 || 'a === 3) && ('c > 10 || 'a === 2) + // will be simplified as ('a == 2) || ('c > 10 && 'a == 3). + // ('a === 2 || 'a === 3) can be pushed down. But the simplified one can't. --- End diff -- I agree with you that we should respect the interaction between CombineFilters, PushDownPredicates and other rules. I do think it's important that cnf conversion run before any of the push-down / reordering rules. And the simplification rules should run afterwards. My concern with rolling this into CombineFilters is that it doesn't get triggered unless there are adjoining Filter nodes. In the example you have: val originalQuery = testRelation .select('a, 'b, ('c + 1) as 'cc) .groupBy('a)('a, count('cc) as 'c) .where('c > 10) .where(('a === 2) || ('c > 10 && 'a === 3)) I think that (a == 2 || a==3) should get pushed down even if you don't have ".where (c > 10)", but I'm not sure that it will be since toCNF is in CombineFilters. Could you confirm ? My suggestion is that toCNF warrants a separate rule -- for example when you're doing joins, and you have select * from A inner join C on (A.a1 = C.c1) where A.a2 = 2 || (C.c2 = 10 && A.a2 = 3), you want (A.a2 = 2 || A.a2 = 3) pushed down into A
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org