You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/03/16 03:43:16 UTC
impala git commit: Revert "IMPALA-6479: Update DESCRIBE to respect
column privileges"
Repository: impala
Updated Branches:
refs/heads/2.x 52f08e7c4 -> 7336839db
Revert "IMPALA-6479: Update DESCRIBE to respect column privileges"
This reverts commit 0d95d3a03c3f82313169058950f9b6d926bad859.
Change-Id: I443889250efb3648bd27a845ca3078057af21729
Reviewed-on: http://gerrit.cloudera.org:8080/9667
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/7336839d
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/7336839d
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/7336839d
Branch: refs/heads/2.x
Commit: 7336839dbb2d609005362fdff174a822462f05fb
Parents: 52f08e7
Author: Adam Holley <gi...@holleyism.com>
Authored: Thu Mar 15 15:31:31 2018 -0500
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Mar 16 01:16:56 2018 +0000
----------------------------------------------------------------------
be/src/service/client-request-state.cc | 4 +-
be/src/service/frontend.cc | 9 +-
be/src/service/frontend.h | 5 +-
common/thrift/Frontend.thrift | 3 -
.../impala/analysis/DescribeTableStmt.java | 28 ++-
.../org/apache/impala/analysis/InsertStmt.java | 2 +-
.../java/org/apache/impala/catalog/Column.java | 12 --
.../java/org/apache/impala/catalog/Table.java | 26 ++-
.../impala/service/DescribeResultFactory.java | 33 ++-
.../org/apache/impala/service/Frontend.java | 58 +-----
.../org/apache/impala/service/JniFrontend.java | 3 +-
.../apache/impala/analysis/AuditingTest.java | 4 +-
.../impala/analysis/AuthorizationTest.java | 204 +------------------
13 files changed, 67 insertions(+), 324 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/be/src/service/client-request-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index 655fdb3..12bf3b7 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -361,8 +361,8 @@ Status ClientRequestState::ExecLocalCatalogOp(
}
case TCatalogOpType::DESCRIBE_TABLE: {
TDescribeResult response;
- const TDescribeTableParams& params = catalog_op.describe_table_params;
- RETURN_IF_ERROR(frontend_->DescribeTable(params, query_ctx_.session, &response));
+ RETURN_IF_ERROR(frontend_->DescribeTable(catalog_op.describe_table_params,
+ &response));
// Set the result set
request_result_set_.reset(new vector<TResultRow>(response.results));
return Status::OK();
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/be/src/service/frontend.cc
----------------------------------------------------------------------
diff --git a/be/src/service/frontend.cc b/be/src/service/frontend.cc
index ecfae0a..db54158 100644
--- a/be/src/service/frontend.cc
+++ b/be/src/service/frontend.cc
@@ -124,13 +124,8 @@ Status Frontend::DescribeDb(const TDescribeDbParams& params,
}
Status Frontend::DescribeTable(const TDescribeTableParams& params,
- const TSessionState& session, TDescribeResult* response) {
- TDescribeTableParams tparams;
- tparams.__set_output_style(params.output_style);
- if (params.__isset.table_name) tparams.__set_table_name(params.table_name);
- if (params.__isset.result_struct) tparams.__set_result_struct(params.result_struct);
- tparams.__set_session(session);
- return JniUtil::CallJniMethod(fe_, describe_table_id_, tparams, response);
+ TDescribeResult* response) {
+ return JniUtil::CallJniMethod(fe_, describe_table_id_, params, response);
}
Status Frontend::ShowCreateTable(const TTableName& table_name, string* response) {
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/be/src/service/frontend.h
----------------------------------------------------------------------
diff --git a/be/src/service/frontend.h b/be/src/service/frontend.h
index 9198b83..a152b7f 100644
--- a/be/src/service/frontend.h
+++ b/be/src/service/frontend.h
@@ -125,9 +125,8 @@ class Frontend {
/// field is set to MINIMAL, only the column definitions are returned. If set to
/// FORMATTED|EXTENDED, extended metadata is returned (in addition to the column defs).
/// This includes info about the table properties, SerDe properties, StorageDescriptor
- /// properties, and more. The current user session is needed for privileges checks.
- Status DescribeTable(const TDescribeTableParams& params, const TSessionState& session,
- TDescribeResult* response);
+ /// properties, and more.
+ Status DescribeTable(const TDescribeTableParams& params, TDescribeResult* response);
/// Returns (in the output parameter) a string containing the CREATE TABLE command that
/// creates the table specified in the params.
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/common/thrift/Frontend.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/Frontend.thrift b/common/thrift/Frontend.thrift
index 837aa88..b81c1a1 100644
--- a/common/thrift/Frontend.thrift
+++ b/common/thrift/Frontend.thrift
@@ -166,9 +166,6 @@ struct TDescribeTableParams {
// Set when describing a path to a nested collection.
3: optional Types.TColumnType result_struct
-
- // Session state for the user who initiated this request.
- 4: optional ImpalaInternalService.TSessionState session
}
// Results of a call to describeDb() and describeTable()
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
index 9de5054..90f8b07 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
@@ -23,7 +23,6 @@ import java.util.List;
import org.apache.commons.lang3.StringUtils;
import org.apache.impala.analysis.Path.PathType;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.catalog.StructType;
import org.apache.impala.catalog.Table;
@@ -87,6 +86,21 @@ public class DescribeTableStmt extends StatementBase {
public Table getTable() { return table_; }
public TDescribeOutputStyle getOutputStyle() { return outputStyle_; }
+ /**
+ * Get the privilege requirement, which depends on the output style.
+ */
+ private Privilege getPrivilegeRequirement() {
+ switch (outputStyle_) {
+ case MINIMAL: return Privilege.ANY;
+ case FORMATTED:
+ case EXTENDED:
+ return Privilege.VIEW_METADATA;
+ default:
+ Preconditions.checkArgument(false);
+ return null;
+ }
+ }
+
@Override
public void collectTableRefs(List<TableRef> tblRefs) {
tblRefs.add(new TableRef(rawPath_, null));
@@ -102,10 +116,12 @@ public class DescribeTableStmt extends StatementBase {
// table/database if the user is not authorized.
if (rawPath_.size() > 1) {
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onTable(rawPath_.get(0), rawPath_.get(1)).any().toRequest());
+ .onTable(rawPath_.get(0), rawPath_.get(1))
+ .allOf(getPrivilegeRequirement()).toRequest());
}
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onTable(analyzer.getDefaultDb(), rawPath_.get(0)).any().toRequest());
+ .onTable(analyzer.getDefaultDb(), rawPath_.get(0))
+ .allOf(getPrivilegeRequirement()).toRequest());
throw ae;
} catch (TableLoadingException tle) {
throw new AnalysisException(tle.getMessage(), tle);
@@ -113,15 +129,11 @@ public class DescribeTableStmt extends StatementBase {
table_ = path_.getRootTable();
// Register authorization and audit events.
- analyzer.getTable(table_.getTableName(), Privilege.ANY);
+ analyzer.getTable(table_.getTableName(), getPrivilegeRequirement());
// Describing a table.
if (path_.destTable() != null) return;
- analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onColumn(path_.getRootTable().getDb().getName(), path_.getRootTable().getName(),
- path_.getRawPath().get(0)).any().toRequest());
-
if (path_.destType().isComplexType()) {
if (outputStyle_ == TDescribeOutputStyle.FORMATTED ||
outputStyle_ == TDescribeOutputStyle.EXTENDED) {
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
index 20116c2..4f8d44b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
@@ -724,7 +724,7 @@ public class InsertStmt extends StatementBase {
// Finally, 'undo' the permutation so that the selectListExprs are in Hive column
// order, and add NULL expressions to all missing columns, unless this is an UPSERT.
- List<Column> columns = table_.getColumnsInHiveOrder();
+ ArrayList<Column> columns = table_.getColumnsInHiveOrder();
for (int col = 0; col < columns.size(); ++col) {
Column tblColumn = columns.get(col);
boolean matchFound = false;
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/catalog/Column.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/Column.java b/fe/src/main/java/org/apache/impala/catalog/Column.java
index bec5852..0f4086c 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Column.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Column.java
@@ -17,7 +17,6 @@
package org.apache.impala.catalog;
-import java.util.ArrayList;
import java.util.List;
import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData;
@@ -137,15 +136,4 @@ public class Column {
for (Column col: columns) colNames.add(col.getName());
return colNames;
}
- /**
- * Returns a struct type from the table columns passed in as a parameter.
- */
- public static StructType columnsToStruct(List<Column> columns) {
- ArrayList<StructField> fields = Lists.newArrayListWithCapacity(columns.size());
- for (Column col: columns) {
- fields.add(new StructField(col.getName(), col.getType(), col.getComment()));
- }
- return new StructType(fields);
- }
-
}
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/catalog/Table.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/Table.java b/fe/src/main/java/org/apache/impala/catalog/Table.java
index 9255f0a..aca9409 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Table.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Table.java
@@ -18,7 +18,6 @@
package org.apache.impala.catalog;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -473,30 +472,39 @@ public abstract class Table extends CatalogObjectImpl {
public String getStorageHandlerClassName() { return null; }
/**
- * Returns an unmodifiable list of all columns, but with partition columns at the end of
+ * Returns the list of all columns, but with partition columns at the end of
* the list rather than the beginning. This is equivalent to the order in
* which Hive enumerates columns.
*/
- public List<Column> getColumnsInHiveOrder() {
+ public ArrayList<Column> getColumnsInHiveOrder() {
ArrayList<Column> columns = Lists.newArrayList(getNonClusteringColumns());
columns.addAll(getClusteringColumns());
- return Collections.unmodifiableList(columns);
+ return columns;
}
+ /**
+ * Returns a struct type with the columns in the same order as getColumnsInHiveOrder().
+ */
+ public StructType getHiveColumnsAsStruct() {
+ ArrayList<StructField> fields = Lists.newArrayListWithCapacity(colsByPos_.size());
+ for (Column col: getColumnsInHiveOrder()) {
+ fields.add(new StructField(col.getName(), col.getType(), col.getComment()));
+ }
+ return new StructType(fields);
+ }
/**
- * Returns an unmodifiable list of all partition columns.
+ * Returns the list of all partition columns.
*/
public List<Column> getClusteringColumns() {
- return Collections.unmodifiableList(colsByPos_.subList(0, numClusteringCols_));
+ return colsByPos_.subList(0, numClusteringCols_);
}
/**
- * Returns an unmodifiable list of all columns excluding any partition columns.
+ * Returns the list of all columns excluding any partition columns.
*/
public List<Column> getNonClusteringColumns() {
- return Collections.unmodifiableList(colsByPos_.subList(numClusteringCols_,
- colsByPos_.size()));
+ return colsByPos_.subList(numClusteringCols_, colsByPos_.size());
}
/**
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java b/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
index 9a5adf4..5e90b06 100644
--- a/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
+++ b/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
@@ -17,7 +17,6 @@
package org.apache.impala.service;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -29,6 +28,7 @@ import org.apache.hadoop.hive.ql.metadata.formatting.MetaDataFormatUtils;
import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.KuduColumn;
+import org.apache.impala.catalog.KuduTable;
import org.apache.impala.catalog.StructField;
import org.apache.impala.catalog.StructType;
import org.apache.impala.catalog.Table;
@@ -181,11 +181,9 @@ public class DescribeResultFactory {
* Builds a TDescribeResult that contains the result of a DESCRIBE FORMATTED|EXTENDED
* <table> command. For the formatted describe output the goal is to be exactly the
* same as what Hive (via HiveServer2) outputs, for compatibility reasons. To do this,
- * Hive's MetadataFormatUtils class is used to build the results. filteredColumns is a
- * list of columns the user is authorized to view.
+ * Hive's MetadataFormatUtils class is used to build the results.
*/
- public static TDescribeResult buildDescribeFormattedResult(Table table,
- List<Column> filteredColumns) {
+ public static TDescribeResult buildDescribeFormattedResult(Table table) {
TDescribeResult result = new TDescribeResult();
result.results = Lists.newArrayList();
@@ -194,19 +192,8 @@ public class DescribeResultFactory {
// For some table formats (e.g. Avro) the column list in the table can differ from the
// one returned by the Hive metastore. To handle this we use the column list from the
// table which has already reconciled those differences.
- // Need to create a new list since if the columns are filtered, this will
- // affect the original list.
- List<Column> nonClustered = new ArrayList<Column>();
- List<Column> clustered = new ArrayList<Column>();
- for (Column col: filteredColumns) {
- if (table.isClusteringColumn(col)) {
- clustered.add(col);
- } else {
- nonClustered.add(col);
- }
- }
- msTable.getSd().setCols(Column.toFieldSchemas(nonClustered));
- msTable.setPartitionKeys(Column.toFieldSchemas(clustered));
+ msTable.getSd().setCols(Column.toFieldSchemas(table.getNonClusteringColumns()));
+ msTable.setPartitionKeys(Column.toFieldSchemas(table.getClusteringColumns()));
// To avoid initializing any of the SerDe classes in the metastore table Thrift
// struct, create the ql.metadata.Table object by calling the empty c'tor and
@@ -261,12 +248,16 @@ public class DescribeResultFactory {
}
/**
- * Builds a TDescribeResult for a kudu table from a list of columns.
+ * Builds a TDescribeResult for a table.
*/
- public static TDescribeResult buildKuduDescribeMinimalResult(List<Column> columns) {
+ public static TDescribeResult buildDescribeMinimalResult(Table table) {
+ if (!(table instanceof KuduTable)) {
+ return buildDescribeMinimalResult(table.getHiveColumnsAsStruct());
+ }
+
TDescribeResult descResult = new TDescribeResult();
descResult.results = Lists.newArrayList();
- for (Column c: columns) {
+ for (Column c: table.getColumnsInHiveOrder()) {
Preconditions.checkState(c instanceof KuduColumn);
KuduColumn kuduColumn = (KuduColumn) c;
// General describe info.
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/service/Frontend.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 539fe31..41395d6 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -66,7 +66,6 @@ import org.apache.impala.authorization.AuthorizationChecker;
import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.AuthorizeableTable;
import org.apache.impala.authorization.ImpalaInternalAdminUser;
-import org.apache.impala.authorization.Privilege;
import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.authorization.User;
@@ -787,65 +786,14 @@ public class Frontend {
* the table metadata.
*/
public TDescribeResult describeTable(TTableName tableName,
- TDescribeOutputStyle outputStyle, User user) throws ImpalaException {
+ TDescribeOutputStyle outputStyle) throws ImpalaException {
Table table = impaladCatalog_.get().getTable(tableName.db_name, tableName.table_name);
- List<Column> filteredColumns;
- if (authzConfig_.isEnabled()) {
- // First run a table check
- PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
- .allOf(Privilege.SELECT).onTable(table.getDb().getName(), table.getName())
- .toRequest();
- if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
- // Filter out columns that the user is not authorized to see.
- filteredColumns = new ArrayList<Column>();
- for (Column col: table.getColumnsInHiveOrder()) {
- String colName = col.getName();
- privilegeRequest = new PrivilegeRequestBuilder()
- .allOf(Privilege.SELECT)
- .onColumn(table.getDb().getName(), table.getName(), colName)
- .toRequest();
- if (authzChecker_.get().hasAccess(user, privilegeRequest)) {
- filteredColumns.add(col);
- }
- }
- } else {
- // User has table-level access
- filteredColumns = table.getColumnsInHiveOrder();
- }
- } else {
- // Authorization is disabled
- filteredColumns = table.getColumnsInHiveOrder();
- }
if (outputStyle == TDescribeOutputStyle.MINIMAL) {
- if (!(table instanceof KuduTable)) {
- return DescribeResultFactory.buildDescribeMinimalResult(
- Column.columnsToStruct(filteredColumns));
- }
- return DescribeResultFactory.buildKuduDescribeMinimalResult(filteredColumns);
+ return DescribeResultFactory.buildDescribeMinimalResult(table);
} else {
Preconditions.checkArgument(outputStyle == TDescribeOutputStyle.FORMATTED ||
outputStyle == TDescribeOutputStyle.EXTENDED);
- TDescribeResult result = DescribeResultFactory.buildDescribeFormattedResult(table,
- filteredColumns);
- // Filter out LOCATION text
- if (authzConfig_.isEnabled()) {
- PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
- .allOf(Privilege.VIEW_METADATA)
- .onTable(table.getDb().getName(),table.getName())
- .toRequest();
- // Only filter if the user doesn't have table access.
- if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
- List<TResultRow> results = new ArrayList<TResultRow>();
- for(TResultRow row: result.getResults()) {
- String stringVal = row.getColVals().get(0).getString_val();
- if (!stringVal.contains("Location")) {
- results.add(row);
- }
- }
- result.setResults(results);
- }
- }
- return result;
+ return DescribeResultFactory.buildDescribeFormattedResult(table);
}
}
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/main/java/org/apache/impala/service/JniFrontend.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/JniFrontend.java b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
index 20a028d..3d99a4a 100644
--- a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
+++ b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
@@ -431,10 +431,9 @@ public class JniFrontend {
JniUtil.deserializeThrift(protocolFactory_, params, thriftDescribeTableParams);
Preconditions.checkState(params.isSetTable_name() ^ params.isSetResult_struct());
- User user = new User(TSessionStateUtil.getEffectiveUser(params.getSession()));
TDescribeResult result = null;
if (params.isSetTable_name()) {
- result = frontend_.describeTable(params.getTable_name(), params.output_style, user);
+ result = frontend_.describeTable(params.getTable_name(), params.output_style);
} else {
Preconditions.checkState(params.output_style == TDescribeOutputStyle.MINIMAL);
StructType structType = (StructType)Type.fromThrift(params.result_struct);
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
index cc91188..610bf56 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
@@ -313,7 +313,7 @@ public class AuditingTest extends FrontendTestBase {
accessEvents = AnalyzeAccessEvents("describe formatted functional.alltypesagg");
Assert.assertEquals(accessEvents, Sets.newHashSet(new TAccessEvent(
- "functional.alltypesagg", TCatalogObjectType.TABLE, "ANY")));
+ "functional.alltypesagg", TCatalogObjectType.TABLE, "VIEW_METADATA")));
accessEvents = AnalyzeAccessEvents("describe functional.complex_view");
Assert.assertEquals(accessEvents, Sets.newHashSet(new TAccessEvent(
@@ -453,7 +453,7 @@ public class AuditingTest extends FrontendTestBase {
// Describe formatted
accessEvents = AnalyzeAccessEvents("describe formatted functional_kudu.testtbl");
Assert.assertEquals(accessEvents, Sets.newHashSet(new TAccessEvent(
- "functional_kudu.testtbl", TCatalogObjectType.TABLE, "ANY")));
+ "functional_kudu.testtbl", TCatalogObjectType.TABLE, "VIEW_METADATA")));
}
/**
http://git-wip-us.apache.org/repos/asf/impala/blob/7336839d/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index a3ff4c1..bcea229 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -48,9 +48,6 @@ import org.apache.impala.common.InternalException;
import org.apache.impala.common.RuntimeEnv;
import org.apache.impala.service.Frontend;
import org.apache.impala.testutil.ImpaladTestCatalog;
-import org.apache.impala.thrift.TColumnValue;
-import org.apache.impala.thrift.TDescribeOutputStyle;
-import org.apache.impala.thrift.TDescribeResult;
import org.apache.impala.thrift.TFunctionBinaryType;
import org.apache.impala.thrift.TMetadataOpRequest;
import org.apache.impala.thrift.TMetadataOpcode;
@@ -58,10 +55,8 @@ import org.apache.impala.thrift.TNetworkAddress;
import org.apache.impala.thrift.TPrivilege;
import org.apache.impala.thrift.TPrivilegeLevel;
import org.apache.impala.thrift.TPrivilegeScope;
-import org.apache.impala.thrift.TResultRow;
import org.apache.impala.thrift.TResultSet;
import org.apache.impala.thrift.TSessionState;
-import org.apache.impala.thrift.TTableName;
import org.apache.impala.util.PatternMatcher;
import org.apache.impala.util.SentryPolicyService;
import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
@@ -1464,15 +1459,13 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzOk("describe functional.alltypessmall");
// User has column level privileges on described column.
AuthzOk("describe functional.allcomplextypes.int_struct_col");
- // User has column level privileges on another column in table but not this one.
- AuthzError("describe functional.allcomplextypes.complex_struct_col",
- "User '%s' does not have privileges to access: functional.allcomplextypes");
+ // User has column level privileges on another column in table.
+ AuthzOk("describe functional.allcomplextypes.complex_struct_col");
// User has table level privileges without column level.
AuthzOk("describe functional_parquet.allcomplextypes.complex_struct_col");
- // Column level privileges will allow describe but with reduced data.
- AuthzOk("describe formatted functional.alltypestiny");
- // Column level privileges will allow describe but with reduced data.
- AuthzOk("describe formatted functional.alltypessmall");
+ // Insufficient privileges on table.
+ AuthzError("describe formatted functional.alltypestiny",
+ "User '%s' does not have privileges to access: functional.alltypestiny");
// Insufficient privileges on table for nested column.
AuthzError("describe functional.complextypestbl.nested_struct",
"User '%s' does not have privileges to access: functional.complextypestbl");
@@ -1482,193 +1475,6 @@ public class AuthorizationTest extends FrontendTestBase {
// Insufficient privileges on db.
AuthzError("describe functional_rc.alltypes",
"User '%s' does not have privileges to access: functional_rc.alltypes");
- // Insufficient privileges on column that is a complex type, trying to access member.
- AuthzError("describe functional.allcomplextypes.complex_struct_col.f2",
- "User '%s' does not have privileges to access: functional.allcomplextypes");
- // Insufficient privileges on column that is not a complex type, trying to access member.
- AuthzError("describe functional.allcomplextypes.nested_struct_col.f1",
- "User '%s' does not have privileges to access: functional.allcomplextypes");
- }
-
- // Expected output of DESCRIBE for a functional table.
- private static final List<String> EXPECTED_DESCRIBE_ALLTYPESSMALL =
- Lists.newArrayList(
- "id","int","",
- "int_col","int", "",
- "year","int", ""
- );
-
- // Expected output of DESCRIBE for a functional table.
- private static final List<String> EXPECTED_DESCRIBE_ALLTYPESAGG =
- Lists.newArrayList(
- "id","int","",
- "bool_col","boolean","",
- "tinyint_col","tinyint","",
- "smallint_col","smallint", "",
- "int_col","int", "",
- "bigint_col","bigint", "",
- "float_col","float", "",
- "double_col","double", "",
- "date_string_col","string", "",
- "string_col","string", "",
- "timestamp_col","timestamp", "",
- "year","int", "",
- "month","int", "",
- "day","int", ""
- );
-
- // Expected output of DESCRIBE for a functional table.
- // "*" is used when the output is variable such as time or user.
- private static final List<String> EXPECTED_DESCRIBE_EXTENDED_ALLTYPESAGG =
- Lists.newArrayList(
- "# col_name","data_type","comment",
- "","NULL","NULL",
- "id","int","NULL",
- "bool_col","boolean","NULL",
- "tinyint_col","tinyint","NULL",
- "smallint_col","smallint","NULL",
- "int_col","int","NULL",
- "bigint_col","bigint","NULL",
- "float_col","float","NULL",
- "double_col","double","NULL",
- "date_string_col","string","NULL",
- "string_col","string","NULL",
- "timestamp_col","timestamp","NULL",
- "","NULL","NULL",
- "# Partition Information","NULL","NULL",
- "# col_name","data_type","comment",
- "","NULL","NULL",
- "year","int","NULL",
- "month","int","NULL",
- "day","int","NULL",
- "","NULL","NULL",
- "# Detailed Table Information","NULL","NULL",
- "Database:","functional","NULL",
- "Owner:","*","NULL",
- "CreateTime:","*","NULL",
- "LastAccessTime:","UNKNOWN","NULL",
- "Protect Mode:","None","NULL",
- "Retention:","0","NULL",
- "Location:","hdfs://localhost:20500/test-warehouse/alltypesagg","NULL",
- "Table Type:","EXTERNAL_TABLE","NULL",
- "Table Parameters:","NULL","NULL",
- "","DO_NOT_UPDATE_STATS","true",
- "","EXTERNAL","TRUE",
- "","STATS_GENERATED_VIA_STATS_TASK","true",
- "","numRows","11000",
- "","totalSize","834279",
- "","transient_lastDdlTime","*",
- "","NULL","NULL",
- "# Storage Information","NULL","NULL",
- "SerDe Library:","org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe","NULL",
- "InputFormat:","org.apache.hadoop.mapred.TextInputFormat","NULL",
- "OutputFormat:","org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat","NULL",
- "Compressed:","No","NULL",
- "Num Buckets:","0","NULL",
- "Bucket Columns:","[]","NULL",
- "Sort Columns:","[]","NULL",
- "Storage Desc Params:","NULL","NULL",
- "","escape.delim","\\\\",
- "","field.delim",",",
- "","serialization.format",","
- );
-
- // Expected output of DESCRIBE for a functional table.
- // "*" is used when the output is variable such as time or user.
- private static final List<String> EXPECTED_DESCRIBE_EXTENDED_ALLTYPESSMALL =
- Lists.newArrayList(
- "# col_name","data_type","comment",
- "","NULL","NULL",
- "id","int","NULL",
- "int_col","int","NULL",
- "","NULL","NULL",
- "# Partition Information","NULL","NULL",
- "# col_name","data_type","comment",
- "","NULL","NULL",
- "year","int","NULL",
- "","NULL","NULL",
- "# Detailed Table Information","NULL","NULL",
- "Database:","functional","NULL",
- "Owner:","*","NULL",
- "CreateTime:","*","NULL",
- "LastAccessTime:","UNKNOWN","NULL",
- "Protect Mode:","None","NULL",
- "Retention:","0","NULL",
- "Table Type:","EXTERNAL_TABLE","NULL",
- "Table Parameters:","NULL","NULL",
- "","DO_NOT_UPDATE_STATS","true",
- "","EXTERNAL","TRUE",
- "","STATS_GENERATED_VIA_STATS_TASK","true",
- "","numRows","100",
- "","totalSize","6472",
- "","transient_lastDdlTime","*",
- "","NULL","NULL",
- "# Storage Information","NULL","NULL",
- "SerDe Library:","org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe","NULL",
- "InputFormat:","org.apache.hadoop.mapred.TextInputFormat","NULL",
- "OutputFormat:","org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat","NULL",
- "Compressed:","No","NULL",
- "Num Buckets:","0","NULL",
- "Bucket Columns:","[]","NULL",
- "Sort Columns:","[]","NULL",
- "Storage Desc Params:","NULL","NULL",
- "","escape.delim","\\\\",
- "","field.delim",",",
- "","serialization.format",","
- );
-
- @Test
- public void TestDescribeTableResults() throws ImpalaException {
- // Verify MINIMAL describe contains all columns
- TDescribeResult result = fe_.describeTable(new TTableName("functional","alltypesagg"),
- TDescribeOutputStyle.MINIMAL, USER);
- Assert.assertEquals(EXPECTED_DESCRIBE_ALLTYPESAGG, resultToStringList(result));
-
- // Verify EXTENDED output contains all columns and data
- result = fe_.describeTable(new TTableName("functional","alltypesagg"),
- TDescribeOutputStyle.EXTENDED, USER);
- verifyOutputWithOptionalData(EXPECTED_DESCRIBE_EXTENDED_ALLTYPESAGG,
- resultToStringList(result));
-
- // Verify FORMATTED output contains all columns and data
- result = fe_.describeTable(new TTableName("functional","alltypesagg"),
- TDescribeOutputStyle.FORMATTED, USER);
- verifyOutputWithOptionalData(EXPECTED_DESCRIBE_EXTENDED_ALLTYPESAGG,
- resultToStringList(result));
-
- // Verify MINIMAL describe on restricted table shows limited columns.
- result = fe_.describeTable(new TTableName("functional","alltypessmall"),
- TDescribeOutputStyle.MINIMAL, USER);
- Assert.assertEquals(EXPECTED_DESCRIBE_ALLTYPESSMALL,
- resultToStringList(result));
-
- // Verify FORMATTED output contains all columns and data
- result = fe_.describeTable(new TTableName("functional","alltypessmall"),
- TDescribeOutputStyle.FORMATTED, USER);
- verifyOutputWithOptionalData(EXPECTED_DESCRIBE_EXTENDED_ALLTYPESSMALL,
- resultToStringList(result));
- }
-
- // Compares two arrays but skips an expected value that contains '*' since we need to
- // compare output but some values change based on builds, environments, etc.
- private void verifyOutputWithOptionalData(List<String> expected, List<String> actual) {
- Assert.assertEquals(expected.size(), actual.size());
- for (int idx = 0; idx < expected.size(); idx++) {
- if (!expected.get(idx).equals("*")) {
- Assert.assertEquals(expected.get(idx), actual.get(idx));
- }
- }
- }
-
- // Convert TDescribeResult to ArrayList of strings.
- private static List<String> resultToStringList(TDescribeResult result) {
- List<String> strarr = new ArrayList<String>();
- for (TResultRow row: result.getResults()) {
- for (TColumnValue col: row.getColVals()) {
- strarr.add(col.getString_val() == null ? "NULL": col.getString_val().trim());
- }
- }
- return strarr;
}
@Test