Attila Jeges has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE ......................................................................
Patch Set 2: (6 comments) Thanks for the review! http://gerrit.cloudera.org:8080/#/c/7564/1/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: PS1, Line 119: RuntimeFilterGener > unnecessary Done PS1, Line 121: : > single line? Done http://gerrit.cloudera.org:8080/#/c/7564/1/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: Line 538: * The assigned filters are the ones for which 'scanNode' can be used a destination > update this comment Done PS1, Line 549: Preconditions.checkNotNull(scanNode); : Analyzer analyzer = ctx.getRootAnalyzer(); > single line? Done PS1, Line 590: e > nit: space before ':' Done http://gerrit.cloudera.org:8080/#/c/7564/1/testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test File testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test: Line 1: ==== > I suppose these are written as query tests and not planner tests because th Correct. These test cases should be in the planner test suite, but implementing them this way is more straightforward. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
