IMPALA-6451: AuthorizationException in CTAS for Kudu tables
CREATE TABLE on EXTERNAL Kudu tables or with TBLPROPERTIES
('kudu.master_addresses') require ALL privileges on SERVER.
See IMPALA-4000.
The patch fixes a bug where the CTAS statement is rewritten
and during the initial analysis phase, 'kudu.master_addresses'
property is added into TBLPROPERTIES which causes the next
analysis phase to assume the statement was executed with
TBLPROPERTIES ('kudu.master_addresses'). Hence, causing the
ALL privilege request on SERVER to be registered.
This patch also refactors the generated Kudu table name into
a single place that stores all generated Kudu properties that
gets cleared when the statement is reset.
Testing:
- Added a new test
- Ran all front-end tests
- Ran Kudu end-to-end query tests
Change-Id: Iac3bbe4dceb80dfefd9756bc322f8c10ceab9f49
Reviewed-on: http://gerrit.cloudera.org:8080/10030
Reviewed-by: Alex Behm <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/b2316be6
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/b2316be6
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/b2316be6
Branch: refs/heads/2.x
Commit: b2316be6c8e672aaa15c51896e5ede053dc94d0e
Parents: b173c53
Author: Fredy Wijaya <[email protected]>
Authored: Wed Apr 11 21:00:50 2018 -0500
Committer: Impala Public Jenkins <[email protected]>
Committed: Wed Apr 18 21:17:47 2018 +0000
----------------------------------------------------------------------
.../apache/impala/analysis/CreateTableStmt.java | 21 ++++++++++----------
.../org/apache/impala/analysis/TableDef.java | 15 +++++++-------
.../org/apache/impala/analysis/ToSqlUtils.java | 19 ++++++++++++++----
.../impala/analysis/AuthorizationTest.java | 11 ++++++++++
4 files changed, 44 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/b2316be6/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
index ee020df..ec689b6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -112,11 +112,11 @@ public class CreateTableStmt extends StatementBase {
Map<String, String> getSerdeProperties() { return
tableDef_.getSerdeProperties(); }
public THdfsFileFormat getFileFormat() { return tableDef_.getFileFormat(); }
RowFormat getRowFormat() { return tableDef_.getRowFormat(); }
- private String getGeneratedKuduTableName() {
- return tableDef_.getGeneratedKuduTableName();
+ private void putGeneratedKuduProperty(String key, String value) {
+ tableDef_.putGeneratedKuduProperty(key, value);
}
- private void setGeneratedKuduTableName(String tableName) {
- tableDef_.setGeneratedKuduTableName(tableName);
+ public Map<String, String> getGeneratedKuduProperties() {
+ return tableDef_.getGeneratedKuduProperties();
}
// Only exposed for ToSqlUtils. Returns the list of primary keys declared by
the user
@@ -166,10 +166,7 @@ public class CreateTableStmt extends StatementBase {
params.setIf_not_exists(getIfNotExists());
params.setSort_columns(getSortColumns());
params.setTable_properties(Maps.newHashMap(getTblProperties()));
- if (!getGeneratedKuduTableName().isEmpty()) {
- params.getTable_properties().put(KuduTable.KEY_TABLE_NAME,
- getGeneratedKuduTableName());
- }
+
params.getTable_properties().putAll(Maps.newHashMap(getGeneratedKuduProperties()));
params.setSerde_properties(getSerdeProperties());
for (KuduPartitionParam d: getKuduPartitionParams()) {
params.addToPartition_by(d.toThrift());
@@ -261,7 +258,8 @@ public class CreateTableStmt extends StatementBase {
throw new AnalysisException("Invalid storage handler specified for Kudu
table: " +
handler);
}
- getTblProperties().put(KuduTable.KEY_STORAGE_HANDLER,
KuduTable.KUDU_STORAGE_HANDLER);
+ putGeneratedKuduProperty(KuduTable.KEY_STORAGE_HANDLER,
+ KuduTable.KUDU_STORAGE_HANDLER);
String masterHosts = getTblProperties().get(KuduTable.KEY_MASTER_HOSTS);
if (Strings.isNullOrEmpty(masterHosts)) {
@@ -271,7 +269,7 @@ public class CreateTableStmt extends StatementBase {
"Table property '%s' is required when the impalad startup flag " +
"-kudu_master_hosts is not used.", KuduTable.KEY_MASTER_HOSTS));
}
- getTblProperties().put(KuduTable.KEY_MASTER_HOSTS, masterHosts);
+ putGeneratedKuduProperty(KuduTable.KEY_MASTER_HOSTS, masterHosts);
}
// TODO: Find out what is creating a directory in HDFS and stop doing
that. Kudu
@@ -357,7 +355,8 @@ public class CreateTableStmt extends StatementBase {
AnalysisUtils.throwIfNotNull(getTblProperties().get(KuduTable.KEY_TABLE_NAME),
String.format("Not allowed to set '%s' manually for managed Kudu
tables .",
KuduTable.KEY_TABLE_NAME));
- setGeneratedKuduTableName(KuduUtil.getDefaultCreateKuduTableName(getDb(),
getTbl()));
+ putGeneratedKuduProperty(KuduTable.KEY_TABLE_NAME,
+ KuduUtil.getDefaultCreateKuduTableName(getDb(), getTbl()));
}
/**
http://git-wip-us.apache.org/repos/asf/impala/blob/b2316be6/fe/src/main/java/org/apache/impala/analysis/TableDef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TableDef.java
b/fe/src/main/java/org/apache/impala/analysis/TableDef.java
index 915bbba..948515b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TableDef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TableDef.java
@@ -17,6 +17,7 @@
package org.apache.impala.analysis;
+import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -88,8 +89,8 @@ class TableDef {
// True if analyze() has been called.
private boolean isAnalyzed_ = false;
- //Kudu table name generated during analysis for managed Kudu tables
- private String generatedKuduTableName_ = "";
+ // Generated Kudu properties set during analysis.
+ private Map<String, String> generatedKuduProperties_ = new HashMap<>();
// END: Members that need to be reset()
/////////////////////////////////////////
@@ -163,7 +164,7 @@ class TableDef {
dataLayout_.reset();
columnDefs_.clear();
isAnalyzed_ = false;
- generatedKuduTableName_ = "";
+ generatedKuduProperties_.clear();
}
public TableName getTblName() {
@@ -187,10 +188,10 @@ class TableDef {
List<ColumnDef> getPrimaryKeyColumnDefs() { return primaryKeyColDefs_; }
boolean isExternal() { return isExternal_; }
boolean getIfNotExists() { return ifNotExists_; }
- String getGeneratedKuduTableName() { return generatedKuduTableName_; }
- void setGeneratedKuduTableName(String tableName) {
- Preconditions.checkNotNull(tableName);
- generatedKuduTableName_ = tableName;
+ Map<String, String> getGeneratedKuduProperties() { return
generatedKuduProperties_; }
+ void putGeneratedKuduProperty(String key, String value) {
+ Preconditions.checkNotNull(key);
+ generatedKuduProperties_.put(key, value);
}
List<KuduPartitionParam> getKuduPartitionParams() {
return dataLayout_.getKuduPartitionParams();
http://git-wip-us.apache.org/repos/asf/impala/blob/b2316be6/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
b/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
index 9a164f0..a3c084d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
@@ -67,8 +67,10 @@ public class ToSqlUtils {
/**
* Removes all hidden properties from the given 'tblProperties' map.
*/
- private static void removeHiddenTableProperties(Map<String, String>
tblProperties) {
+ private static void removeHiddenTableProperties(Map<String, String>
tblProperties,
+ Map<String, String> generatedTblProperties) {
for (String key: HIDDEN_TABLE_PROPERTIES) tblProperties.remove(key);
+ generatedTblProperties.remove(KuduTable.KEY_TABLE_NAME);
}
/**
@@ -165,11 +167,17 @@ public class ToSqlUtils {
for (ColumnDef col: stmt.getPartitionColumnDefs()) {
partitionColsSql.add(col.toString());
}
+ LinkedHashMap<String, String> properties = Maps.newLinkedHashMap(
+ stmt.getTblProperties());
+ LinkedHashMap<String, String> generatedProperties = Maps.newLinkedHashMap(
+ stmt.getGeneratedKuduProperties());
+ removeHiddenTableProperties(properties, generatedProperties);
+ properties.putAll(generatedProperties);
String kuduParamsSql = getKuduPartitionByParams(stmt);
// TODO: Pass the correct compression, if applicable.
return getCreateTableSql(stmt.getDb(), stmt.getTbl(), stmt.getComment(),
colsSql,
partitionColsSql, stmt.getTblPrimaryKeyColumnNames(), kuduParamsSql,
- stmt.getSortColumns(), stmt.getTblProperties(),
stmt.getSerdeProperties(),
+ stmt.getSortColumns(), properties, stmt.getSerdeProperties(),
stmt.isExternal(), stmt.getIfNotExists(), stmt.getRowFormat(),
HdfsFileFormat.fromThrift(stmt.getFileFormat()), HdfsCompression.NONE,
null,
stmt.getLocation());
@@ -190,7 +198,10 @@ public class ToSqlUtils {
// Use a LinkedHashMap to preserve the ordering of the table properties.
LinkedHashMap<String, String> properties =
Maps.newLinkedHashMap(innerStmt.getTblProperties());
- removeHiddenTableProperties(properties);
+ LinkedHashMap<String, String> generatedProperties = Maps.newLinkedHashMap(
+ stmt.getCreateStmt().getGeneratedKuduProperties());
+ removeHiddenTableProperties(properties, generatedProperties);
+ properties.putAll(generatedProperties);
String kuduParamsSql = getKuduPartitionByParams(innerStmt);
// TODO: Pass the correct compression, if applicable.
String createTableSql = getCreateTableSql(innerStmt.getDb(),
innerStmt.getTbl(),
@@ -220,7 +231,7 @@ public class ToSqlUtils {
msTable.getTableType().equals(TableType.EXTERNAL_TABLE.toString());
List<String> sortColsSql = getSortColumns(properties);
String comment = properties.get("comment");
- removeHiddenTableProperties(properties);
+ removeHiddenTableProperties(properties, Maps.<String, String>newHashMap());
ArrayList<String> colsSql = Lists.newArrayList();
ArrayList<String> partitionColsSql = Lists.newArrayList();
boolean isHbaseTable = table instanceof HBaseTable;
http://git-wip-us.apache.org/repos/asf/impala/blob/b2316be6/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 bcea229..db31eaf 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -923,6 +923,17 @@ public class AuthorizationTest extends FrontendTestBase {
AuthzOk("create table tpch.kudu_tbl (i int, j int, primary key (i))" +
" PARTITION BY HASH (i) PARTITIONS 9 stored as kudu");
+ // IMPALA-6451: CTAS for Kudu tables on non-external tables and without
+ // TBLPROPERTIES ('kudu.master_addresses') should not require ALL
privileges
+ // on SERVER.
+ // User has ALL privilege on tpch database and SELECT privilege on
+ // functional.alltypesagg table.
+ // The statement below causes the SQL statement to be rewritten.
+ AuthzOk("create table tpch.kudu_tbl primary key (bigint_col) stored as
kudu as " +
+ "select bigint_col, string_col, current_timestamp() as ins_date " +
+ "from functional.alltypesagg " +
+ "where exists (select 1 from functional.alltypesagg)");
+
// User does not have permission to create table at the specified
location..
AuthzError("create table tpch.new_table (i int) location " +
"'hdfs://localhost:20500/test-warehouse/alltypes'",
