You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2023/03/02 17:46:59 UTC
[impala] 01/03: IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 8b375a66a29cfea33a4e418b1fa3b9b5139cf907
Author: xiabaike <xi...@163.com>
AuthorDate: Thu Sep 8 09:18:50 2022 +0000
IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table
Impala already supports IF NOT EXISTS in alter table add columns for
general hive table in IMPALA-7832, but not for kudu/iceberg table.
This patch try to add such semantics for kudu/iceberg table.
Testing:
- Updated E2E DDL tests
- Added fe tests
Change-Id: I82590e5372e881f2e81d4ed3dd0d32a2d3ddb517
Reviewed-on: http://gerrit.cloudera.org:8080/18953
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
---
common/thrift/JniCatalog.thrift | 2 +-
.../impala/analysis/AlterTableAddColsStmt.java | 17 +++-
.../apache/impala/service/CatalogOpExecutor.java | 36 ++++++---
.../apache/impala/analysis/AnalyzeKuduDDLTest.java | 5 ++
.../queries/QueryTest/iceberg-alter.test | 57 ++++++++++++++
.../queries/QueryTest/kudu_alter.test | 92 ++++++++++++++++++++++
6 files changed, 194 insertions(+), 15 deletions(-)
diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift
index 3efe0d0e5..c41762a6f 100755
--- a/common/thrift/JniCatalog.thrift
+++ b/common/thrift/JniCatalog.thrift
@@ -222,7 +222,7 @@ struct TAlterTableOrViewRenameParams {
// Parameters for ALTER TABLE ADD COLUMNS commands.
struct TAlterTableAddColsParams {
// List of columns to add to the table
- 1: required list<CatalogObjects.TColumn> columns
+ 1: optional list<CatalogObjects.TColumn> columns
// If true, no error is raised when a column already exists.
2: required bool if_not_exists
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
index e373e9fa3..dece3c4f8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
@@ -32,6 +32,7 @@ import org.apache.impala.thrift.TAlterTableType;
import org.apache.impala.util.KuduUtil;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -74,7 +75,9 @@ public class AlterTableAddColsStmt extends AlterTableStmt {
// are all valid and unique, and that none of the columns conflict with
// partition columns.
Set<String> colNames = new HashSet<>();
- for (ColumnDef c: columnDefs_) {
+ Iterator<ColumnDef> iterator = columnDefs_.iterator();
+ while (iterator.hasNext()){
+ ColumnDef c = iterator.next();
c.analyze(analyzer);
String colName = c.getColName().toLowerCase();
if (existingPartitionKeys.contains(colName)) {
@@ -83,9 +86,15 @@ public class AlterTableAddColsStmt extends AlterTableStmt {
}
Column col = t.getColumn(colName);
- if (col != null && !ifNotExists_) {
- throw new AnalysisException("Column already exists: " + colName);
- } else if (!colNames.add(colName)) {
+ if (col != null) {
+ if (!ifNotExists_) {
+ throw new AnalysisException("Column already exists: " + colName);
+ }
+ // remove the existing column
+ iterator.remove();
+ continue;
+ }
+ if (!colNames.add(colName)) {
throw new AnalysisException("Duplicate column name: " + colName);
}
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index dcf11da26..bc8f1af1a 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1035,10 +1035,16 @@ public class CatalogOpExecutor {
}
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- boolean added = alterTableAddCols(tbl, addColParams.getColumns(),
- addColParams.isIf_not_exists());
- reloadTableSchema = true;
+ boolean added = false;
+ // Columns could be ignored/cleared in AlterTableAddColsStmt,
+ // that may cause columns to be empty.
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ added = alterTableAddCols(tbl, addColParams.getColumns(),
+ addColParams.isIf_not_exists());
+ reloadTableSchema = true;
+ }
if (added) {
responseSummaryMsg = "New column(s) have been added to the table.";
} else {
@@ -1267,9 +1273,14 @@ public class CatalogOpExecutor {
Preconditions.checkState(tbl.isWriteLockedByCurrentThread());
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- KuduCatalogOpExecutor.addColumn(tbl, addColParams.getColumns());
- addSummary(response, "Column(s) have been added.");
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ KuduCatalogOpExecutor.addColumn(tbl, addColParams.getColumns());
+ addSummary(response, "Column(s) have been added.");
+ } else {
+ addSummary(response, "No new column(s) have been added to the table.");
+ }
break;
case REPLACE_COLUMNS:
TAlterTableReplaceColsParams replaceColParams = params.getReplace_cols_params();
@@ -1334,9 +1345,14 @@ public class CatalogOpExecutor {
org.apache.iceberg.Transaction iceTxn = IcebergUtil.getIcebergTransaction(tbl);
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- IcebergCatalogOpExecutor.addColumns(iceTxn, addColParams.getColumns());
- addSummary(response, "Column(s) have been added.");
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ IcebergCatalogOpExecutor.addColumns(iceTxn, addColParams.getColumns());
+ addSummary(response, "Column(s) have been added.");
+ } else {
+ addSummary(response, "No new column(s) have been added to the table.");
+ }
break;
case DROP_COLUMN:
TAlterTableDropColParams dropColParams = params.getDrop_col_params();
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
index 5f90441d7..ed6d8d775 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
@@ -729,6 +729,11 @@ public class AnalyzeKuduDDLTest extends FrontendTestBase {
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int encoding rle)");
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int compression lz4)");
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int block_size 10)");
+ // Test 'if not exists'
+ AnalyzesOk("alter table functional_kudu.testtbl add if not exists columns " +
+ "(name string null)");
+ AnalyzesOk("alter table functional_kudu.testtbl add if not exists columns " +
+ "(a string null, b int, c string null)");
// REPLACE columns is not supported for Kudu tables
AnalysisError("alter table functional_kudu.testtbl replace columns (a int null)",
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
index b653199d0..5d20f8660 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
@@ -436,3 +436,60 @@ DESCRIBE FORMATTED iceberg_upgrade_v2_merge_mode;
---- TYPES
string, string, string
====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a bigint, d bigint)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe ice_alter_cols;
+---- RESULTS
+'d','decimal(22,3)','','true'
+'a','bigint','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
+---- QUERY
+# Add column for the same name, but of a different type
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a string, d string)
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a bigint, b bigint)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+DESCRIBE ice_alter_cols
+---- RESULTS
+'d','decimal(22,3)','','true'
+'a','bigint','','true'
+'b','bigint','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
+---- QUERY
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (c bigint, c string)
+---- CATCH
+AnalysisException: Duplicate column name: c
+====
+---- QUERY
+ALTER TABLE ice_alter_cols DROP COLUMN a;
+ALTER TABLE ice_alter_cols DROP COLUMN b;
+DESCRIBE ice_alter_cols;
+---- RESULTS
+'d','decimal(22,3)','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
\ No newline at end of file
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
index 63e5c6a45..0ed93f69f 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
@@ -347,6 +347,98 @@ alter table tbl_to_alter add columns (invalid_col int not null)
A new non-null column must have a default value
====
---- QUERY
+# Add a column that already exists and a new column that does not exist.
+alter table tbl_to_alter add columns (new_col4 string, new_col5 int)
+---- CATCH
+AnalysisException: Column already exists: new_col4
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+alter table tbl_to_alter add if not exists columns (new_col4 int, new_col5 int)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+alter table tbl_to_alter add if not exists columns (new_col4 string, new_col6 int)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT,INT
+====
+---- QUERY
+# All new columns are existing columns.
+alter table tbl_to_alter add if not exists columns (new_col3 string, new_col4 int);
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Test for duplicated columns.
+alter table tbl_to_alter add if not exists columns (new_col7 string, new_col7 int);
+---- CATCH
+AnalysisException: Duplicate column name: new_col7
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT,INT
+====
+---- QUERY
+# Recover status after add 'new_col5' column
+alter table tbl_to_alter drop column new_col5
+---- RESULTS
+'Column has been dropped.'
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT
+====
+---- QUERY
+# Recover status after add 'new_col6' column
+alter table tbl_to_alter drop column new_col6
+---- RESULTS
+'Column has been dropped.'
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT
+====
+---- QUERY
# Add a column with name reserved by Kudu engine
alter table tbl_to_alter add columns (auto_incrementing_id bigint)
---- CATCH