You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2021/03/26 03:16:05 UTC
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17235
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
When creating a StmtMetadataLoader, the 'user' field could be null in
places that don't need to resolve column-masking/row-filtering policies.
E.g. in processing GetColumns HS2 operation, or some FE tests.
This patch skips resolving the table masks in such cases to avoid
NullPointerException.
Tests:
- Run CORE tests and verified no NullPointerException found.
Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
---
M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
2 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/17235/1
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Mar 2021 03:19:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7013/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Mar 2021 06:40:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17235/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17235/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
I think the change looks reasonable but I wanted to confirm a few things:
- Is the null user ok for the GetColumns types operations because on the catalog side we are not enforcing the table masking. i.e it is enforced only for actual query planning.
- The JIRA says NPE was seen in the log file. Did that not fail the unit test ?
- In RangerAuthorizationChecker.java, perhaps we should add Preconditions checkNotNull for user in the relevant places.
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 26 Mar 2021 17:36:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17235
to look at the new patch set (#2).
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
When creating a StmtMetadataLoader, the 'user' field could be null in
places that don't need to resolve column-masking/row-filtering policies.
E.g. in processing GetColumns HS2 operation, or some FE tests.
This patch skips resolving the table masks in such cases to avoid
NullPointerException.
Tests:
- Run CORE tests and verified no NullPointerException found.
Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
---
M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
M fe/src/main/java/org/apache/impala/authorization/TableMask.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
4 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/17235/2
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Mar 2021 12:26:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17235/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17235/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
> Is the null user ok for the GetColumns types operations because on the catalog side we are not enforcing the table masking. i.e it is enforced only for actual query planning.
Yeah, they just get the metadata so won't trigger data masking.
> The JIRA says NPE was seen in the log file. Did that not fail the unit test ?
No, we catch any Exception thrown by collectPolicyTables(). So the NPE was catched and just logged. All the unit tests don't have subquery masking exprs or row filters, so they don't fail.
> In RangerAuthorizationChecker.java, perhaps we should add Preconditions checkNotNull for user in the relevant places.
Sure. Done.
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Mar 2021 01:55:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8452/ : 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/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 27 Mar 2021 02:15:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
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/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
When creating a StmtMetadataLoader, the 'user' field could be null in
places that don't need to resolve column-masking/row-filtering policies.
E.g. in processing GetColumns HS2 operation, or some FE tests.
This patch skips resolving the table masks in such cases to avoid
NullPointerException.
Tests:
- Run CORE tests and verified no NullPointerException found.
Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Reviewed-on: http://gerrit.cloudera.org:8080/17235
Reviewed-by: Aman Sinha <am...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
M fe/src/main/java/org/apache/impala/authorization/TableMask.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
4 files changed, 20 insertions(+), 8 deletions(-)
Approvals:
Aman Sinha: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17235 )
Change subject: IMPALA-10609: Fix NPE in resolving table masks in StmtMetadataLoader
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8448/ : 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/17235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aa20458b02e8a93a871b6dd875decfab82c4eae
Gerrit-Change-Number: 17235
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 26 Mar 2021 03:36:24 +0000
Gerrit-HasComments: No