impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5003: Constant propagation in scan nodes
Date Wed, 05 Apr 2017 05:56:52 GMT
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 <zamsden@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message