You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2021/03/29 09:15:07 UTC
[impala] 02/03: IMPALA-10609: Fix NPE in resolving table masks in
StmtMetadataLoader
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit cf463f28c3ff98749cf38093ab1deeab0b070895
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Thu Mar 25 17:32:48 2021 +0800
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>
---
.../java/org/apache/impala/analysis/StmtMetadataLoader.java | 10 ++++++++--
.../main/java/org/apache/impala/authorization/TableMask.java | 10 +++++-----
.../authorization/ranger/RangerAuthorizationChecker.java | 3 +++
fe/src/test/java/org/apache/impala/common/FrontendFixture.java | 5 ++++-
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
index b400f93..fa8ecc4 100644
--- a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
+++ b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
@@ -103,7 +103,8 @@ public class StmtMetadataLoader {
/**
* The 'fe' and 'sessionDb' arguments must be non-null. A null 'timeline' may be passed
- * if no events should be marked.
+ * if no events should be marked. A null 'user' may be passed if don't need to resolve
+ * column-masking/row-filtering policies.
*/
public StmtMetadataLoader(Frontend fe, String sessionDb, EventSequence timeline,
User user) {
@@ -113,6 +114,11 @@ public class StmtMetadataLoader {
user_ = user;
}
+ /**
+ * Constructor used by callers that don't need to resolve column-masking/row-filtering
+ * policies (which may introduce new tables), e.g. MetadataOp to get columns, primary
+ * keys, cross reference of a table.
+ */
public StmtMetadataLoader(Frontend fe, String sessionDb, EventSequence timeline) {
this(fe, sessionDb, timeline, null);
}
@@ -310,7 +316,7 @@ public class StmtMetadataLoader {
}
// Adds tables/views introduced by column-masking/row-filtering policies.
if (fe_.getAuthzFactory().getAuthorizationConfig().isEnabled()
- && fe_.getAuthzFactory().supportsTableMasking()) {
+ && fe_.getAuthzFactory().supportsTableMasking() && user_ != null) {
try {
viewTbls.addAll(collectPolicyTables(tbl));
} catch (Exception e) {
diff --git a/fe/src/main/java/org/apache/impala/authorization/TableMask.java b/fe/src/main/java/org/apache/impala/authorization/TableMask.java
index 44e954b..085577f 100644
--- a/fe/src/main/java/org/apache/impala/authorization/TableMask.java
+++ b/fe/src/main/java/org/apache/impala/authorization/TableMask.java
@@ -49,11 +49,11 @@ public class TableMask {
public TableMask(AuthorizationChecker authzChecker, String dbName, String tableName,
List<Column> requiredColumns, User user) {
- this.authChecker_ = authzChecker;
- this.user_ = user;
- this.dbName_ = dbName;
- this.tableName_ = tableName;
- this.requiredColumns_ = requiredColumns;
+ this.authChecker_ = Preconditions.checkNotNull(authzChecker);
+ this.user_ = Preconditions.checkNotNull(user);
+ this.dbName_ = Preconditions.checkNotNull(dbName);
+ this.tableName_ = Preconditions.checkNotNull(tableName);
+ this.requiredColumns_ = Preconditions.checkNotNull(requiredColumns);
this.requiredColumnNames_ = requiredColumns.stream().map(Column::getName)
.collect(Collectors.toList());
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
index 337ff97..7d66835 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
@@ -440,6 +440,7 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
private RangerAccessResult evalColumnMask(User user, String dbName,
String tableName, String columnName, RangerBufferAuditHandler auditHandler)
throws InternalException {
+ Preconditions.checkNotNull(user);
RangerAccessResourceImpl resource = new RangerImpalaResourceBuilder()
.database(dbName)
.table(tableName)
@@ -461,6 +462,7 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
*/
private RangerAccessResult evalRowFilter(User user, String dbName, String tableName,
RangerBufferAuditHandler auditHandler) throws InternalException {
+ Preconditions.checkNotNull(user);
RangerAccessResourceImpl resource = new RangerImpalaResourceBuilder()
.database(dbName)
.table(tableName)
@@ -487,6 +489,7 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
@Override
public Set<String> getUserGroups(User user) throws InternalException {
+ Preconditions.checkNotNull(user);
UserGroupInformation ugi;
if (RuntimeEnv.INSTANCE.isTestEnv() ||
BackendConfig.INSTANCE.useCustomizedUserGroupsMapperForRanger()) {
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
index 064eb6b..862b782 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
@@ -38,6 +38,7 @@ import org.apache.impala.analysis.StmtMetadataLoader;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
import org.apache.impala.authorization.AuthorizationFactory;
import org.apache.impala.authorization.NoopAuthorizationFactory;
+import org.apache.impala.authorization.User;
import org.apache.impala.catalog.AggregateFunction;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogException;
@@ -63,6 +64,7 @@ import org.apache.impala.util.EventSequence;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import org.apache.impala.util.TSessionStateUtil;
/**
* Test fixture for the front-end as a whole. Logically equivalent to a running
@@ -360,8 +362,9 @@ public class FrontendFixture {
public AnalysisResult parseAndAnalyze(String stmt, AnalysisContext ctx)
throws ImpalaException {
StatementBase parsedStmt = Parser.parse(stmt, ctx.getQueryOptions());
+ User user = new User(TSessionStateUtil.getEffectiveUser(ctx.getQueryCtx().session));
StmtMetadataLoader mdLoader =
- new StmtMetadataLoader(frontend_, ctx.getQueryCtx().session.database, null);
+ new StmtMetadataLoader(frontend_, ctx.getQueryCtx().session.database, null, user);
StmtTableCache stmtTableCache = mdLoader.loadTables(parsedStmt);
return ctx.analyzeAndAuthorize(parsedStmt, stmtTableCache,
frontend_.getAuthzChecker());