[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 26 Oct 2017 17:06:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Alex Behm has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Reviewed-on: http://gerrit.cloudera.org:8080/7564 Tested-by: Impala Public Jenkins Reviewed-by: Alex Behm --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test 12 files changed, 775 insertions(+), 108 deletions(-) Approvals: Impala Public Jenkins: Verified Alex Behm: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 14 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 13: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 26 Oct 2017 10:50:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1393/ -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 26 Oct 2017 07:01:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 13: > Uploaded patch set 13: Patch Set 12 was rebased. Thanks for the review. I think it needed a rebase. Could you restart the job? -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 26 Oct 2017 06:11:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test 12 files changed, 775 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/13 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 12: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1388/ -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 12 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 25 Oct 2017 22:44:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 12: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1388/ -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 12 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 25 Oct 2017 18:40:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Alex Behm has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 12: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@288
PS10, Line 288: if (!currentTestCase.isValid()) {
> Correct, every PlannerTest .test file ends with "". I've removed parse
I agree. This code is really confusing and could use some cleanup - but not in
this patch.
I believe we already have a check similar to the one you proposed. Try creating
an invalid .test file, e.g., by omitting the last "".
--
To view, visit http://gerrit.cloudera.org:8080/7564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 12
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Wed, 25 Oct 2017 18:39:16 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test 12 files changed, 775 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/12 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 12 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 11:
(10 comments)
Thanks for the review!
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@455
PS10, Line 455: TQueryOptions options;
> Thanks for investigating and providing an explanation!
Done
http://gerrit.cloudera.org:8080/#/c/7564/11/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/11/be/src/service/fe-support.cc@454
PS11, Line 454: JNIEnv* env, jclass caller_class, jstring options_str,
jbyteArray tquery_options) {
> rename options_str to csv_query_options for consistency
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@288
PS10, Line 288: if (!currentTestCase.isValid()) {
> All PlannerTest .test files should end with "", so I think the code her
Correct, every PlannerTest .test file ends with "". I've removed
parseQueryOptions() from L338.
I find the code in L339-L344 very confusing. If the .test file doesn't end
with "", the last non-terminated test case is still validated and returned
to the caller. In addition, if the very last section is not "" terminated
it will be silently ignored.
Maybe we should do something like this instead:
if (!currentTestCase.isEmpty() || !sectionContents.isEmpty()) {
throw new IllegalStateException("Test case" +
" at line " + currentTestCase.startLineNum +
" is not '' terminated.");
}
return currentTestCase;
http://gerrit.cloudera.org:8080/#/c/7564/11/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/11/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@359
PS11, Line 359: " - " + e.getMessage());
> also pass in 'e' as the cause
Done
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
File
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test:
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@38
PS11, Line 38: DISTRIBUTEDPLAN
> What's the value in keeping this section?
I would like to keep it as a reference, so that we can see the effect of
RUNTIME_FILTER_MODE=LOCAL in L339 and L422.
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@174
PS11, Line 174: DISTRIBUTEDPLAN
> What's the value in keeping this section?
Removed it.
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@257
PS11, Line 257: DISTRIBUTEDPLAN
> What's the value in keeping this section?
Removed it.
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@392
PS11, Line 392: MAX_NUM_RUNTIME_FILTERS=4
> How about setting to 3 so we can see both local and non-local filters being
Done
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@582
PS11, Line 582: DISTRIBUTEDPLAN
> What's the value in keeping this section?
Removed it.
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@657
PS11, Line 657: DISTRIBUTEDPLAN
> What's the value in keeping this section?
Removed it.
--
To view, visit http://gerrit.cloudera.org:8080/7564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 11
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Wed, 25 Oct 2017 16:48:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Alex Behm has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 11:
(13 comments)
Looks good, final comments.
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@455
PS10, Line 455: TQueryOptions options;
> I've renamed the parameter.
Thanks for investigating and providing an explanation!
We should summarize your findings and add them to the function comment of
FeSupport#NativeParseQueryOptions() to explain why we must pass in an existing
TQueryOptions for this to work as intended.
http://gerrit.cloudera.org:8080/#/c/7564/11/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/11/be/src/service/fe-support.cc@453
PS11, Line 453:
Java_org_apache_impala_service_FeSupport_NativeParseQueryOptions(
Nice and clean! Thanks.
http://gerrit.cloudera.org:8080/#/c/7564/11/be/src/service/fe-support.cc@454
PS11, Line 454: JNIEnv* env, jclass caller_class, jstring options_str,
jbyteArray tquery_options) {
rename options_str to csv_query_options for consistency
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@397
PS10, Line 397: int maxNumFilters =
ctx.getQueryOptions().getMax_num_runtime_filters();
> 0 is a valid value. MAX_NUM_RUNTIME_FILTERS=0 effectively removes every run
Sorry, my mistake. For some reason I didn't read the ">=" correctly.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/service/FeSupport.java@93
PS10, Line 93:
> See my response to be/src/service/fe-support.cc L455.
Yikes! What a can of worms. Thanks for investigating and explaining.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@288
PS10, Line 288: if (!currentTestCase.isValid()) {
> L288-L293 handles test cases found in the middle of the test file.
All PlannerTest .test files should end with "", so I think the code here in
L228 is enough. I tried your patch locally using a .test file with 3 tests and
the query option parsing worked as expected without L338.
I might be missing something, if so, can you point me to an example file where
we also need the parseQueryOptions() call in L338?
http://gerrit.cloudera.org:8080/#/c/7564/11/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/11/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@359
PS11, Line 359: " - " + e.getMessage());
also pass in 'e' as the cause
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
File
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test:
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@38
PS11, Line 38: DISTRIBUTEDPLAN
What's the value in keeping this section?
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@174
PS11, Line 174: DISTRIBUTEDPLAN
What's the value in keeping this section?
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@257
PS11, Line 257: DISTRIBUTEDPLAN
What's the value in keeping this section?
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@392
PS11, Line 392: MAX_NUM_RUNTIME_FILTERS=4
How about setting to 3 so we can see both local and non-local filters being
removed from the distributed plan
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test@582
PS11, Line 582: DISTRIBUTEDPLAN
What's the value in keeping this section?
http://gerrit.cloudera.org:8080/#/c/7564/11/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-q
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 10:
(26 comments)
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@455
PS10, Line 455: jbyteArray thrift_in_query_options) {
> Not sure what 'thrift_in_query_options' means. How about using 'tquery_opti
I've renamed the parameter.
I decided to pass in the existing query options because the thrift-generated
C++ and Java TQueryOptions classes don't handle the isset flags properly.
If I don't pass in the existing query options, then:
1. I'd have to create an "empty" TQueryOptions instance inside this function.
2. The __isset member of that instance would contain true values for every
option even though they haven't been set explicitly. Therefore I'd have to
overwrite __isset with false values somehow before calling
impala::ParseQueryOptions().
3. When FeSupport.ParseQueryOptions() deserializes the TQueryOptions instance
returned by Java_org_apache_impala_service_FeSupport_NativeParseQueryOptions(),
it would still find that some query options have been set, even if they haven't
been touched (e.g. TQueryOptions.setExplain_levelIsSet() always returns true
since explain_level != null even if it hasn't been set explicitly).
I'm not sure why thrift-generated classes behave this way. Maybe there's a
better way to work around the isset bugs, but I couldn't think of any.
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@457
PS10, Line 457: DeserializeThriftMsg(env, thrift_in_query_options, &options);
> Needs error handling, e.g. using THROW_IF_ERROR_RET
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@458
PS10, Line 458: const char *o = env->GetStringUTFChars(options_str, NULL);
> * You need to release the string UTF chars, take a look at JniUtfCharGuard
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@461
PS10, Line 461: TParseQueryOptionsResult result;
> I think it's simpler to convert all Status to an exception. That way we don
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/Planner.java@119
PS10, Line 119: // Create runtime filters. 'singleNodePlan' now points to
the root of the distributed
> Comment is wrong. 'singleNodePlan' definitely does not point to the root of
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@354
PS10, Line 354: public void addTarget(RuntimeFilterTarget target) {
targets_.add(target); }
> add newline before
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@397
PS10, Line 397: Preconditions.checkState(maxNumFilters >= 0);
> Why is 0 not a valid value?
0 is a valid value. MAX_NUM_RUNTIME_FILTERS=0 effectively removes every runtime
filter.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@423
PS10, Line 423: filter.computeNdvEstimate();
> My understanding is that IMPALA-3450 has been fixed and we can remove this
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@537
PS10, Line 537:* The assigned filters are the ones for which 'scanNode' can
be used a destination
> can be used as a destination node
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@539
PS10, Line 539:* 1. If DISABLE_ROW_RUNTIME_FILTERING query option is set, a
filter is only assigned to
> If the DISABLE_ROW_RUNTIME_FILTERING query option is set ...
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@541
PS10, Line 541:* 2. If RUNTIME_FILTER_MODE query option is set to LOCAL, a
filter is only assigned to
> If the RUNTIME_FILTER_MODE query option is set ...
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@542
PS10, Line 542:*'scanNode' if the filter is local to the scan node.
> This doesn't really explain what the LOCAL option means. How about:
Done
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/ja
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test 12 files changed, 927 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/11 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 11 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Alex Behm has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 10:
(27 comments)
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@455
PS10, Line 455: jbyteArray thrift_in_query_options) {
Not sure what 'thrift_in_query_options' means. How about using 'tquery_options'
instead? Ideally, we would not need to pass in the existing query options.
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@457
PS10, Line 457: DeserializeThriftMsg(env, thrift_in_query_options, &options);
Needs error handling, e.g. using THROW_IF_ERROR_RET
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@458
PS10, Line 458: const char *o = env->GetStringUTFChars(options_str, NULL);
* You need to release the string UTF chars, take a look at JniUtfCharGuard in
jni-util.h
* style: const char* o
http://gerrit.cloudera.org:8080/#/c/7564/10/be/src/service/fe-support.cc@461
PS10, Line 461: TParseQueryOptionsResult result;
I think it's simpler to convert all Status to an exception. That way we don't
need the TParseQueryOptionsResult at all, and the error handling is consistent
(always throws an exception in case of error).
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/Planner.java@119
PS10, Line 119: // Create runtime filters. 'singleNodePlan' now points to
the root of the distributed
Comment is wrong. 'singleNodePlan' definitely does not point to the root of the
distributed plan.
Use rootFragment.getPlanRoot() instead
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@354
PS10, Line 354: public void addTarget(RuntimeFilterTarget target) {
targets_.add(target); }
add newline before
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@397
PS10, Line 397: Preconditions.checkState(maxNumFilters >= 0);
Why is 0 not a valid value?
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@423
PS10, Line 423: filter.computeNdvEstimate();
My understanding is that IMPALA-3450 has been fixed and we can remove this code.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@537
PS10, Line 537:* The assigned filters are the ones for which 'scanNode' can
be used a destination
can be used as a destination node
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@539
PS10, Line 539:* 1. If DISABLE_ROW_RUNTIME_FILTERING query option is set, a
filter is only assigned to
If the DISABLE_ROW_RUNTIME_FILTERING query option is set ...
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@541
PS10, Line 541:* 2. If RUNTIME_FILTER_MODE query option is set to LOCAL, a
filter is only assigned to
If the RUNTIME_FILTER_MODE query option is set ...
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@542
PS10, Line 542:*'scanNode' if the filter is local to the scan node.
This doesn't really explain what the LOCAL option means. How about:
... a filter is only assigned to 'scanNode' if the filter is produced within
the same fragment that contains the scan node.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@563
PS10, Line 563: if (!isSingleNodeExec && runtimeFilterMode ==
TRuntimeFilterMode.LOCAL &&
Why the !isSingleNodeExec condition? I think that !isLocalTarget implies
!isSingleNodeExec.
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@576
PS10, Line 576: static private boolean IsLocalTarget(RuntimeFilter filter,
ScanNode targetNode) {
isLocalTarget()
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@583
PS10, Line 583: static private boolean IsBoundByPartitionColumns(Analyzer
analyzer, Expr targetExpr,
isBoundByPartitionColumns()
http://gerrit.cloudera.org:8080/#/c/7564/10/fe/src/main/java/org/apach
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 10: > (5 comments) > > Nice. So much better. I'll let Alex take a look as well and do the > final +2. Thanks for the review! -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 06 Oct 2017 10:29:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 13 files changed, 743 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/10 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java@292
PS9, Line 292: paresResult
> I believe you meant parseResult?
Yes, fixed it.
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@125
PS9, Line 125: public int getStartingLineNum() {
: return startLineNum;
: }
> single line
Done
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@129
PS9, Line 129: public TQueryOptions getOptions() {
: return this.options;
: }
> nit: single line
Done
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@133
PS9, Line 133: public void setOptions(TQueryOptions options) {
: this.options = options;
: }
> single line
Done
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@367
PS9, Line 367:
> Add a Preconditions.checkNotNull(result) here.
Done
--
To view, visit http://gerrit.cloudera.org:8080/7564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 9
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Fri, 06 Oct 2017 10:18:11 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 9: Code-Review+1
(5 comments)
Nice. So much better. I'll let Alex take a look as well and do the final +2.
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java@292
PS9, Line 292: paresResult
I believe you meant parseResult?
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@125
PS9, Line 125: public int getStartingLineNum() {
: return startLineNum;
: }
single line
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@129
PS9, Line 129: public TQueryOptions getOptions() {
: return this.options;
: }
nit: single line
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@133
PS9, Line 133: public void setOptions(TQueryOptions options) {
: this.options = options;
: }
single line
http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@367
PS9, Line 367:
Add a Preconditions.checkNotNull(result) here.
--
To view, visit http://gerrit.cloudera.org:8080/7564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 9
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Thu, 05 Oct 2017 18:55:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 13 files changed, 744 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/9 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 9 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. (
http://gerrit.cloudera.org:8080/7564 )
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 8:
(3 comments)
Thanks! That looks much better.
http://gerrit.cloudera.org:8080/#/c/7564/8/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
http://gerrit.cloudera.org:8080/#/c/7564/8/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@734
PS8, Line 734: if (options == null) {
: options = defaultQueryOptions();
: } else {
: options = mergeQueryOptions(defaultQueryOptions(),
options);
: }
Move above L732 and make options an input parameter to TestFileParser
constructor.
http://gerrit.cloudera.org:8080/#/c/7564/8/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:
http://gerrit.cloudera.org:8080/#/c/7564/8/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@26
PS8, Line 26: import java.lang.reflect.Method;
Is this needed?
http://gerrit.cloudera.org:8080/#/c/7564/8/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@355
PS8, Line 355: private void parseOneQueryOption(String line, TestCase testCase)
{
Why parsing one query option at a time? ParseQueryOptions() simply expects a
comma-separated list of options and parses all of them. Wouldn't be simpler to
make a JNI call to ParseQueryOptions() instead?
--
To view, visit http://gerrit.cloudera.org:8080/7564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 8
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Wed, 04 Oct 2017 18:09:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 8: > Uploaded patch set 8. Rebased again because of a conflict in hdfs-scan-node-base.cc. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 03 Oct 2017 11:57:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/scan-node.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 13 files changed, 756 insertions(+), 110 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/8 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 7: > Thanks for adding the support for parsing the query options. Did > you try to reuse the c++ implementation through a JNI call? It > would be nice if we could avoid implementing the same functionality > in both c++ and java. See FeSupport.java class for other cases > where we call into the backend from the java side. Thanks. Done. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 03 Oct 2017 10:58:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M be/src/service/fe-support.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 13 files changed, 756 insertions(+), 110 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/7 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 6: Thanks for adding the support for parsing the query options. Did you try to reuse the c++ implementation through a JNI call? It would be nice if we could avoid implementing the same functionality in both c++ and java. See FeSupport.java class for other cases where we call into the backend from the java side. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 28 Sep 2017 17:51:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 6: > Uploaded patch set 6. Rebased against master. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 28 Sep 2017 15:18:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 10 files changed, 721 insertions(+), 110 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/6 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 5: > Uploaded patch set 5. Added support for query options to .test files. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 28 Sep 2017 14:45:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/7564 ) Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java A testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test 11 files changed, 725 insertions(+), 120 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/5 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-Change-Number: 7564 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: > For this options serialization question, we have a C++ > implementation of "set x=y" -> Thrift at impala::SetQueryOption() > (https://github.com/apache/incubator-impala/blob/545eab6d6202ca3968279d14fad28bd2a6d566f6/be/src/service/query-options.cc#L113 > ). I don't know how important it is to keep one implementation of > that, but that one can be made available to us here. Good point. It's not clear how are we going to expose that to the PlannerTests through. Thoughts? -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Philip Zeyliger has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: For this options serialization question, we have a C++ implementation of "set x=y" -> Thrift at impala::SetQueryOption() (https://github.com/apache/incubator-impala/blob/545eab6d6202ca3968279d14fad28bd2a6d566f6/be/src/service/query-options.cc#L113 ). I don't know how important it is to keep one implementation of that, but that one can be made available to us here. -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: > > > > (1 comment) > > > > > > The tests in this change use only 3 query options. Adding a new > > > section to the .test files to support only these 3 options > > wouldn't > > > be too much work. > > > > > > On the other hand, adding support for all the query options > that > > > Impala supports would be a lot harder. Probably we would have > to > > > implement that using some Java reflection trickery. > > > > I don't think you need to use Java reflection. The generated Java > > class for TQueryOptions has a number of helper functions to > search > > and set a field by name. So, for instance the QUERY_OPTIONS > section > > could have key=value pairs that correspond to query options. Then > > we could write a small function that parses the key value pairs > and > > uses the helper functions to check for valid query options and > set > > the values. Do you want to give it a try? Thanks > > Thanks. I'm still confused though how to implement setting enum > query options without reflection. e.g.: > QUERYOPTIONS > COMPRESSION_CODEC=GZIP > > Here we have to know that the type of TQueryOptions.compression_codec > is org.apache.impala.thrift.THdfsCompression, otherwise we cannot > parse "GZIP". Am I missing something? If you detect that this query option refers to a compression_codec, the only thing you need to do is map the string literal "GZIP" to an instance of HdfsCompression (the latter is just an enum). From that you can get the THdfsCompression object that you pass in the setFieldValue() method. You don't need to handle all the query options in one go. You can add the infrastructure needed, i.e. parsing the key=value pairs, validating the key part and then simply add the logic to handle the options we're interesting in for this patch. Once the infrastructure is in place, then adding support for other options should be quite easy, but you don't need to do everything. -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: > > > (1 comment) > > > > The tests in this change use only 3 query options. Adding a new > > section to the .test files to support only these 3 options > wouldn't > > be too much work. > > > > On the other hand, adding support for all the query options that > > Impala supports would be a lot harder. Probably we would have to > > implement that using some Java reflection trickery. > > I don't think you need to use Java reflection. The generated Java > class for TQueryOptions has a number of helper functions to search > and set a field by name. So, for instance the QUERY_OPTIONS section > could have key=value pairs that correspond to query options. Then > we could write a small function that parses the key value pairs and > uses the helper functions to check for valid query options and set > the values. Do you want to give it a try? Thanks Thanks. I'm still confused though how to implement setting enum query options without reflection. e.g.: QUERYOPTIONS COMPRESSION_CODEC=GZIP Here we have to know that the type of TQueryOptions.compression_codec is org.apache.impala.thrift.THdfsCompression, otherwise we cannot parse "GZIP". Am I missing something? -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: > > (1 comment) > > The tests in this change use only 3 query options. Adding a new > section to the .test files to support only these 3 options wouldn't > be too much work. > > On the other hand, adding support for all the query options that > Impala supports would be a lot harder. Probably we would have to > implement that using some Java reflection trickery. I don't think you need to use Java reflection. The generated Java class for TQueryOptions has a number of helper functions to search and set a field by name. So, for instance the QUERY_OPTIONS section could have key=value pairs that correspond to query options. Then we could write a small function that parses the key value pairs and uses the helper functions to check for valid query options and set the values. Do you want to give it a try? Thanks -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 4: > (1 comment) The tests in this change use only 3 query options. Adding a new section to the .test files to support only these 3 options wouldn't be too much work. On the other hand, adding support for all the query options that Impala supports would be a lot harder. Probably we would have to implement that using some Java reflection trickery. -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 2:
(1 comment)
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)
> Let me give you some context:
Can't we expand our testing infrastructure to enable query options to be set on
a per query basis in addition to those set before the call to
runPlannerTestFile()? I am not suggesting supporting SET statements inside the
.test files but we could add another section e.g. QUERY_OPTIONS, that we parse
and use to set query options accordingly. You may want to take a look at
PlannerTestBase.java (testPlan() function) and see how much work it would
require to do something like that.
--
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
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#4). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 666 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/4 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Hello Thomas Tauber-Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7564 to look at the new patch set (#4). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 666 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/4 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
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
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 2:
(4 comments)
Just some answers on the comments. Haven't looked at the new patch yet.
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
> The single node plan is created on L104 and assigned to 'singleNodePlan'. D
Let's expand to the comment to mention that 'singleNodePlan' now points to the
root of the distributed plan.
PS2, Line 117: // create runtime filters
: if (ctx_.getQueryOptions().getRuntime_filter_mode() !=
TRuntimeFilterMode.OFF) {
: RuntimeFilterGenerator.generateRuntimeFilters(ctx_,
singleNodePlan);
: ctx_.getAnalysisResult().getTimeline().markEvent("Runtime
filters computed");
: }
> The block was moved here because the distributed plan is created on L114 an
Thanks for explaining.
http://gerrit.cloudera.org:8080/#/c/7564/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
PS2, Line 165:
tFilterTarget.setIs_bound_by_partition_columns(isBoundByPartitionColumns);
: tFilterTarget.setIs_local_target(isLocalTarget);
> The BE uses these flags to create instances of Coordinator::FilterTarget th
I see. Thanks
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)
> These are written as query tests and not planner tests because the planner
I don't follow. Did you check how the query option is set in
testRuntimeFilterPropagation() in PlannerTest.java?
--
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
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#3). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 665 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/3 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Hello Thomas Tauber-Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7564 to look at the new patch set (#3). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 665 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/3 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
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:
(8 comments)
Thanks for the review.
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
> The comment in L448 (RuntimeFilterGenerator) suggests that this function is
The single node plan is created on L104 and assigned to 'singleNodePlan'.
Distributed plan is created in-place on L114, therefore after L114
'singleNodePlan' refers to the distributed plan. Maybe I should rename
'singleNodePlan' to something else to avoid confusion?
PS2, Line 117: // create runtime filters
: if (ctx_.getQueryOptions().getRuntime_filter_mode() !=
TRuntimeFilterMode.OFF) {
: RuntimeFilterGenerator.generateRuntimeFilters(ctx_,
singleNodePlan);
: ctx_.getAnalysisResult().getTimeline().markEvent("Runtime
filters computed");
: }
> Why was this moved here?
The block was moved here because the distributed plan is created on L114 and I
wanted to assign runtime filters on the distributed plan instead of the single
node plan. Alex suggested this approach. This way we can enforce all the
constraints (described in the commit message) during filter assignment directly.
http://gerrit.cloudera.org:8080/#/c/7564/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
PS2, Line 142: public boolean isBoundByPartitionColumns = false;
: // Indicates if 'node' is in the same fragment as the join
that produces the
: // filter
: public boolean isLocalTarget = false;
> can they change once they are set? If not, make them final.
Done
PS2, Line 165:
tFilterTarget.setIs_bound_by_partition_columns(isBoundByPartitionColumns);
: tFilterTarget.setIs_local_target(isLocalTarget);
> Do you have to send these to the backend? I think they are only used in DCH
The BE uses these flags to create instances of Coordinator::FilterTarget that
are used in coordinator.cc.
PS2, Line 455: Preconditions.checkNotNull(ctx);
: Preconditions.checkNotNull(root);
> Remove, not really doing anything useful here.
Done
PS2, Line 548: Preconditions.checkNotNull(ctx);
: Preconditions.checkNotNull(scanNode);
> remove
Done
PS2, Line 550: Analyzer analyzer = ctx.getRootAnalyzer();
: boolean isSingleNodeExec = ctx.isSingleNodeExec();
: boolean disableRowRuntimeFiltering =
: ctx.getQueryOptions().isDisable_row_runtime_filtering();
: TRuntimeFilterMode runtimeFilterMode =
ctx.getQueryOptions().getRuntime_filter_mode();
> nit: move these lines below L557. No need to get these values if we don't e
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)
> These should be Planner tests (see PlannerTest.java)
These are written as query tests and not planner tests because the planner
tests don't have a good way to 'SET' query options on a per-test case basis.
(different test cases in 'runtime_filters_distrib_pruning.test' and
'runtime_filters_singlenode_pruning.test' use different query options)
--
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
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-3548: Prune runtime filters based on query options in
the FE
..
Patch Set 2:
(8 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
The comment in L448 (RuntimeFilterGenerator) suggests that this function is
operating on the distributed plan but here you're passing the singleNodePlan.
Am I missing something?
PS2, Line 117: // create runtime filters
: if (ctx_.getQueryOptions().getRuntime_filter_mode() !=
TRuntimeFilterMode.OFF) {
: RuntimeFilterGenerator.generateRuntimeFilters(ctx_,
singleNodePlan);
: ctx_.getAnalysisResult().getTimeline().markEvent("Runtime
filters computed");
: }
Why was this moved here?
http://gerrit.cloudera.org:8080/#/c/7564/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:
PS2, Line 142: public boolean isBoundByPartitionColumns = false;
: // Indicates if 'node' is in the same fragment as the join
that produces the
: // filter
: public boolean isLocalTarget = false;
can they change once they are set? If not, make them final.
PS2, Line 165:
tFilterTarget.setIs_bound_by_partition_columns(isBoundByPartitionColumns);
: tFilterTarget.setIs_local_target(isLocalTarget);
Do you have to send these to the backend? I think they are only used in DCHECKs
and don't change the BE logic at all.
PS2, Line 455: Preconditions.checkNotNull(ctx);
: Preconditions.checkNotNull(root);
Remove, not really doing anything useful here.
PS2, Line 548: Preconditions.checkNotNull(ctx);
: Preconditions.checkNotNull(scanNode);
remove
PS2, Line 550: Analyzer analyzer = ctx.getRootAnalyzer();
: boolean isSingleNodeExec = ctx.isSingleNodeExec();
: boolean disableRowRuntimeFiltering =
: ctx.getQueryOptions().isDisable_row_runtime_filtering();
: TRuntimeFilterMode runtimeFilterMode =
ctx.getQueryOptions().getRuntime_filter_mode();
nit: move these lines below L557. No need to get these values if we don't even
have a valid scan node.
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)
These should be Planner tests (see PlannerTest.java)
--
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
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 2: Code-Review+1 -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#2). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 666 insertions(+), 93 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/2 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new patch set (#2). Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 666 insertions(+), 93 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/2 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 1: (6 comments) 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: // compute filters unnecessary PS1, Line 121: ctx_.getAnalysisResult().getTimeline().markEvent( : "Runtime filters computed"); single line? 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 PS1, Line 549: TRuntimeFilterMode runtimeFilterMode = : ctx.getQueryOptions().getRuntime_filter_mode(); single line? PS1, Line 590: : nit: space before ':' 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 the planner tests don't have a good way to 'SET' on a per-test case basis? Its unfortunate and we should fix that (though I'm not saying you need to for this patch). -- 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: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new change for review. http://gerrit.cloudera.org:8080/7564 Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to the single node plan without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. The runtime filter assignment is done on the distributed plan and the above constraints are enforced there directly. Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/coordinator.cc M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 10 files changed, 663 insertions(+), 92 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/7564/1 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has abandoned this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Abandoned It was decided that another approach should be taken to implement this feature. I'm abandoning this change. -- To view, visit http://gerrit.cloudera.org:8080/7096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I422e48847428cab9887aee899fed47a8de95c323 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Attila Jeges has uploaded a new change for review. http://gerrit.cloudera.org:8080/7096 Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. IMPALA-3548: Prune runtime filters based on query options in the FE Currently, the FE generates a number of runtime filters and assigns them to plan nodes without taking the value of RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options into account. The backend then removes filters from exec nodes, based on the following rules: 1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from the exec nodes that are marked as targets not bound by partition columns. 2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from the exec nodes that are marked as remote targets. This may cause some confusion to users because they may see runtime filters in the output of explain that are not applied when the query is executed. This change moves the logic of runtime filter pruning to the planner in the FE. Pruning based on the value of DISABLE_ROW_RUNTIME_FILTERING query option takes place as early as when runtime filters are assigned to the single node plan. Pruning based on RUNTIME_FILTER_MODE query option on the other hand has to wait until the distributed plan is ready (since it requires information about the locality of runtime filter targets). Change-Id: I422e48847428cab9887aee899fed47a8de95c323 --- M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_distrib_pruning.test A testdata/workloads/functional-query/queries/QueryTest/runtime_filters_singlenode_pruning.test M tests/query_test/test_runtime_filters.py 8 files changed, 665 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/7096/2 -- To view, visit http://gerrit.cloudera.org:8080/7096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I422e48847428cab9887aee899fed47a8de95c323 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges
