Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14894 )
Change subject: IMPALA-9009: Core support for Ranger column masking ...................................................................... Patch Set 13: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/14894/13/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java: http://gerrit.cloudera.org:8080/#/c/14894/13/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java@49 PS13, Line 49: // Disable table masking since we don't actually read the data nit: missing . http://gerrit.cloudera.org:8080/#/c/14894/13/fe/src/main/java/org/apache/impala/analysis/FromClause.java File fe/src/main/java/org/apache/impala/analysis/FromClause.java: http://gerrit.cloudera.org:8080/#/c/14894/13/fe/src/main/java/org/apache/impala/analysis/FromClause.java@116 PS13, Line 116: if (origTblRef instanceof InlineViewRef : && ((InlineViewRef) origTblRef).isTableMaskingView()) { : origTblRef = ((InlineViewRef) origTblRef).getUnMaskedTableRef(); : } : if (origTblRef.isResolved() && !(origTblRef instanceof InlineViewRef)) { The new origTblRef will only overwrite the old one in line 130, so it needs the next branch to be taken. It is probably always taken, as origTblRef cannot be InlineViewRef after unmasking, but the correctness of the code is not self evident. I would do something like: bool needsUnmasking = origTblRef instanceof InlineViewRef && ((InlineViewRef) origTblRef).isTableMaskingView(); if (needsUnmasking) origTblRef = ((InlineViewRef) origTblRef).getUnMaskedTableRef(); bool replaceWithUnresolved = origTblRef.isResolved() && !(origTblRef instanceof InlineViewRef) Preconditions.checkState(!needsUnmasking || replaceWithUnresolved); if (replaceWithUnresolved) { ... } -- To view, visit http://gerrit.cloudera.org:8080/14894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4cad60e0e69ea573b7ecfc011b142c46ef52ed61 Gerrit-Change-Number: 14894 Gerrit-PatchSet: 13 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Wed, 08 Jan 2020 13:52:33 +0000 Gerrit-HasComments: Yes
