You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Peter Rozsa (Code Review)" <ge...@cloudera.org> on 2023/06/29 07:45:08 UTC
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Peter Rozsa has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20133
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder expression
set can be calculated. If there's no residual expression, the behaviour
is the same as before, all predicate pushdown is skipped. If there is a
residual expression, an Iceberg expression visitor implementation tries
to locate each Impala predicate in the expression tree that the Iceberg
planning returned as a residual expression. If the visitor's result is
mismatched (the number of expressions does not match the number of
located expressions), then it resorts to the original behaviour, all
predicates are pushed down to the scanners. The visitor implementation
handles IN, NULL and binary expressions, and does not handle compound
expressions created by Impala, and aggregates, startsWith, isNan
expressions by Iceberg.
A new query option is added: iceberg_predicate_subsetting. The query
option's default value is true. It can be turned off by setting it to
false.
Tests:
- unit tests added for ImpalaExpressionVisitor
- planner test cases added for disabled mode
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
A fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 622 insertions(+), 36 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/1
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 371 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/9
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/20133/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20133/6//COMMIT_MSG@56
PS6, Line 56: Performance of the predicate location is measured on two edge cases:
Just curious: In terms of the # of slots being materialised, was this a select * query or did skipping the predicates also reduce the number of materialised slots too?
But anyway, quite impressive numbers!
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@421
PS5, Line 421: return;
nit: curly brackets aren't needed around the return statement.
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@422
PS5, Line 422: }
I'm wondering if putting the rest of the code in this function into another function would increase readability. like trySubsettingPredicatesBeingPushedDown(). There might be a better and shorter name for that function, though.
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@429
PS5, Line 429: for (Expression located : locatedExpressions) {
nit: linebreak not necessary here
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@433
PS5, Line 433: filtering the predicates to be pushed down to Impala scanner.
nit: curly brackets not needed.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1005
PS4, Line 1005: further ro
> This is an example for a query that produces untranslated expression.
what I meant is that instead of comparing for equality, there is n "IS NULL" expression that could be tried out in my opinion.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1025
PS4, Line 1025: ned_position
> The Iceberg expression conversion can't convert binary predicates with NULL
I get that but for future readers this might not be that obvious.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Aug 2023 09:18:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder expression
set can be calculated. If there's no residual expression, the behaviour
is the same as before, all predicate pushdown is skipped. If there is a
residual expression, an Iceberg expression visitor implementation tries
to locate each Impala predicate in the expression tree that the Iceberg
planning returned as a residual expression. If the visitor's result is
mismatched (the number of expressions does not match the number of
located expressions), then it resorts to the original behaviour, all
predicates are pushed down to the scanners. The visitor implementation
handles IN, NULL and binary expressions, and does not handle compound
expressions created by Impala, and aggregates, startsWith, isNan
expressions by Iceberg.
A new query option is added: iceberg_predicate_subsetting. The query
option's default value is true. It can be turned off by setting it to
false.
Tests:
- unit tests added for ImpalaExpressionVisitor
- planner test cases added for disabled mode
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
A fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 622 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/2
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9750/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Sep 2023 07:10:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 4:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@24
PS4, Line 24: residual expression backtracking
This 'residual expression backtracking' sounds pretty mysterious. Might be better this way: 'Iceberg residual expression handling'
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@37
PS4, Line 37: If the expression backtracking
'If Impala is unable to match all residual expression to Impala conjuncts then all the conjunct are pushed dow to Impala scanner' ?
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@53
PS4, Line 53: iceberg_predicate_subsetting
I think it's in fact 'iceberg_predicate_pushdown_subsetting'
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@57
PS4, Line 57: Tests:
Did you have the chance to make some perf measurements when you start increasing the number of predicates so that we can be sure converting Iceberg preds back to Impala preds doesn't get too expensive with scale?
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java
File fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java:
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java@33
PS4, Line 33: PredicateVisitor
From Impala's perspective this class is only a Visitor for Iceberg predicates/expressions, right? IcebergPredicateVisitor would be a more suitable name, then.
But being even more precise, this class is actually a collector of Iceberg predicates (using the visitor pattern). So something like IcebergPredicateCollector or IcebergExpressionCollector would be even more precise in my opinion.
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@118
PS4, Line 118: translatedExpressions_
I think something like 'impalaIcebergPredicateMapping' would be more verbose.
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@375
PS4, Line 375: IcebergUtil.planFiles(getIceTable(),
indentation is off with these 2 lines.
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@418
PS4, Line 418: conjuncts_.removeAll(translatedExpressions_.values());
Might be personal preference, but if you returned here then you wouldn't need to put the rest of the function in an else block.
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@421
PS4, Line 421: return;
Doesn't this fit into the line above?
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@430
PS4, Line 430: if (expr == null) {
Could you please add some comment here for more clarity, like "If we fail to translate any of the Iceberg residual expressions then we skip filtering the predicates to be pushed down to Impala scanner."
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@431
PS4, Line 431: return;
merge into the line above
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@437
PS4, Line 437: expressionsToRetain.forEach(skippedExpressions_::remove);
I think something the other way around would be more readable if feasible. Like skippedExpressions_.stream().filter(some condition) ?
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test@1
PS4, Line 1: # If one or more non-partition predicates are present, then all predicates are pushed down
Please mention in this comment that the pred subsetting feature is off. I know this info is in the file name but you can't emphasise this enough.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test@77
PS4, Line 77: ====
I think this iceberg-predicates.test file is the place where we should test this feature more heavily. e.g. include the use case I mentioned in another test file where some of the residuals can't be matched to Impala predicates so we fallback to push down everything.
Also some further combinations of predicates, and also predicates vs partition transformed table would be nice here. You can go wild and come up with all kind of tricky predicates to see how this feature handle them.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1005
PS4, Line 1005: user = null
user is null ?
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1025
PS4, Line 1025: untranslated
What is an 'untranslated expression' and it's not 100% clear for me why 'action = null' is untranslated.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1035
PS4, Line 1035: ====
Would be nice to have a test case where there are residual expression but not all of them could be matched to an Impala predicate so we backtrack to push down all the predicates to Impala.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 12:06:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13882/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 08:22:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20133
to look at the new patch set (#8).
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 372 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/8
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 369 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/10
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 9:
(2 comments)
Thank you, Gabor!
http://gerrit.cloudera.org:8080/#/c/20133/8/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/20133/8/common/thrift/ImpalaService.thrift@846
PS8, Line 846: the sca
> nit: typo
Done
http://gerrit.cloudera.org:8080/#/c/20133/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@426
PS8, Line 426:
> I'd also push down this logging into the newly introduced function to reduc
Done
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 07:56:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 370 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/11
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder expression
set can be calculated. If there's no residual expression, the behaviour
is the same as before, all predicate pushdown is skipped. If there is a
residual expression, an Iceberg expression visitor implementation tries
to locate each Impala predicate in the expression tree that the Iceberg
planning returned as a residual expression. If the visitor's result is
mismatched (the number of expressions does not match the number of
located expressions), then it resorts to the original behaviour, all
predicates are pushed down to the scanners. The visitor implementation
handles IN, NULL and binary expressions, and does not handle compound
expressions created by Impala, and aggregates, startsWith, isNan
expressions by Iceberg.
A new query option is added: iceberg_predicate_subsetting. The query
option's default value is true. It can be turned off by setting it to
false.
Tests:
- unit tests added for ImpalaExpressionVisitor
- planner test cases added for disabled mode
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
A fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 656 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/3
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 10:42:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 373 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/7
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2023 14:36:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14100/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 08:41:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13432/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Jun 2023 12:05:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13713/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Aug 2023 09:35:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 12
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 09:09:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Reviewed-on: http://gerrit.cloudera.org:8080/20133
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 370 insertions(+), 72 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 13
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9660/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Aug 2023 07:38:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13429/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Jun 2023 08:06:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 355 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/6
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14017/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Sep 2023 09:10:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 11
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 09:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 12
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 14:16:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the residual expression backtracking is improved by
locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If the expression backtracking can't match all residual expressions in
the Impala conjunct list, then the filtering resorts to the original
all-or-nothing approach.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_subsetting. The query
option's default value is true. It can be turned off by setting it to
false.
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 304 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/4
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 5:
(17 comments)
Thank you for the review Gabor!
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@24
PS4, Line 24: Iceberg residual expression hand
> This 'residual expression backtracking' sounds pretty mysterious. Might be
Done
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@37
PS4, Line 37: If Impala is unable to match a
> 'If Impala is unable to match all residual expression to Impala conjuncts t
Done
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@53
PS4, Line 53: is true. It can be turned of
> I think it's in fact 'iceberg_predicate_pushdown_subsetting'
Done
http://gerrit.cloudera.org:8080/#/c/20133/4//COMMIT_MSG@57
PS4, Line 57: - 1000 expression, 999 skipped: on avreage 2 ms
> Did you have the chance to make some perf measurements when you start incre
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java
File fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java:
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/analysis/PredicateVisitor.java@33
PS4, Line 33:
> From Impala's perspective this class is only a Visitor for Iceberg predicat
Renamed to IcebergExpressionCollector
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@118
PS4, Line 118: impalaIcebergPredicate
> I think something like 'impalaIcebergPredicateMapping' would be more verbos
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@375
PS4, Line 375: IcebergUtil.planFiles(getIceTable(),
> indentation is off with these 2 lines.
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@418
PS4, Line 418: conjuncts_.removeAll(impalaIcebergPredicateMapping_.values());
> Might be personal preference, but if you returned here then you wouldn't ne
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@421
PS4, Line 421: if (!analyzer_.getQueryOptions().iceberg_predicate_pushdown_subsetting) { return; }
> Doesn't this fit into the line above?
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@430
PS4, Line 430: If we fail to locate any of the Iceberg residual expressions then we skip
> Could you please add some comment here for more clarity, like "If we fail t
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@431
PS4, Line 431: filtering the predicates to be pushed down to Impala scanner.
> merge into the line above
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@437
PS4, Line 437: skippedExpressions_.addAll(
> I think something the other way around would be more readable if feasible.
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test@1
PS4, Line 1: # If one or more non-partition predicates are present, then all predicates are pushed down
> Please mention in this comment that the pred subsetting feature is off. I k
Done
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test@77
PS4, Line 77: ====
> I think this iceberg-predicates.test file is the place where we should test
Newly added test cases moved to this file
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test:
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1005
PS4, Line 1005: further ro
> user is null ?
This is an example for a query that produces untranslated expression.
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1025
PS4, Line 1025: ned_position
> What is an 'untranslated expression' and it's not 100% clear for me why 'ac
The Iceberg expression conversion can't convert binary predicates with NULL literals: https://github.com/apache/impala/blob/c8422136962b8d08e5f44d8351fb4fe7cdb675b8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java#L678
http://gerrit.cloudera.org:8080/#/c/20133/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1035
PS4, Line 1035: |
> Would be nice to have a test case where there are residual expression but n
Added
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Aug 2023 09:12:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13832/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Aug 2023 12:34:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13430/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Jun 2023 11:24:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 3:
(30 comments)
Thanks for your work, Peter!
http://gerrit.cloudera.org:8080/#/c/20133/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20133/3//COMMIT_MSG@9
PS3, Line 9: This change adds a predicate filtering mechanism at planning time that
For me, this paragraph was a bit difficult to read. I can imagine that someone with no background knowledge on this optimization would find it hard to understand what this patch does based on this explanation. I'd split this into multiple paragraphs for more readability/ eg.:
- what is the current behaviour (all-or-nothing predicate filtering).
- What we mean be residual (preds returned from Iceberg after a planFiles() call). Also emphasize that there is pred pushdown in Iceberg and in Impala too and here we optimize the pushdown in Impala.
- How we want to achieve this (also a bit more structured description would be nice).
- What we gain here.
http://gerrit.cloudera.org:8080/#/c/20133/3//COMMIT_MSG@20
PS3, Line 20: compound
: expressions created by Impala
example pls
http://gerrit.cloudera.org:8080/#/c/20133/3//COMMIT_MSG@19
PS3, Line 19: The visitor implementation
: handles IN, NULL and binary expressions, and does not handle compound
: expressions created by Impala, and aggregates, startsWith, isNan
: expressions by Iceberg.
Is there a reason for not covering these? due to difficulty in implementation? Are there plans to add this support later on?
http://gerrit.cloudera.org:8080/#/c/20133/3//COMMIT_MSG@30
PS3, Line 30: - planner test cases added for disabled mode
I see also coverage for the 'enabled' mode in the existing tests.
http://gerrit.cloudera.org:8080/#/c/20133/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/20133/3/common/thrift/ImpalaService.thrift@800
PS3, Line 800: ICEBERG_PREDICATE_SUBSETTING = 158;
Wondering if ICEBERG_PREDICATE_PUSHDOWN_SUBSETTING would be an even more self-explanatory name. Might be too long, though.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java
File fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java:
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@45
PS3, Line 45: ImpalaExpressionVisitor
this is rather an ImpalaPredicateLocator, isn't it?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@47
PS3, Line 47: OPERATORS
A more verbose name with be nice. Like IMPALA_ICEBERG_OPERATOR_MAP
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@56
PS3, Line 56: expressions
I think impalaExpressions would be a better name as we deal with Iceberg expressions here as well, so would be nice to easily differentiate between them.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@67
PS3, Line 67: if (literal.to(StringType.get()) == literal) {
This is a check if T is a string, right? Can't we simply call literal.toString() for all the cases? Wouldn't that work for String literals?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@86
PS3, Line 86: return result;
this seems weird, but honestly not 100% up to speed with this Iceberg visitor thing :)
Could you add a test to exercise this 'not' operator? I guess we'd need a bool partition column for this.
Giving this a second thought, with this visitor we only check the occurrence of certain expressions within the Iceberg expression tree. So I think I get why we just simply return the intermediate results for a 'not' operation as it won't change what expressions we have seen so far.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@102
PS3, Line 102: public <T> ImpalaExpressionVisitorResult predicate(BoundPredicate<T> pred) {
Could you help me understand what is this BoundPredicate and why it doesn't require such a treatment as the below UnboundPredicate does?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@112
PS3, Line 112: if (!(pred.term() instanceof NamedReference)) {
you could merge L112-114 into one line.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@119
PS3, Line 119: case NOT_NULL:
I personally find the case blocks more readable if surrounded by { } chars. I think there are multiple occurrences of this format in the Impala code.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@220
PS3, Line 220: if (new HashSet<>(literals).containsAll(impalaLiterals)
For performance considerations I'd check first if the sizes equal and then the containsAll() as it's a less expensive operation and with lazy evaluation we could avoid converting to HashSet and checking the content if the sizes mismatch.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@225
PS3, Line 225: case STARTS_WITH:
If these are no-op cases then no need to mention them here with the same block as 'default'. Unless you plan to implement them later on and the presence of these cases are a reminder. But in that case add a TODO pls.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@237
PS3, Line 237: return ImpalaExpressionVisitorResult.EMPTY;
Returning empty result here means that this Iceberg aggregate is not supported for Impala's predicate subsetting, right? Could you add tests that cover these?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@263
PS3, Line 263: allMatched
locatedAllPredicates() ?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/analysis/ImpalaExpressionVisitor.java@264
PS3, Line 264: return icebergPredicateCount_ == locatedExpressions_.size()
Technically this should return true of icebergPredicateCount_ is zero and locatedExpressions_ is empty, shouldn't it?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@121
PS3, Line 121: private List<Expr> skippedImpalaPredicates = new ArrayList<>();
This is a subset of 'impalaIcebergPredicates', right? Could you mention this in the comment?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@122
PS3, Line 122: are cannot be
cannot be
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@125
PS3, Line 125: residualExpressions
According to naming convention please add a trailing underscore to these names.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@408
PS3, Line 408: toRetain
predicatesToRetain?
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@411
PS3, Line 411: ImpalaExpressionVisitor
This name might be misleading as it says ImpalaExpressionVisitor, however, it is a visitor on the Iceberg expression tree if I'm not mistaken.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@414
PS3, Line 414: if (result.allMatched()) {
I think it would make the code simpler if you negated the condition like this:
if (!result.allMatched()) return;
// do stuff here
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@416
PS3, Line 416: toRetain.addAll(locatedExpressions);
This line could be merged with the above
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java
File fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java:
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java@219
PS3, Line 219: public void testCompoundExpression() throws SqlCastException {
Would be nice to have a comment to describe the expression for better understanding the test case. I think this could be beneficial for the other test cases as well.
http://gerrit.cloudera.org:8080/#/c/20133/3/fe/src/test/java/org/apache/impala/analysis/ImpalaExpressionVisitorTest.java@248
PS3, Line 248: }
Just played with the patch and saw something weird:
This skips the predicate on 'year' col.
select * from iceberg_partition_evolution where year=2010;
while this pushes down the predicate to Impala scanner.
select * from iceberg_partition_evolution where year=null;
Do you know why this happens?
http://gerrit.cloudera.org:8080/#/c/20133/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test:
http://gerrit.cloudera.org:8080/#/c/20133/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test@1
PS3, Line 1: ore
typo
http://gerrit.cloudera.org:8080/#/c/20133/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test@3
PS3, Line 3: select * from iceberg_partitioned where action = "download" and user = "Lisa";
Instead of selecting all columns, I think it also add some extra value to project to a subset of the columns. With this we can also test that the row-size changes based on what predicates we push down, assuming that the columns involved in the predicates aren't included in the select list.
http://gerrit.cloudera.org:8080/#/c/20133/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test:
http://gerrit.cloudera.org:8080/#/c/20133/3/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1004
PS3, Line 1004: ====
Is there a test where the conversion between Iceberg residual to Impala predicate fails ('untranslatableImpalaPredicates' not empty) for some reason and Impala decides to subset the predicates being pushed down?
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jul 2023 14:12:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13801/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 09:35:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9772/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 12
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 09:09:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
IMPALA-12089: Be able to skip pushing down a subset of the predicates
This change adds a predicate filtering mechanism at planning time that
locates Impala's predicates in the residual expressions from Iceberg
planning. By locating all residual expressions, the remainder
expression set can be calculated.
The current implementation is an all-or-nothing filter, if 'planFiles()'
(Iceberg API) returns no residual expression, then all Impala
predicates can be skipped, if there's any residual expression, every
Impala predicate is pushed down to the Impala scanner.
Residual expressions are the remaining filter expressions after the
pushdown of predicates into the Iceberg table scan. By locating the
remainder expression, we can reduce the number of predicates that will
be pushed down to the Impala scanner.
After this change, the Iceberg residual expression handling is improved
by locating the simple conjuncts in the residual expression and mapping
back them to Impala conjuncts. For example, if the list of Impala
conjuncts consists of two predicates 'col_i != 100' and 'col_s = "a"'
and 'col_i' happens to be a partition column in the Iceberg table
definition and Iceberg table scan can eliminate the expression, the
residual expression will be 'col_s = "a"'. This expression can be mapped
back as an Impala predicate, and any other expression can be removed
from the effective Impala conjunct list, and pushed down to the scanner,
skipping the unnecessary filtering of 'col_i'.
If there's no residual expression, the behavior is the same as before,
all predicate pushdown is skipped.
If Impala is unable to match all residual expression to Impala conjuncts
then all the conjunct are pushed dow to Impala scanner.
This change offers the advantage of not pushing down already evaluated
filters to the Impala scanner nodes, resulting in enhanced scanning
performance. Additionally, if the filter expression affects columns that
are unnecessary for the final result and can be filtered out during
Iceberg's table scan, it leads to a reduced row size, thereby optimizing
data retrieval and improving overall query efficiency.
This solution is limited to cases where Impala's expression list
contains only conjuncts, compound expressions are not supported, because
partial elimination of compounds would involve expression rewrites in
the Impala expression.
A new query option is added: iceberg_predicate_pushdown_subsetting. The
query option's default value is true. It can be turned off by setting it
to false.
Performance of the predicate location is measured on two edge cases:
- 1000 expression, 999 skipped: on avreage 2 ms
- 1000 expression, 1 skipped: on average 25 ms
Tests:
- planner test cases added for disabled mode
- existing planner test cases adjusted
- core tests passed
Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
A fe/src/main/java/org/apache/impala/analysis/IcebergExpressionCollector.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates-disabled-subsetting.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables-hash-join.test
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
12 files changed, 353 insertions(+), 72 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/20133/5
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@118
PS5, Line 118: private final BiMap<Expression, Expr> impalaIcebergPredicateMapping_ = HashBiMap.create();
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@173
PS5, Line 173: return !impalaIcebergPredicateMapping_.isEmpty() || tblRef_.getTimeTravelSpec() != null;
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Aug 2023 09:13:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13833/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Aug 2023 14:54:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 8: Code-Review+1
(2 comments)
Just some minor nits, looks good in overall.
http://gerrit.cloudera.org:8080/#/c/20133/8/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/20133/8/common/thrift/ImpalaService.thrift@846
PS8, Line 846: the the
nit: typo
http://gerrit.cloudera.org:8080/#/c/20133/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/8/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@426
PS8, Line 426: LOG.debug("Iceberg predicate pushdown subsetting took {} ms",
I'd also push down this logging into the newly introduced function to reduce the noise from this level.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Aug 2023 13:23:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 9: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9660/
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Aug 2023 11:50:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13580/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jul 2023 12:07:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates in Iceberg
......................................................................
Patch Set 10: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9750/
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 10
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Sep 2023 11:22:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12089: Be able to skip pushing down a subset of the predicates
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/20133 )
Change subject: IMPALA-12089: Be able to skip pushing down a subset of the predicates
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@118
PS5, Line 118: private final BiMap<Expression, Expr> impalaIcebergPredicateMapping_ =
> line too long (92 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/20133/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@173
PS5, Line 173: private boolean needIcebergForPlanning() {
> line too long (92 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/20133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I597f69ad03ecaf9e304613ef934654e3d9614ae8
Gerrit-Change-Number: 20133
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 09:09:20 +0000
Gerrit-HasComments: Yes