You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bo...@apache.org on 2019/08/27 12:19:20 UTC
[impala] 04/04: IMPALA-8889: Fix error messages for unsupported
operations on acid tables
This is an automated email from the ASF dual-hosted git repository.
boroknagyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 3f4cbe961702552a187c0bbc04b616e0029c1412
Author: Yongzhi Chen <yc...@cloudera.com>
AuthorDate: Fri Aug 23 16:17:46 2019 -0400
IMPALA-8889: Fix error messages for unsupported operations on acid tables
Provides up-to-date error messages for acid tables.
Makes minor code change for ensureTableWriteSupported after
HIVEMANAGEDINSERTWRITE is enabled.
Tests:
Fixed and tested AnalyerTest
Fixed acid-negative test
Ran all core tests for Hive 3
Change-Id: I732bf651405c9ed75d1843390050b786720e3ffe
Reviewed-on: http://gerrit.cloudera.org:8080/14133
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
---
.../org/apache/impala/analysis/AlterTableStmt.java | 2 +-
.../java/org/apache/impala/analysis/Analyzer.java | 13 ++++++-------
.../org/apache/impala/analysis/DropStatsStmt.java | 2 +-
.../java/org/apache/impala/analysis/LoadDataStmt.java | 2 +-
.../java/org/apache/impala/analysis/TruncateStmt.java | 2 +-
.../java/org/apache/impala/analysis/AnalyzerTest.java | 19 +++++++------------
.../queries/QueryTest/acid-negative.test | 4 ++--
7 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
index 47f4ca1..79c3b64 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
@@ -91,7 +91,7 @@ public abstract class AlterTableStmt extends StatementBase {
Preconditions.checkState(tableRef instanceof BaseTableRef);
table_ = tableRef.getTable();
analyzer.checkTableCapability(table_, Analyzer.OperationType.WRITE);
- analyzer.ensureTableNotTransactional(table_);
+ analyzer.ensureTableNotTransactional(table_, "ALTER TABLE");
if (table_ instanceof FeDataSourceTable
&& !(this instanceof AlterTableSetColumnStats)) {
throw new AnalysisException(String.format(
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index 75c1fbe..3936722 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -145,8 +145,7 @@ public class Analyzer {
"Table %s not supported. Transactional (ACID) tables are " +
"only supported when they are configured as insert_only.";
private static final String TRANSACTIONAL_TABLE_NOT_SUPPORTED =
- "Table %s not supported. Transactional (ACID) tables are " +
- "only supported for read.";
+ "%s not supported on transactional (ACID) table: %s" ;
private static final String BUCKETED_TABLE_NOT_SUPPORTED =
"%s is a bucketed table. Only read operations are supported on such tables.";
private static final String TABLE_NOT_SUPPORTED =
@@ -236,11 +235,11 @@ public class Analyzer {
table.getFullName());
}
- public static void ensureTableNotTransactional(FeTable table)
+ public static void ensureTableNotTransactional(FeTable table, String operationStr)
throws AnalysisException {
if (AcidUtils.isTransactionalTable(table.getMetaStoreTable().getParameters())) {
throw new AnalysisException(String.format(TRANSACTIONAL_TABLE_NOT_SUPPORTED,
- table.getFullName()));
+ operationStr, table.getFullName()));
}
}
@@ -292,13 +291,13 @@ public class Analyzer {
if (KuduTable.isKuduTable(table.getMetaStoreTable())) return;
if (!MetastoreShim.hasTableCapability(table.getMetaStoreTable(), writeRequires)) {
// Error messages with explanations.
- ensureTableNotTransactional(table);
+ ensureTableNotFullAcid(table);
throw new AnalysisException(String.format(TABLE_NOT_SUPPORTED, "Write",
table.getFullName(),
MetastoreShim.getTableAccessType(table.getMetaStoreTable())));
}
} else {
- ensureTableNotTransactional(table);
+ ensureTableNotTransactional(table, "Write");
}
}
@@ -322,7 +321,7 @@ public class Analyzer {
MetastoreShim.getTableAccessType(table.getMetaStoreTable())));
}
} else {
- ensureTableNotTransactional(table);
+ ensureTableNotTransactional(table, "Operation");
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
index 6ec1868..de91981 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
@@ -103,7 +103,7 @@ public class DropStatsStmt extends StatementBase {
}
tableRef_.analyze(analyzer);
// There is no transactional HMS API to drop stats at the moment (HIVE-22104).
- analyzer.ensureTableNotTransactional(tableRef_.getTable());
+ analyzer.ensureTableNotTransactional(tableRef_.getTable(), "DROP STATS");
if (partitionSet_ != null) {
partitionSet_.setTableName(tableRef_.getTable().getTableName());
partitionSet_.setPrivilegeRequirement(Privilege.ALTER);
diff --git a/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java b/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
index ba3d01a..90ab829 100644
--- a/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
@@ -110,7 +110,7 @@ public class LoadDataStmt extends StatementBase {
dbName_ + "." + getTbl());
}
analyzer.checkTableCapability(table, Analyzer.OperationType.WRITE);
- analyzer.ensureTableNotTransactional(table);
+ analyzer.ensureTableNotTransactional(table, "LOAD DATA");
// Analyze the partition spec, if one was specified.
if (partitionSpec_ != null) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java b/fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
index 15c0a3e..b08b3ca 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
@@ -68,7 +68,7 @@ public class TruncateStmt extends StatementBase {
"TRUNCATE TABLE not supported on non-HDFS table: %s", table_.getFullName()));
}
analyzer.checkTableCapability(table_, Analyzer.OperationType.WRITE);
- analyzer.ensureTableNotTransactional(table_);
+ analyzer.ensureTableNotTransactional(table_, "TRUNCATE TABLE");
}
@Override
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
index 029f28d..4c042fe 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
@@ -540,13 +540,8 @@ public class AnalyzerTest extends FrontendTestBase {
"Table functional_orc_def.full_transactional_table not supported. Transactional (ACID)" +
" tables are only supported when they are configured as insert_only.";
- String insertOnlyErrorMsg =
- "Table functional.insert_only_transactional_table not supported. " +
- "Transactional (ACID) tables are only supported for read.";
-
- String insertOnlyErrorForFullMsg =
- "Table functional_orc_def.full_transactional_table not supported. " +
- "Transactional (ACID) tables are only supported for read.";
+ String insertOnlyErrorMsg = "%s not supported on " +
+ "transactional (ACID) table: functional.insert_only_transactional_table";
AnalysisError(
"create table test as select * from functional_orc_def.full_transactional_table",
@@ -580,14 +575,14 @@ public class AnalyzerTest extends FrontendTestBase {
AnalysisError(
"drop table functional_orc_def.full_transactional_table",
- insertOnlyErrorForFullMsg);
+ errorMsg);
AnalyzesOk("drop table functional.insert_only_transactional_table");
AnalysisError(
"truncate table functional_orc_def.full_transactional_table",
- insertOnlyErrorForFullMsg);
+ errorMsg);
AnalysisError("truncate table functional.insert_only_transactional_table",
- insertOnlyErrorMsg);
+ String.format(insertOnlyErrorMsg, "TRUNCATE TABLE"));
AnalysisError(
"alter table functional_orc_def.full_transactional_table " +
@@ -596,13 +591,13 @@ public class AnalyzerTest extends FrontendTestBase {
AnalysisError(
"alter table functional.insert_only_transactional_table " +
"add columns (col2 string)",
- insertOnlyErrorMsg);
+ String.format(insertOnlyErrorMsg, "ALTER TABLE"));
AnalysisError(
"drop stats functional_orc_def.full_transactional_table",
errorMsg);
AnalysisError("drop stats functional.insert_only_transactional_table",
- insertOnlyErrorMsg);
+ String.format(insertOnlyErrorMsg, "DROP STATS"));
AnalyzesOk("describe functional.insert_only_transactional_table");
AnalyzesOk("describe functional_orc_def.full_transactional_table");
diff --git a/testdata/workloads/functional-query/queries/QueryTest/acid-negative.test b/testdata/workloads/functional-query/queries/QueryTest/acid-negative.test
index 826622b..cfd8416 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/acid-negative.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/acid-negative.test
@@ -2,12 +2,12 @@
---- QUERY
alter table functional.insert_only_transactional_table change column x y bigint;
---- CATCH
-AnalysisException: Table functional.insert_only_transactional_table not supported. Transactional (ACID) tables are only supported for read.
+AnalysisException: ALTER TABLE not supported on transactional (ACID) table: functional.insert_only_transactional_table
====
---- QUERY
drop stats functional.insert_only_transactional_table;
---- CATCH
-AnalysisException: Table functional.insert_only_transactional_table not supported. Transactional (ACID) tables are only supported for read.
+AnalysisException: DROP STATS not supported on transactional (ACID) table: functional.insert_only_transactional_table
====
---- QUERY
select * from functional_orc_def.full_transactional_table;