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: (2 comments) http://gerrit.cloudera.org:8080/#/c/7564/2/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: PS2, Line 119: singleNodePlan > Let's expand to the comment to mention that 'singleNodePlan' now points to Done http://gerrit.cloudera.org:8080/#/c/7564/2/tests/query_test/test_runtime_filters.py File tests/query_test/test_runtime_filters.py: PS2, Line 87: def test_distrib_plan_filter_pruning(self, vector): : vector.get_value('exec_option')['num_nodes'] = 0 : self.run_test_case('QueryTest/runtime_filters_distrib_pruning', vector) : : def test_singlenode_plan_filter_pruning(self, vector): : vector.get_value('exec_option')['num_nodes'] = 1 : self.run_test_case('QueryTest/runtime_filters_singlenode_pruning', vector) > I don't follow. Did you check how the query option is set in testRuntimeFil Let me give you some context: In the current patch-set 'runtime_filters_distrib_pruning.test' and 'runtime_filters_singlenode_pruning.test' contain 11 test cases in total. Each of them tests runtime filter assignment with a different combination of query options. There are inline SET statements inside the .test files before each test case to set the options for that test. To implement this in PlannerTest.java, I would have to create 11 '.test' files, one for each test case. This is necessary because PlannerTest.java does not support inline SET statements inside the .test files. This approach seemed too much trouble to me. What do you think? -- 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
