Alex Behm has posted comments on this change. Change subject: IMPALA-5003: Constant propagation in scan nodes ......................................................................
Patch Set 15: (5 comments) http://gerrit.cloudera.org:8080/#/c/6389/15/testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test File testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test: Line 109: # Aggregate doesn't affect pushing predicates This test is not specific to constant propagation, so let's leave it out. Line 124: # Again, the contradiction is derived in the ScanNode Same here, this test seems to be more about predicate assignment than constant propagation. Line 139: # Make sure predicates are not pushed into inline views with limits Same here, this test is about predicate assignment (plenty of coverage elsewhere already). We should really only be testing the optimizeConjuncts() functionality in isolation here, Line 161: # We can optimize in some cases Instead of testing various predicate assignment scenarios, we should focus more on interesting cases for optimizeConjuncts(). Some ideas; - All conjuncts are optimized to a single "true" - Conjuncts with NULLs - Duplicate conjuncts - Test propagation into non-trivial exprs, e.g. disjunctions, function call exprs - Test errors during constant propagation (I'm still thinking whether there's an easy way to do this) For HDFS scans, we should also optimize the collectionConjuncts_, and add 1-2 tests for that case. Have you tried an extreme example with lots of conjuncts to see how long this optimization takes? Should we consider having a limit on when not to apply constant propagation due to prohibitive optimization cost? http://gerrit.cloudera.org:8080/#/c/6389/15/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: Line 127: ---- SCANRANGELOCATIONS Don't think we need the SCANRANGELOCATIONS and DISTRIBUTEDPLAN sections. -- To view, visit http://gerrit.cloudera.org:8080/6389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79750a8edb945effee2a519fa3b8192b77042cb4 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
