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 2019/01/24 17:58:16 UTC
[impala] 05/05: IMPALA-7832: Support for IF NOT EXISTS in ALTER
TABLE ADD COLUMN(S)
This is an automated email from the ASF dual-hosted git repository.
mikeb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit bfb9ccc8e02be20fb8b57bae4d55e4094ab7ea3f
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Tue Jan 8 12:45:59 2019 -0800
IMPALA-7832: Support for IF NOT EXISTS in ALTER TABLE ADD COLUMN(S)
This patch adds IF NOT EXISTS support in ALTER TABLE ADD COLUMN and
ALTER TABLE ADD COLUMNS. If IF NOT EXISTS is specified and a column
already exists with this name, no error is thrown. If IF NOT EXISTS
is specified for multiple columns and a column already exists, no
error is thrown and a new column that does not exist will be added.
Syntax:
ALTER TABLE tbl ADD COLUMN [IF NOT EXISTS] i int
ALTER TABLE tbl ADD [IF NOT EXISTS] COLUMNS (i int, j int)
Testing:
- Added new FE tests
- Ran all FE tests
- Updated E2E DDL tests
- Ran all E2E DDL tests
Change-Id: I60ed22c8a8eefa10e94ad3dedf32fe67c16642d9
Reviewed-on: http://gerrit.cloudera.org:8080/12181
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
common/thrift/JniCatalog.thrift | 22 ++-
fe/src/main/cup/sql-parser.cup | 24 +--
...aceColsStmt.java => AlterTableAddColsStmt.java} | 75 ++++-----
...olsStmt.java => AlterTableReplaceColsStmt.java} | 81 +++-------
.../apache/impala/service/CatalogOpExecutor.java | 106 +++++++-----
.../org/apache/impala/analysis/AnalyzeDDLTest.java | 179 ++++++++++++++++++---
.../impala/analysis/AuthorizationStmtTest.java | 1 +
.../org/apache/impala/analysis/ParserTest.java | 20 ++-
.../queries/QueryTest/alter-table.test | 64 ++++++++
.../queries/QueryTest/kudu_alter.test | 4 +-
.../queries/QueryTest/kudu_insert.test | 2 +-
11 files changed, 397 insertions(+), 181 deletions(-)
diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift
index b936773..2d97f43 100644
--- a/common/thrift/JniCatalog.thrift
+++ b/common/thrift/JniCatalog.thrift
@@ -92,7 +92,8 @@ struct TAlterDbParams {
// Types of ALTER TABLE commands supported.
enum TAlterTableType {
- ADD_REPLACE_COLUMNS,
+ ADD_COLUMNS,
+ REPLACE_COLUMNS,
ADD_PARTITION,
ADD_DROP_RANGE_PARTITION,
ALTER_COLUMN,
@@ -207,13 +208,19 @@ struct TAlterTableOrViewRenameParams {
1: required CatalogObjects.TTableName new_table_name
}
-// Parameters for ALTER TABLE ADD|REPLACE COLUMNS commands.
-struct TAlterTableAddReplaceColsParams {
+// Parameters for ALTER TABLE ADD COLUMNS commands.
+struct TAlterTableAddColsParams {
// List of columns to add to the table
1: required list<CatalogObjects.TColumn> columns
- // If true, replace all existing columns. If false add (append) columns to the table.
- 2: required bool replace_existing_cols
+ // If true, no error is raised when a column already exists.
+ 2: required bool if_not_exists
+}
+
+// Parameters for ALTER TABLE REPLACE COLUMNS commands.
+struct TAlterTableReplaceColsParams {
+ // List of columns to replace to the table
+ 1: required list<CatalogObjects.TColumn> columns
}
// Parameters for specifying a single partition in ALTER TABLE ADD PARTITION
@@ -385,7 +392,7 @@ struct TAlterTableParams {
3: optional TAlterTableOrViewRenameParams rename_params
// Parameters for ALTER TABLE ADD COLUMNS
- 4: optional TAlterTableAddReplaceColsParams add_replace_cols_params
+ 4: optional TAlterTableAddColsParams add_cols_params
// Parameters for ALTER TABLE ADD PARTITION
5: optional TAlterTableAddPartitionParams add_partition_params
@@ -422,6 +429,9 @@ struct TAlterTableParams {
// Parameters for ALTER TABLE/VIEW SET OWNER
16: optional TAlterTableOrViewSetOwnerParams set_owner_params
+
+ // Parameters for ALTER TABLE REPLACE COLUMNS
+ 17: optional TAlterTableReplaceColsParams replace_cols_params
}
// Parameters of CREATE TABLE LIKE commands
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index bbd5ccf..051bb48 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -480,7 +480,6 @@ nonterminal THdfsFileFormat file_format_create_table_val;
nonterminal Boolean if_exists_val;
nonterminal Boolean if_not_exists_val;
nonterminal Boolean is_primary_key_val;
-nonterminal Boolean replace_existing_cols_val;
nonterminal HdfsUri location_val;
nonterminal RowFormat row_format_val, opt_row_format_val;
nonterminal String field_terminator_val;
@@ -1095,9 +1094,19 @@ alter_db_stmt ::=
// a partition clause does not make sense for this stmt. If a partition
// is given, manually throw a parse error.
alter_tbl_stmt ::=
- KW_ALTER KW_TABLE table_name:table replace_existing_cols_val:replace KW_COLUMNS
- LPAREN column_def_list:col_defs RPAREN
- {: RESULT = new AlterTableAddReplaceColsStmt(table, col_defs, replace); :}
+ KW_ALTER KW_TABLE table_name:table KW_ADD KW_COLUMN if_not_exists_val:if_not_exists
+ column_def:col_def
+ {:
+ List<ColumnDef> list = new ArrayList<>();
+ list.add(col_def);
+ RESULT = new AlterTableAddColsStmt(table, if_not_exists, list);
+ :}
+ | KW_ALTER KW_TABLE table_name:table KW_ADD if_not_exists_val:if_not_exists KW_COLUMNS
+ LPAREN column_def_list:col_defs RPAREN
+ {: RESULT = new AlterTableAddColsStmt(table, if_not_exists, col_defs); :}
+ | KW_ALTER KW_TABLE table_name:table KW_REPLACE KW_COLUMNS
+ LPAREN column_def_list:col_defs RPAREN
+ {: RESULT = new AlterTableReplaceColsStmt(table, col_defs); :}
| KW_ALTER KW_TABLE table_name:table KW_ADD if_not_exists_val:if_not_exists
partition_def_list:partitions
{: RESULT = new AlterTableAddPartitionStmt(table, if_not_exists, partitions); :}
@@ -1199,13 +1208,6 @@ opt_kw_column ::=
| /* empty */
;
-replace_existing_cols_val ::=
- KW_REPLACE
- {: RESULT = true; :}
- | KW_ADD
- {: RESULT = false; :}
- ;
-
create_db_stmt ::=
KW_CREATE db_or_schema_kw if_not_exists_val:if_not_exists ident_or_default:db_name
opt_comment_val:comment location_val:location
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
similarity index 74%
copy from fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
copy to fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
index 1a506a1..f849537 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
@@ -17,56 +17,38 @@
package org.apache.impala.analysis;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import org.apache.hadoop.hive.metastore.api.FieldSchema;
-
import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.FeHBaseTable;
import org.apache.impala.catalog.FeKuduTable;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.common.AnalysisException;
-import org.apache.impala.thrift.TAlterTableAddReplaceColsParams;
+import org.apache.impala.thrift.TAlterTableAddColsParams;
import org.apache.impala.thrift.TAlterTableParams;
import org.apache.impala.thrift.TAlterTableType;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
/**
- * Represents an ALTER TABLE ADD|REPLACE COLUMNS (colDef1, colDef2, ...) statement.
+ * Represents
+ * - ALTER TABLE ADD [IF NOT EXISTS] COLUMNS (colDef1, colDef2, ...)
+ * - ALTER TABLE ADD COLUMN [IF NOT EXISTS] colDef
+ * statements.
*/
-public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
+public class AlterTableAddColsStmt extends AlterTableStmt {
+ private final boolean ifNotExists_;
private final List<ColumnDef> columnDefs_;
- private final boolean replaceExistingCols_;
- public AlterTableAddReplaceColsStmt(TableName tableName, List<ColumnDef> columnDefs,
- boolean replaceExistingCols) {
+ public AlterTableAddColsStmt(TableName tableName, boolean ifNotExists,
+ List<ColumnDef> columnDefs) {
super(tableName);
+ ifNotExists_ = ifNotExists;
Preconditions.checkState(columnDefs != null && columnDefs.size() > 0);
columnDefs_ = Lists.newArrayList(columnDefs);
- replaceExistingCols_ = replaceExistingCols;
- }
-
- public List<ColumnDef> getColumnDescs() { return columnDefs_; }
-
- // Replace columns instead of appending new columns.
- public boolean getReplaceExistingCols() {
- return replaceExistingCols_;
- }
-
- @Override
- public TAlterTableParams toThrift() {
- TAlterTableParams params = super.toThrift();
- params.setAlter_type(TAlterTableType.ADD_REPLACE_COLUMNS);
- TAlterTableAddReplaceColsParams colParams = new TAlterTableAddReplaceColsParams();
- for (ColumnDef col: getColumnDescs()) {
- colParams.addToColumns(col.toThrift());
- }
- colParams.setReplace_existing_cols(replaceExistingCols_);
- params.setAdd_replace_cols_params(colParams);
- return params;
}
@Override
@@ -76,16 +58,10 @@ public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
// TODO: Support column-level DDL on HBase tables. Requires updating the column
// mappings along with the table columns.
if (t instanceof FeHBaseTable) {
- throw new AnalysisException("ALTER TABLE ADD|REPLACE COLUMNS not currently " +
+ throw new AnalysisException("ALTER TABLE ADD COLUMNS not currently " +
"supported on HBase tables.");
}
- boolean isKuduTable = t instanceof FeKuduTable;
- if (isKuduTable && replaceExistingCols_) {
- throw new AnalysisException("ALTER TABLE REPLACE COLUMNS is not " +
- "supported on Kudu tables.");
- }
-
// Build a set of the partition keys for the table.
Set<String> existingPartitionKeys = new HashSet<>();
for (FieldSchema fs: t.getMetaStoreTable().getPartitionKeys()) {
@@ -105,13 +81,13 @@ public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
}
Column col = t.getColumn(colName);
- if (col != null && !replaceExistingCols_) {
+ if (col != null && !ifNotExists_) {
throw new AnalysisException("Column already exists: " + colName);
} else if (!colNames.add(colName)) {
throw new AnalysisException("Duplicate column name: " + colName);
}
- if (isKuduTable) {
+ if (t instanceof FeKuduTable) {
if (c.getType().isComplexType()) {
throw new AnalysisException("Kudu tables do not support complex types: " +
c.toString());
@@ -130,4 +106,17 @@ public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
}
}
}
+
+ @Override
+ public TAlterTableParams toThrift() {
+ TAlterTableParams params = super.toThrift();
+ params.setAlter_type(TAlterTableType.ADD_COLUMNS);
+ TAlterTableAddColsParams colParams = new TAlterTableAddColsParams();
+ for (ColumnDef col: columnDefs_) {
+ colParams.addToColumns(col.toThrift());
+ }
+ colParams.setIf_not_exists(ifNotExists_);
+ params.setAdd_cols_params(colParams);
+ return params;
+ }
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
similarity index 60%
rename from fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
rename to fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
index 1a506a1..c9c4dd9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
@@ -17,56 +17,32 @@
package org.apache.impala.analysis;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import org.apache.hadoop.hive.metastore.api.FieldSchema;
-
import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.FeHBaseTable;
import org.apache.impala.catalog.FeKuduTable;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.common.AnalysisException;
-import org.apache.impala.thrift.TAlterTableAddReplaceColsParams;
import org.apache.impala.thrift.TAlterTableParams;
+import org.apache.impala.thrift.TAlterTableReplaceColsParams;
import org.apache.impala.thrift.TAlterTableType;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
/**
- * Represents an ALTER TABLE ADD|REPLACE COLUMNS (colDef1, colDef2, ...) statement.
+ * Represents an ALTER TABLE REPLACE COLUMNS (colDef1, colDef2, ...) statement.
*/
-public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
+public class AlterTableReplaceColsStmt extends AlterTableStmt {
private final List<ColumnDef> columnDefs_;
- private final boolean replaceExistingCols_;
- public AlterTableAddReplaceColsStmt(TableName tableName, List<ColumnDef> columnDefs,
- boolean replaceExistingCols) {
+ public AlterTableReplaceColsStmt(TableName tableName, List<ColumnDef> columnDefs) {
super(tableName);
Preconditions.checkState(columnDefs != null && columnDefs.size() > 0);
columnDefs_ = Lists.newArrayList(columnDefs);
- replaceExistingCols_ = replaceExistingCols;
- }
-
- public List<ColumnDef> getColumnDescs() { return columnDefs_; }
-
- // Replace columns instead of appending new columns.
- public boolean getReplaceExistingCols() {
- return replaceExistingCols_;
- }
-
- @Override
- public TAlterTableParams toThrift() {
- TAlterTableParams params = super.toThrift();
- params.setAlter_type(TAlterTableType.ADD_REPLACE_COLUMNS);
- TAlterTableAddReplaceColsParams colParams = new TAlterTableAddReplaceColsParams();
- for (ColumnDef col: getColumnDescs()) {
- colParams.addToColumns(col.toThrift());
- }
- colParams.setReplace_existing_cols(replaceExistingCols_);
- params.setAdd_replace_cols_params(colParams);
- return params;
}
@Override
@@ -76,12 +52,12 @@ public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
// TODO: Support column-level DDL on HBase tables. Requires updating the column
// mappings along with the table columns.
if (t instanceof FeHBaseTable) {
- throw new AnalysisException("ALTER TABLE ADD|REPLACE COLUMNS not currently " +
+ throw new AnalysisException("ALTER TABLE REPLACE COLUMNS not currently " +
"supported on HBase tables.");
}
boolean isKuduTable = t instanceof FeKuduTable;
- if (isKuduTable && replaceExistingCols_) {
+ if (isKuduTable) {
throw new AnalysisException("ALTER TABLE REPLACE COLUMNS is not " +
"supported on Kudu tables.");
}
@@ -104,30 +80,21 @@ public class AlterTableAddReplaceColsStmt extends AlterTableStmt {
"Column name conflicts with existing partition column: " + colName);
}
- Column col = t.getColumn(colName);
- if (col != null && !replaceExistingCols_) {
- throw new AnalysisException("Column already exists: " + colName);
- } else if (!colNames.add(colName)) {
+ if (!colNames.add(colName)) {
throw new AnalysisException("Duplicate column name: " + colName);
}
+ }
+ }
- if (isKuduTable) {
- if (c.getType().isComplexType()) {
- throw new AnalysisException("Kudu tables do not support complex types: " +
- c.toString());
- }
- if (c.isPrimaryKey()) {
- throw new AnalysisException("Cannot add a primary key using an ALTER TABLE " +
- "ADD COLUMNS statement: " + c.toString());
- }
- if (c.isExplicitNotNullable() && !c.hasDefaultValue()) {
- throw new AnalysisException("A new non-null column must have a default " +
- "value: " + c.toString());
- }
- } else if (c.hasKuduOptions()) {
- throw new AnalysisException("The specified column options are only supported " +
- "in Kudu tables: " + c.toString());
- }
+ @Override
+ public TAlterTableParams toThrift() {
+ TAlterTableParams params = super.toThrift();
+ params.setAlter_type(TAlterTableType.REPLACE_COLUMNS);
+ TAlterTableReplaceColsParams colParams = new TAlterTableReplaceColsParams();
+ for (ColumnDef col: columnDefs_) {
+ colParams.addToColumns(col.toThrift());
}
+ params.setReplace_cols_params(colParams);
+ return params;
}
}
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 04ec23d..66f27cd 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -90,14 +90,15 @@ import org.apache.impala.compat.MetastoreShim;
import org.apache.impala.thrift.JniCatalogConstants;
import org.apache.impala.thrift.TAlterDbParams;
import org.apache.impala.thrift.TAlterDbSetOwnerParams;
+import org.apache.impala.thrift.TAlterTableAddColsParams;
import org.apache.impala.thrift.TAlterTableAddDropRangePartitionParams;
import org.apache.impala.thrift.TAlterTableAddPartitionParams;
-import org.apache.impala.thrift.TAlterTableAddReplaceColsParams;
import org.apache.impala.thrift.TAlterTableAlterColParams;
import org.apache.impala.thrift.TAlterTableDropColParams;
import org.apache.impala.thrift.TAlterTableDropPartitionParams;
import org.apache.impala.thrift.TAlterTableOrViewSetOwnerParams;
import org.apache.impala.thrift.TAlterTableParams;
+import org.apache.impala.thrift.TAlterTableReplaceColsParams;
import org.apache.impala.thrift.TAlterTableSetCachedParams;
import org.apache.impala.thrift.TAlterTableSetFileFormatParams;
import org.apache.impala.thrift.TAlterTableSetLocationParams;
@@ -420,18 +421,23 @@ public class CatalogOpExecutor {
return;
}
switch (params.getAlter_type()) {
- case ADD_REPLACE_COLUMNS:
- TAlterTableAddReplaceColsParams addReplaceColParams =
- params.getAdd_replace_cols_params();
- alterTableAddReplaceCols(tbl, addReplaceColParams.getColumns(),
- addReplaceColParams.isReplace_existing_cols());
+ case ADD_COLUMNS:
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ boolean added = alterTableAddCols(tbl, addColParams.getColumns(),
+ addColParams.isIf_not_exists());
reloadTableSchema = true;
- if (addReplaceColParams.isReplace_existing_cols()) {
- addSummary(response, "Table columns have been replaced.");
- } else {
+ if (added) {
addSummary(response, "New column(s) have been added to the table.");
+ } else {
+ addSummary(response, "No new column(s) have been added to the table.");
}
break;
+ case REPLACE_COLUMNS:
+ TAlterTableReplaceColsParams replaceColParams = params.getReplace_cols_params();
+ alterTableReplaceCols(tbl, replaceColParams.getColumns());
+ reloadTableSchema = true;
+ addSummary(response, "Table columns have been replaced.");
+ break;
case ADD_PARTITION:
// Create and add HdfsPartition objects to the corresponding HdfsTable and load
// their block metadata. Get the new table object with an updated catalog
@@ -585,7 +591,8 @@ public class CatalogOpExecutor {
* Kudu in addition to the HMS table.
*/
private boolean altersKuduTable(TAlterTableType type) {
- return type == TAlterTableType.ADD_REPLACE_COLUMNS
+ return type == TAlterTableType.ADD_COLUMNS
+ || type == TAlterTableType.REPLACE_COLUMNS
|| type == TAlterTableType.DROP_COLUMN
|| type == TAlterTableType.ALTER_COLUMN
|| type == TAlterTableType.ADD_DROP_RANGE_PARTITION;
@@ -598,29 +605,31 @@ public class CatalogOpExecutor {
KuduTable tbl, long newCatalogVersion) throws ImpalaException {
Preconditions.checkState(tbl.getLock().isHeldByCurrentThread());
switch (params.getAlter_type()) {
- case ADD_REPLACE_COLUMNS:
- TAlterTableAddReplaceColsParams addReplaceColParams =
- params.getAdd_replace_cols_params();
- KuduCatalogOpExecutor.addColumn((KuduTable) tbl,
- addReplaceColParams.getColumns());
- addSummary(response, "Column has been added/replaced.");
+ case ADD_COLUMNS:
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ KuduCatalogOpExecutor.addColumn(tbl, addColParams.getColumns());
+ addSummary(response, "Column(s) have been added.");
+ break;
+ case REPLACE_COLUMNS:
+ TAlterTableReplaceColsParams replaceColParams = params.getReplace_cols_params();
+ KuduCatalogOpExecutor.addColumn(tbl, replaceColParams.getColumns());
+ addSummary(response, "Column(s) have been replaced.");
break;
case DROP_COLUMN:
TAlterTableDropColParams dropColParams = params.getDrop_col_params();
- KuduCatalogOpExecutor.dropColumn((KuduTable) tbl,
- dropColParams.getCol_name());
+ KuduCatalogOpExecutor.dropColumn(tbl, dropColParams.getCol_name());
addSummary(response, "Column has been dropped.");
break;
case ALTER_COLUMN:
TAlterTableAlterColParams alterColParams = params.getAlter_col_params();
- KuduCatalogOpExecutor.alterColumn((KuduTable) tbl, alterColParams.getCol_name(),
+ KuduCatalogOpExecutor.alterColumn(tbl, alterColParams.getCol_name(),
alterColParams.getNew_col_def());
addSummary(response, "Column has been altered.");
break;
case ADD_DROP_RANGE_PARTITION:
TAlterTableAddDropRangePartitionParams partParams =
params.getAdd_drop_range_partition_params();
- KuduCatalogOpExecutor.addDropRangePartition((KuduTable) tbl, partParams);
+ KuduCatalogOpExecutor.addDropRangePartition(tbl, partParams);
addSummary(response, "Range partition has been " +
(partParams.type == TRangePartitionOperationType.ADD ?
"added." : "dropped."));
@@ -2027,28 +2036,49 @@ public class CatalogOpExecutor {
}
/**
- * Appends one or more columns to the given table, optionally replacing all existing
- * columns.
+ * Appends one or more columns to the given table. Returns true if there a column was
+ * added; false otherwise.
*/
- private void alterTableAddReplaceCols(Table tbl, List<TColumn> columns,
- boolean replaceExistingCols) throws ImpalaException {
+ private boolean alterTableAddCols(Table tbl, List<TColumn> columns, boolean ifNotExists)
+ throws ImpalaException {
Preconditions.checkState(tbl.getLock().isHeldByCurrentThread());
org.apache.hadoop.hive.metastore.api.Table msTbl = tbl.getMetaStoreTable().deepCopy();
- List<FieldSchema> newColumns = buildFieldSchemaList(columns);
- if (replaceExistingCols) {
- msTbl.getSd().setCols(newColumns);
- String sortByKey = AlterTableSortByStmt.TBL_PROP_SORT_COLUMNS;
- if (msTbl.getParameters().containsKey(sortByKey)) {
- String oldColumns = msTbl.getParameters().get(sortByKey);
- String alteredColumns = MetaStoreUtil.intersectCsvListWithColumNames(oldColumns,
- columns);
- msTbl.getParameters().put(sortByKey, alteredColumns);
- }
- } else {
+ List<TColumn> colsToAdd = new ArrayList<>();
+ for (TColumn column: columns) {
+ Column col = tbl.getColumn(column.getColumnName());
+ if (ifNotExists && col != null) continue;
+ if (col != null) {
+ throw new CatalogException(
+ String.format("Column '%s' in table '%s' already exists.",
+ col.getName(), tbl.getName()));
+ }
+ colsToAdd.add(column);
+ }
+ // Only add columns that do not exist.
+ if (!colsToAdd.isEmpty()) {
// Append the new column to the existing list of columns.
- for (FieldSchema fs: buildFieldSchemaList(columns)) {
- msTbl.getSd().addToCols(fs);
- }
+ msTbl.getSd().getCols().addAll(buildFieldSchemaList(colsToAdd));
+ applyAlterTable(msTbl, true);
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Replaces all existing columns to the given table.
+ */
+ private void alterTableReplaceCols(Table tbl, List<TColumn> columns)
+ throws ImpalaException {
+ Preconditions.checkState(tbl.getLock().isHeldByCurrentThread());
+ org.apache.hadoop.hive.metastore.api.Table msTbl = tbl.getMetaStoreTable().deepCopy();
+ List<FieldSchema> newColumns = buildFieldSchemaList(columns);
+ msTbl.getSd().setCols(newColumns);
+ String sortByKey = AlterTableSortByStmt.TBL_PROP_SORT_COLUMNS;
+ if (msTbl.getParameters().containsKey(sortByKey)) {
+ String oldColumns = msTbl.getParameters().get(sortByKey);
+ String alteredColumns = MetaStoreUtil.intersectCsvListWithColumNames(oldColumns,
+ columns);
+ msTbl.getParameters().put(sortByKey, alteredColumns);
}
applyAlterTable(msTbl, true);
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
index b7949ba..7ffa719 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -321,39 +321,104 @@ public class AnalyzeDDLTest extends FrontendTestBase {
}
@Test
- public void TestAlterTableAddReplaceColumns() throws AnalysisException {
+ public void TestAlterTableAddColumn() {
+ AnalyzesOk("alter table functional.alltypes add column new_col int");
+ AnalyzesOk("alter table functional.alltypes add column NEW_COL int");
+ AnalyzesOk("alter table functional.alltypes add column if not exists int_col int");
+ AnalyzesOk("alter table functional.alltypes add column if not exists INT_COL int");
+
+ // Column name must be unique for add.
+ AnalysisError("alter table functional.alltypes add column int_col int",
+ "Column already exists: int_col");
+ AnalysisError("alter table functional.alltypes add column INT_COL int",
+ "Column already exists: int_col");
+ // Add a column with same name as a partition column.
+ AnalysisError("alter table functional.alltypes add column year int",
+ "Column name conflicts with existing partition column: year");
+ AnalysisError("alter table functional.alltypes add column if not exists year int",
+ "Column name conflicts with existing partition column: year");
+ AnalysisError("alter table functional.alltypes add column YEAR int",
+ "Column name conflicts with existing partition column: year");
+ AnalysisError("alter table functional.alltypes add column if not exists YEAR int",
+ "Column name conflicts with existing partition column: year");
+ // Invalid column name.
+ AnalysisError("alter table functional.alltypes add column `???` int",
+ "Invalid column/field name: ???");
+
+ // Table/Db does not exist.
+ AnalysisError("alter table db_does_not_exist.alltypes add column i int",
+ "Could not resolve table reference: 'db_does_not_exist.alltypes'");
+ AnalysisError("alter table functional.table_does_not_exist add column i int",
+ "Could not resolve table reference: 'functional.table_does_not_exist'");
+
+ // Cannot ALTER TABLE a view.
+ AnalysisError("alter table functional.alltypes_view add column c1 string",
+ "ALTER TABLE not allowed on a view: functional.alltypes_view");
+ // Cannot ALTER TABLE a nested collection.
+ AnalysisError("alter table allcomplextypes.int_array_col add column c1 string",
+ createAnalysisCtx("functional"),
+ "ALTER TABLE not allowed on a nested collection: allcomplextypes.int_array_col");
+ // Cannot ALTER TABLE produced by a data source.
+ AnalysisError("alter table functional.alltypes_datasource add column c1 string",
+ "ALTER TABLE not allowed on a table produced by a data source: " +
+ "functional.alltypes_datasource");
+
+ // Cannot ALTER TABLE ADD COLUMNS on an HBase table.
+ AnalysisError("alter table functional_hbase.alltypes add column i int",
+ "ALTER TABLE ADD COLUMNS not currently supported on HBase tables.");
+
+ // Cannot ALTER ADD COLUMN primary key on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add column " +
+ "new_col int primary key",
+ "Cannot add a primary key using an ALTER TABLE ADD COLUMNS statement: " +
+ "new_col INT PRIMARY KEY");
+
+ // A non-null column must have a default on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add column new_col int not null",
+ "A new non-null column must have a default value: new_col INT NOT NULL");
+
+ // Cannot ALTER ADD COLUMN complex type on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add column c struct<f1:int>",
+ "Kudu tables do not support complex types: c STRUCT<f1:INT>");
+
+ // A not null is a Kudu only option..
+ AnalysisError("alter table functional.alltypes add column new_col int not null",
+ "The specified column options are only supported in Kudu tables: " +
+ "new_col INT NOT NULL");
+ }
+
+ @Test
+ public void TestAlterTableAddColumns() {
AnalyzesOk("alter table functional.alltypes add columns (new_col int)");
+ AnalyzesOk("alter table functional.alltypes add columns (NEW_COL int)");
AnalyzesOk("alter table functional.alltypes add columns (c1 string comment 'hi')");
AnalyzesOk("alter table functional.alltypes add columns (c struct<f1:int>)");
- AnalyzesOk(
- "alter table functional.alltypes replace columns (c1 int comment 'c', c2 int)");
- AnalyzesOk("alter table functional.alltypes replace columns (c array<string>)");
+ AnalyzesOk("alter table functional.alltypes add if not exists columns (int_col int)");
+ AnalyzesOk("alter table functional.alltypes add if not exists columns (INT_COL int)");
- // Column name must be unique for add
+ // Column name must be unique for add.
AnalysisError("alter table functional.alltypes add columns (int_col int)",
"Column already exists: int_col");
- // Add a column with same name as a partition column
+ // Add a column with same name as a partition column.
AnalysisError("alter table functional.alltypes add columns (year int)",
"Column name conflicts with existing partition column: year");
+ AnalysisError("alter table functional.alltypes add if not exists columns (year int)",
+ "Column name conflicts with existing partition column: year");
// Invalid column name.
AnalysisError("alter table functional.alltypes add columns (`???` int)",
"Invalid column/field name: ???");
- AnalysisError("alter table functional.alltypes replace columns (`???` int)",
- "Invalid column/field name: ???");
- // Replace should not throw an error if the column already exists
- AnalyzesOk("alter table functional.alltypes replace columns (int_col int)");
- // It is not possible to replace a partition column
- AnalysisError("alter table functional.alltypes replace columns (Year int)",
- "Column name conflicts with existing partition column: year");
-
- // Duplicate column names
+ // Duplicate column names.
AnalysisError("alter table functional.alltypes add columns (c1 int, c1 int)",
"Duplicate column name: c1");
- AnalysisError("alter table functional.alltypes replace columns (c1 int, C1 int)",
+ AnalysisError("alter table functional.alltypes add columns (c1 int, C1 int)",
"Duplicate column name: c1");
+ AnalysisError("alter table functional.alltypes add if not exists columns " +
+ "(c1 int, c1 int)", "Duplicate column name: c1");
+ AnalysisError("alter table functional.alltypes add if not exists columns " +
+ "(c1 int, C1 int)", "Duplicate column name: c1");
- // Table/Db does not exist
+ // Table/Db does not exist.
AnalysisError("alter table db_does_not_exist.alltypes add columns (i int)",
"Could not resolve table reference: 'db_does_not_exist.alltypes'");
AnalysisError("alter table functional.table_does_not_exist add columns (i int)",
@@ -374,9 +439,85 @@ public class AnalyzeDDLTest extends FrontendTestBase {
"ALTER TABLE not allowed on a table produced by a data source: " +
"functional.alltypes_datasource");
- // Cannot ALTER TABLE ADD/REPLACE COLUMNS on an HBase table.
+ // Cannot ALTER TABLE ADD COLUMNS on an HBase table.
AnalysisError("alter table functional_hbase.alltypes add columns (i int)",
- "ALTER TABLE ADD|REPLACE COLUMNS not currently supported on HBase tables.");
+ "ALTER TABLE ADD COLUMNS not currently supported on HBase tables.");
+
+ // Cannot ALTER ADD COLUMNS primary key on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add columns " +
+ "(new_col int primary key)",
+ "Cannot add a primary key using an ALTER TABLE ADD COLUMNS statement: " +
+ "new_col INT PRIMARY KEY");
+
+ // A non-null column must have a default on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add columns" +
+ "(new_col int not null)",
+ "A new non-null column must have a default value: new_col INT NOT NULL");
+
+ // Cannot ALTER ADD COLUMN complex type on Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes add columns (c struct<f1:int>)",
+ "Kudu tables do not support complex types: c STRUCT<f1:INT>");
+
+ // A not null is a Kudu only option..
+ AnalysisError("alter table functional.alltypes add columns(new_col int not null)",
+ "The specified column options are only supported in Kudu tables: " +
+ "new_col INT NOT NULL");
+ }
+
+ @Test
+ public void TestAlterTableReplaceColumns() {
+ AnalyzesOk("alter table functional.alltypes replace columns " +
+ "(c1 int comment 'c', c2 int)");
+ AnalyzesOk("alter table functional.alltypes replace columns " +
+ "(C1 int comment 'c', C2 int)");
+ AnalyzesOk("alter table functional.alltypes replace columns (c array<string>)");
+ // Invalid column name.
+ AnalysisError("alter table functional.alltypes replace columns (`???` int)",
+ "Invalid column/field name: ???");
+
+ // Replace should not throw an error if the column already exists.
+ AnalyzesOk("alter table functional.alltypes replace columns (int_col int)");
+ AnalyzesOk("alter table functional.alltypes replace columns (INT_COL int)");
+ // It is not possible to replace a partition column.
+ AnalysisError("alter table functional.alltypes replace columns (year int)",
+ "Column name conflicts with existing partition column: year");
+ AnalysisError("alter table functional.alltypes replace columns (Year int)",
+ "Column name conflicts with existing partition column: year");
+
+ // Duplicate column names.
+ AnalysisError("alter table functional.alltypes replace columns (c1 int, c1 int)",
+ "Duplicate column name: c1");
+ AnalysisError("alter table functional.alltypes replace columns (c1 int, C1 int)",
+ "Duplicate column name: c1");
+
+ // Table/Db does not exist
+ AnalysisError("alter table db_does_not_exist.alltypes replace columns (i int)",
+ "Could not resolve table reference: 'db_does_not_exist.alltypes'");
+ AnalysisError("alter table functional.table_does_not_exist replace columns (i int)",
+ "Could not resolve table reference: 'functional.table_does_not_exist'");
+
+ // Cannot ALTER TABLE a view.
+ AnalysisError("alter table functional.alltypes_view " +
+ "replace columns (c1 string comment 'hi')",
+ "ALTER TABLE not allowed on a view: functional.alltypes_view");
+ // Cannot ALTER TABLE a nested collection.
+ AnalysisError("alter table allcomplextypes.int_array_col " +
+ "replace columns (c1 string comment 'hi')",
+ createAnalysisCtx("functional"),
+ "ALTER TABLE not allowed on a nested collection: allcomplextypes.int_array_col");
+ // Cannot ALTER TABLE produced by a data source.
+ AnalysisError("alter table functional.alltypes_datasource " +
+ "replace columns (c1 string comment 'hi')",
+ "ALTER TABLE not allowed on a table produced by a data source: " +
+ "functional.alltypes_datasource");
+
+ // Cannot ALTER TABLE REPLACE COLUMNS on an HBase table.
+ AnalysisError("alter table functional_hbase.alltypes replace columns (i int)",
+ "ALTER TABLE REPLACE COLUMNS not currently supported on HBase tables.");
+
+ // Cannot ALTER TABLE REPLACE COLUMNS on an Kudu table.
+ AnalysisError("alter table functional_kudu.alltypes replace columns (i int)",
+ "ALTER TABLE REPLACE COLUMNS is not supported on Kudu tables.");
}
@Test
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index 2d7ada8..24086cc 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -1939,6 +1939,7 @@ public class AuthorizationStmtTest extends FrontendTestBase {
@Test
public void testAlterTable() throws ImpalaException {
for (AuthzTest test: new AuthzTest[]{
+ authorize("alter table functional.alltypes add column c1 int"),
authorize("alter table functional.alltypes add columns(c1 int)"),
authorize("alter table functional.alltypes replace columns(c1 int)"),
authorize("alter table functional.alltypes change int_col c1 int"),
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index 0704188..2810890 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -2123,8 +2123,23 @@ public class ParserTest extends FrontendTestBase {
}
@Test
+ public void TestAlterTableAddColumn() {
+ for (String keyword: new String[]{"", "IF NOT EXISTS"}) {
+ ParsesOk(String.format("ALTER TABLE Foo ADD COLUMN %s i int", keyword));
+ ParsesOk(String.format("ALTER TABLE TestDb.Foo ADD COLUMN %s i int", keyword));
+
+ ParserError(String.format("ALTER TestDb.Foo ADD COLUMN %s", keyword));
+ ParserError(String.format("ALTER Foo ADD COLUMN %s", keyword));
+ ParserError(String.format("ALTER TABLE TestDb.Foo ADD COLUMN %s (i int)", keyword));
+ ParserError(String.format("ALTER TABLE Foo ADD COLUMN %s (i int)", keyword));
+ ParserError(String.format("ALTER Foo %s ADD COLUMN i int", keyword));
+ ParserError(String.format("ALTER TestDb.Foo %s ADD COLUMN i int", keyword));
+ }
+ }
+
+ @Test
public void TestAlterTableAddReplaceColumns() {
- String[] addReplaceKw = {"ADD", "REPLACE"};
+ String[] addReplaceKw = {"ADD", "ADD IF NOT EXISTS", "REPLACE"};
for (String addReplace: addReplaceKw) {
ParsesOk(String.format(
"ALTER TABLE Foo %s COLUMNS (i int, s string)", addReplace));
@@ -2151,9 +2166,6 @@ public class ParserTest extends FrontendTestBase {
ParserError(String.format("ALTER TestDb.Foo %s COLUMNS ()", addReplace));
ParserError(String.format("ALTER Foo %s COLUMNS (i int, s string)", addReplace));
ParserError(String.format("ALTER TABLE %s COLUMNS (i int, s string)", addReplace));
- // Don't yet support ALTER TABLE ADD COLUMN syntax
- ParserError(String.format("ALTER TABLE Foo %s COLUMN i int", addReplace));
- ParserError(String.format("ALTER TABLE Foo %s COLUMN (i int)", addReplace));
}
}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/alter-table.test b/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
index e6332d6..5d9e6cf 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
@@ -22,11 +22,65 @@ alter table t1 add columns (t tinyint, s string comment 'Str Col')
string
====
---- QUERY
+# Add columns that already exist with "if not exists" clause.
+alter table t1 add if not exists columns (t tinyint, s string comment 'Str Col')
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add columns that do not exist with "if not exists" clause.
+alter table t1 add if not exists columns (t2 tinyint, s2 string comment 'Str Col')
+---- RESULTS
+'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 t1 add if not exists columns (t3 tinyint, s2 string comment 'Str Col')
+---- RESULTS
+'New column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add a new column that does not exist.
+alter table t1 add column t4 tinyint
+---- RESULTS
+'New column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add a new column that does not exist with "if not exists" clause.
+alter table t1 add column if not exists t5 tinyint
+---- RESULTS
+'New column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add a new column that already exists with "if not exists" clause.
+alter table t1 add column if not exists t5 tinyint
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
describe t1
---- RESULTS
'i','int',''
't','tinyint',''
's','string','Str Col'
+'t2','tinyint',''
+'s2','string','Str Col'
+'t3','tinyint',''
+'t4','tinyint',''
+'t5','tinyint',''
---- TYPES
string,string,string
====
@@ -78,6 +132,11 @@ describe t2
'i','int',''
't','tinyint',''
's','string','Str Col'
+'t2','tinyint',''
+'s2','string','Str Col'
+'t3','tinyint',''
+'t4','tinyint',''
+'t5','tinyint',''
---- TYPES
string,string,string
====
@@ -92,6 +151,11 @@ describe t2
---- RESULTS
'i','int',''
's','string','Str Col'
+'t2','tinyint',''
+'s2','string','Str Col'
+'t3','tinyint',''
+'t4','tinyint',''
+'t5','tinyint',''
---- TYPES
string,string,string
====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
index a0d95d9..0bed123 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
@@ -219,7 +219,7 @@ alter table tbl_to_alter add range partition 1 < values <= 20;
alter table tbl_to_alter add columns (new_col1 int not null default 10,
new_col2 bigint not null default 1000)
---- RESULTS
-'Column has been added/replaced.'
+'Column(s) have been added.'
====
---- QUERY
# Verify partition layout
@@ -272,7 +272,7 @@ INT,STRING,BIGINT,INT,BIGINT
# Add nullable columns: with and without a default
alter table tbl_to_alter add columns (new_col3 string null, new_col4 int null default -1)
---- RESULTS
-'Column has been added/replaced.'
+'Column(s) have been added.'
====
---- QUERY
# Add a row
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
index f38b3c5..981a734 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
@@ -363,7 +363,7 @@ INT,INT,INT,INT,INT,INT,STRING,BOOLEAN,DECIMAL
---- QUERY
alter table tbl_with_defaults add columns (j int null, k int not null default 10000)
---- RESULTS
-'Column has been added/replaced.'
+'Column(s) have been added.'
====
---- QUERY
select * from tbl_with_defaults