You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org> on 2021/03/02 17:32:10 UTC
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15838
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Need to figure out why the following query could not be analyzed
correctly.
with t as (select id, bool_col, int_col, string_col from default.my_alltypestiny)
select id, bool_col, string_col from t;
Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
3 files changed, 145 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/15838/2
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@721
PS2, Line 721: // Fang-Yu: Could consider making the following block a method, e.g., computeResolvedPath().
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@774
PS2, Line 774: // // factory supports column masking. If both of these are false, return the unmasked
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@279
PS2, Line 279: // tblRef.analyze(analyzer) in FromClause#analyze() (inside that for-loop).
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 17:32:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has abandoned this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Abandoned
Abandon this since the fix has been merged.
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@258
PS2, Line 258: List<TableRef> tableRefs = fromClause_.getTableRefs();
: TableRef leftTblRef = null;
: for (int i = 0; i < tableRefs.size(); ++i) {
> This approach may not work since it seems to only process the TableRef's im
The above analysis does not seem accurate. SelectAnalyzer#analyze() is called for each SelectStmt in a query. As long as this is the case, we should not have the problem mentioned above.
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Mar 2021 21:20:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@258
PS2, Line 258: List<TableRef> tableRefs = fromClause_.getTableRefs();
: TableRef leftTblRef = null;
: for (int i = 0; i < tableRefs.size(); ++i) {
This approach may not work since it seems to only process the TableRef's immediately under 'fromClause_'. Those TableRef's involved in subqueries under 'fromClause_' may not be processed.
In this regard, we need to figure out how to properly traverse the SelectStms after "analyzeSelectClause()", i.e., when the columns that need to be materialized have been identified.
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Mar 2021 18:47:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@266
PS2, Line 266: TableMask tableMask = new TableMask(authChecker, table, user);
The constructor of TableMask adds all columns in 'table' to 'requiredColumns', which is unnecessary.
Since SelectAnalyzer#analyzeSelectClause() is completed, we will be able to figure out the name and the type of a column that is needed in the output of the corresponding SelectStmt.
To be more precise, given a SelectListItem, the name of a column could be accessed via its 'expr_.label_'. The type of the column could be accessed via its 'expr_.label_.desc_.type_'.
http://gerrit.cloudera.org:8080/#/c/15838/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@272
PS2, Line 272: createTableMaskView
We should revise createTableMaskView() since the current implementation adds all the columns involved in 'resolvedPath' when creating the InlineViewRef, which is not necessary.
The revised createTableMaskView() should take the SelectList of the SelectStmt as one of the input arguments so that the created InlineViewRef will not contain unnecessary/redundant columns.
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Mar 2021 01:01:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15838 )
Change subject: IMPALA-9661: [WIP] Avoid introducing unused columns in table masking view
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8287/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/15838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc7f15347a50626a9f2aff549bc49338cf73831
Gerrit-Change-Number: 15838
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 17:57:44 +0000
Gerrit-HasComments: No