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 2022/08/08 23:37:09 UTC
[impala] 12/27: IMPALA-11302, IMPALA-11303: Fix error handling of external Iceberg tables
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch branch-4.1.1
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 40ceefa9a3d5878b0fc82ff61649fd6dfe4a52ab
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Tue May 24 14:14:55 2022 +0200
IMPALA-11302, IMPALA-11303: Fix error handling of external Iceberg tables
IMPALA-11302: Improve error message for CREATE EXTERNAL TABLE iceberg
command
The error message complained about failed table loading. The new
error message is more precise, and also notifies the user to use
plain 'CREATE TABLE' for creating new Iceberg tables.
IMPALA-11303: Exception is not raised for Iceberg DDL that misses
LOCATION clause
We returned the error in the result summary. Instead of that now
we raise an error, and also notify the user about how to create
new Iceberg tables.
Testing:
* e2e tests
Change-Id: I659115cc97a1a00e1ddf3fbb7dbe1f286bf1edcf
Reviewed-on: http://gerrit.cloudera.org:8080/18563
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../apache/impala/analysis/CreateTableStmt.java | 29 ++++++++++++++++++++--
.../queries/QueryTest/iceberg-create.test | 9 -------
.../queries/QueryTest/iceberg-negative.test | 17 +++++++++++++
3 files changed, 44 insertions(+), 11 deletions(-)
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 9976b11b9..114bbdb04 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -340,8 +340,7 @@ public class CreateTableStmt extends StatementBase {
}
analyzeKuduTableProperties(analyzer);
- if (isExternal() && !Boolean.parseBoolean(getTblProperties().get(
- Table.TBL_PROP_EXTERNAL_TABLE_PURGE))) {
+ if (isExternalWithNoPurge()) {
// this is an external table
analyzeExternalKuduTableParams(analyzer);
} else {
@@ -736,6 +735,19 @@ public class CreateTableStmt extends StatementBase {
throw new AnalysisException(String.format("%s cannot be set for Iceberg table " +
"stored in hive.catalog", IcebergTable.ICEBERG_CATALOG_LOCATION));
}
+ if (isExternalWithNoPurge()) {
+ String tableId = getTblProperties().get(IcebergTable.ICEBERG_TABLE_IDENTIFIER);
+ if (tableId == null || tableId.isEmpty()) {
+ tableId = getTblProperties().get(Catalogs.NAME);
+ }
+ if (tableId == null || tableId.isEmpty()) {
+ throw new AnalysisException(String.format("Table property '%s' is necessary " +
+ "for external Iceberg tables stored in hive.catalog. " +
+ "For creating a completely new Iceberg table, use 'CREATE TABLE' " +
+ "(no EXTERNAL keyword).",
+ IcebergTable.ICEBERG_TABLE_IDENTIFIER));
+ }
+ }
}
private void validateTableInHadoopCatalog() throws AnalysisException {
@@ -757,6 +769,11 @@ public class CreateTableStmt extends StatementBase {
throw new AnalysisException(String.format("%s cannot be set for Iceberg table " +
"stored in hadoop.tables", IcebergTable.ICEBERG_CATALOG_LOCATION));
}
+ if (isExternalWithNoPurge() && getLocation() == null) {
+ throw new AnalysisException("Set LOCATION for external Iceberg tables " +
+ "stored in hadoop.tables. For creating a completely new Iceberg table, use " +
+ "'CREATE TABLE' (no EXTERNAL keyword).");
+ }
}
private void validateTableInCatalogs() {
@@ -829,4 +846,12 @@ public class CreateTableStmt extends StatementBase {
getColumnDefs().addAll(getPartitionColumnDefs());
getPartitionColumnDefs().clear();
}
+
+ /**
+ * @return true for external tables that don't have "external.table.purge" set to true.
+ */
+ private boolean isExternalWithNoPurge() {
+ return isExternal() && !Boolean.parseBoolean(getTblProperties().get(
+ Table.TBL_PROP_EXTERNAL_TABLE_PURGE));
+ }
}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
index 67ad59fb1..1c0d86d11 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
@@ -66,15 +66,6 @@ DESCRIBE iceberg_hadoop_tables;
STRING,STRING,STRING,STRING
====
---- QUERY
-CREATE EXTERNAL TABLE iceberg_hadoop_tbls_external(
- level STRING
-)
-STORED AS ICEBERG
-TBLPROPERTIES('iceberg.catalog'='hadoop.tables');
----- RESULTS
-'Location is necessary for external iceberg table.'
-====
----- QUERY
CREATE TABLE iceberg_hadoop_tbls_with_loc(
level STRING
)
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
index 0a3555f51..15dc723e3 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
@@ -79,6 +79,23 @@ TBLPROPERTIES('iceberg.catalog'='hadoop.catalog',
AnalysisException: Table property 'iceberg.catalog_location' is necessary for Iceberg table with 'hadoop.catalog'.
====
---- QUERY
+CREATE EXTERNAL TABLE iceberg_hadoop_tbls_external(
+ level STRING
+)
+STORED AS ICEBERG
+TBLPROPERTIES('iceberg.catalog'='hadoop.tables');
+---- CATCH
+Set LOCATION for external Iceberg tables stored in hadoop.tables. For creating a completely new Iceberg table, use 'CREATE TABLE' (no EXTERNAL keyword).
+====
+---- QUERY
+CREATE EXTERNAL TABLE iceberg_hive_tbls_external(
+ level STRING
+)
+STORED AS ICEBERG;
+---- CATCH
+Table property 'iceberg.table_identifier' is necessary for external Iceberg tables stored in hive.catalog. For creating a completely new Iceberg table, use 'CREATE TABLE' (no EXTERNAL keyword).
+====
+---- QUERY
CREATE EXTERNAL TABLE fake_iceberg_table_hadoop_catalog
STORED AS ICEBERG
TBLPROPERTIES('iceberg.catalog'='hadoop.catalog',