diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift index d0fccc471..bac562614 100644 --- a/common/thrift/JniCatalog.thrift +++ b/common/thrift/JniCatalog.thrift @@ -144,13 +144,18 @@ struct TAlterTableChangeColParams { 2: required CatalogObjects.TColumn new_col_def } -// Parameters for ALTER TABLE SET TBLPROPERTIES|SERDEPROPERTIES commands. +// Parameters for ALTER TABLE SET [PARTITION ('k1'='a', 'k2'='b'...)] +// TBLPROPERTIES|SERDEPROPERTIES commands. struct TAlterTableSetTblPropertiesParams { // The target table property that is being altered. 1: required CatalogObjects.TTablePropertyType target // Map of property names to property values. 2: required map properties + + // If set, alters the properties of the given partition, otherwise + // those of the table. + 3: optional list partition_spec } // Parameters for ALTER TABLE SET [PARTITION partitionSpec] FILEFORMAT commands. diff --git a/fe/src/main/cup/sql-parser.y b/fe/src/main/cup/sql-parser.y index 1c362cf9b..e76a6b772 100644 --- a/fe/src/main/cup/sql-parser.y +++ b/fe/src/main/cup/sql-parser.y @@ -545,13 +545,7 @@ alter_tbl_stmt ::= {: RESULT = new AlterTableOrViewRenameStmt(table, new_table, true); :} | KW_ALTER KW_TABLE table_name:table partition_spec:partition KW_SET table_property_type:target LPAREN properties_map:properties RPAREN - {: - // Include unnecessary partition_spec to avoid a shift/reduce conflict on KW_SET. - if (partition != null) { - parser.parseError("partition", SqlParserSymbols.KW_PARTITION); - } - RESULT = new AlterTableSetTblProperties(table, target, properties); - :} + {: RESULT = new AlterTableSetTblProperties(table, partition, target, properties); :} ; table_property_type ::= diff --git a/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetStmt.java b/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetStmt.java index d6bcff13f..872544c7a 100644 --- a/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetStmt.java +++ b/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetStmt.java @@ -24,7 +24,7 @@ import com.cloudera.impala.common.AnalysisException; * Base class for all ALTER TABLE ... SET statements */ public class AlterTableSetStmt extends AlterTableStmt { - private final PartitionSpec partitionSpec_; + protected final PartitionSpec partitionSpec_; public AlterTableSetStmt(TableName tableName, PartitionSpec partitionSpec) { super(tableName); diff --git a/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetTblProperties.java b/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetTblProperties.java index fad440c2b..9722f292e 100644 --- a/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetTblProperties.java +++ b/fe/src/main/java/com/cloudera/impala/analysis/AlterTableSetTblProperties.java @@ -23,16 +23,16 @@ import com.cloudera.impala.thrift.TTablePropertyType; import com.google.common.base.Preconditions; /** -* Represents an ALTER TABLE SET TBLPROPERTIES|SERDEPROPERTIES ('p1'='v1', ...) statement. +* Represents an ALTER TABLE SET [PARTITION ('k1'='a', 'k2'='b'...)] +* TBLPROPERTIES|SERDEPROPERTIES ('p1'='v1', ...) statement. */ public class AlterTableSetTblProperties extends AlterTableSetStmt { private final TTablePropertyType targetProperty_; private final HashMap tblProperties_; - public AlterTableSetTblProperties(TableName tableName, - TTablePropertyType targetProperty, - HashMap tblProperties) { - super(tableName, null); + public AlterTableSetTblProperties(TableName tableName, PartitionSpec partitionSpec, + TTablePropertyType targetProperty, HashMap tblProperties) { + super(tableName, partitionSpec); Preconditions.checkNotNull(tblProperties); Preconditions.checkNotNull(targetProperty); targetProperty_ = targetProperty; @@ -50,6 +50,9 @@ public class AlterTableSetTblProperties extends AlterTableSetStmt { new TAlterTableSetTblPropertiesParams(); tblPropertyParams.setTarget(targetProperty_); tblPropertyParams.setProperties(tblProperties_); + if (partitionSpec_ != null) { + tblPropertyParams.setPartition_spec(partitionSpec_.toThrift()); + } params.setSet_tbl_properties_params(tblPropertyParams); return params; } diff --git a/fe/src/main/java/com/cloudera/impala/service/DdlExecutor.java b/fe/src/main/java/com/cloudera/impala/service/DdlExecutor.java index f55db5615..2baf3fd38 100644 --- a/fe/src/main/java/com/cloudera/impala/service/DdlExecutor.java +++ b/fe/src/main/java/com/cloudera/impala/service/DdlExecutor.java @@ -1094,29 +1094,53 @@ public class DdlExecutor { } /** - * Appends to the table property metadata for the given table, replacing the values - * of any keys that already exist. + * Appends to the table or partition property metadata for the given table, replacing + * the values of any keys that already exist. */ private void alterTableSetTblProperties(TableName tableName, - TAlterTableSetTblPropertiesParams params) throws MetaException, - InvalidObjectException, TException, DatabaseNotFoundException, - TableNotFoundException, TableLoadingException { + TAlterTableSetTblPropertiesParams params) throws TException, CatalogException { Map properties = params.getProperties(); Preconditions.checkNotNull(properties); synchronized (metastoreDdlLock) { - org.apache.hadoop.hive.metastore.api.Table msTbl = getMetaStoreTable(tableName); - switch (params.getTarget()) { - case TBL_PROPERTY: - msTbl.getParameters().putAll(properties); - break; - case SERDE_PROPERTY: - msTbl.getSd().getSerdeInfo().getParameters().putAll(properties); - break; - default: - throw new UnsupportedOperationException( - "Unknown target TTablePropertyType: " + params.getTarget()); + if (params.isSetPartition_spec()) { + // Alter partition params. + HdfsPartition partition = catalog.getHdfsPartition( + tableName.getDb(), tableName.getTbl(), params.getPartition_spec()); + org.apache.hadoop.hive.metastore.api.Partition msPartition = + partition.getMetaStorePartition(); + Preconditions.checkNotNull(msPartition); + switch (params.getTarget()) { + case TBL_PROPERTY: + msPartition.getParameters().putAll(properties); + break; + case SERDE_PROPERTY: + msPartition.getSd().getSerdeInfo().getParameters().putAll(properties); + break; + default: + throw new UnsupportedOperationException( + "Unknown target TTablePropertyType: " + params.getTarget()); + } + try { + applyAlterPartition(tableName, msPartition); + } finally { + partition.markDirty(); + } + } else { + // Alter table params. + org.apache.hadoop.hive.metastore.api.Table msTbl = getMetaStoreTable(tableName); + switch (params.getTarget()) { + case TBL_PROPERTY: + msTbl.getParameters().putAll(properties); + break; + case SERDE_PROPERTY: + msTbl.getSd().getSerdeInfo().getParameters().putAll(properties); + break; + default: + throw new UnsupportedOperationException( + "Unknown target TTablePropertyType: " + params.getTarget()); + } + applyAlterTable(msTbl); } - applyAlterTable(msTbl); } } diff --git a/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java index dc370840a..bb9b85261 100644 --- a/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java +++ b/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java @@ -254,6 +254,8 @@ public class AnalyzeDDLTest extends AnalyzerTest { public void TestAlterTableSet() throws AnalysisException { AnalyzesOk("alter table functional.alltypes set fileformat sequencefile"); AnalyzesOk("alter table functional.alltypes set location '/a/b'"); + AnalyzesOk("alter table functional.alltypes set tblproperties('a'='1')"); + AnalyzesOk("alter table functional.alltypes set serdeproperties('a'='2')"); AnalyzesOk("alter table functional.alltypes PARTITION (Year=2010, month=11) " + "set location '/a/b'"); AnalyzesOk("alter table functional.alltypes PARTITION (month=11, year=2010) " + @@ -262,6 +264,10 @@ public class AnalyzeDDLTest extends AnalyzerTest { "(string_col='partition1') set fileformat parquet"); AnalyzesOk("alter table functional.stringpartitionkey PARTITION " + "(string_col='PaRtiTion1') set location '/a/b/c'"); + AnalyzesOk("alter table functional.alltypes PARTITION (year=2010, month=11) " + + "set tblproperties('a'='1')"); + AnalyzesOk("alter table functional.alltypes PARTITION (year=2010, month=11) " + + "set serdeproperties ('a'='2')"); // Arbitrary exprs as partition key values. Constant exprs are ok. AnalyzesOk("alter table functional.alltypes PARTITION " + "(year=cast(100*20+10 as INT), month=cast(2+9 as INT)) " + @@ -283,6 +289,13 @@ public class AnalyzeDDLTest extends AnalyzerTest { AnalysisError("alter table functional.alltypes PARTITION (year=2014, month=11) " + "set location '/a/b'", "Partition spec does not exist: (year=2014, month=11)"); + AnalysisError("alter table functional.alltypes PARTITION (year=2014, month=11) " + + "set tblproperties('a'='1')", + "Partition spec does not exist: (year=2014, month=11)"); + AnalysisError("alter table functional.alltypes PARTITION (year=2010) " + + "set tblproperties('a'='1')", + "Items in partition spec must exactly match the partition columns " + + "in the table definition: functional.alltypes (1 vs 2)"); AnalysisError("alter table functional.alltypes PARTITION (year=2010, year=2010) " + "set location '/a/b'", "Duplicate partition key name: year"); diff --git a/fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java b/fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java index 59709c6b4..cdf342cfb 100644 --- a/fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java +++ b/fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java @@ -505,8 +505,8 @@ public class ParserTest { // Multiple with clauses. Operands must be in parenthesis to // have their own with clause. ParsesOk("with t as (select 1) " + - "(with t as (select 2) select * from t) union all " + - "(with t as (select 3) select * from t)"); + "(with t as (select 2) select * from t) union all " + + "(with t as (select 3) select * from t)"); ParsesOk("with t as (select 1) " + "(with t as (select 2) select * from t) union all " + "(with t as (select 3) select * from t) order by 1 limit 1"); @@ -955,8 +955,10 @@ public class ParserTest { // semantically incorrect negation, but parses ok ParsesOk("select a, b, c from t where a = " + notStr + "5"); // unbalanced parentheses - ParserError("select a, b, c from t where (a = 5 " + orStr + " b = 6) " + andStr + " c = 7)"); - ParserError("select a, b, c from t where ((a = 5 " + orStr + " b = 6) " + andStr + " c = 7"); + ParserError("select a, b, c from t where " + + "(a = 5 " + orStr + " b = 6) " + andStr + " c = 7)"); + ParserError("select a, b, c from t where " + + "((a = 5 " + orStr + " b = 6) " + andStr + " c = 7"); // incorrectly positioned negation (!) ParserError("select a, b, c from t where a = 5 " + orStr + " " + notStr); ParserError("select a, b, c from t where " + notStr + "(a = 5) " + orStr + " " + notStr); @@ -1538,21 +1540,25 @@ public class ParserTest { ParserError("ALTER TABLE TestDb.Foo SET"); String[] tblPropTypes = {"TBLPROPERTIES", "SERDEPROPERTIES"}; + String[] partClauses = {"", "PARTITION(k1=10, k2=20)"}; for (String propType: tblPropTypes) { - ParsesOk(String.format("ALTER TABLE Foo SET %s ('a'='b')", propType)); - ParsesOk(String.format("ALTER TABLE Foo SET %s ('abc'='123')", propType)); - ParsesOk(String.format("ALTER TABLE Foo SET %s ('abc'='123', 'a'='1')", propType)); - ParsesOk(String.format( - "ALTER TABLE Foo SET %s ('a'='1', 'b'='2', 'c'='3')", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s ( )", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s ('a', 'b')", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s ('a'='b',)", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s ('a'=b)", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s (a='b')", propType)); - ParserError(String.format("ALTER TABLE Foo SET %s (a=b)", propType)); - // Setting TBLPROPERTIES/SERDEPROPERTIES on a partition is not currently allowed - ParserError(String.format( - "ALTER TABLE Foo PARTITION (s='str') SET %s ('a'='b')", propType)); + for (String part: partClauses) { + ParsesOk(String.format("ALTER TABLE Foo %s SET %s ('a'='b')", part, propType)); + ParsesOk(String.format("ALTER TABLE Foo %s SET %s ('abc'='123')", + part, propType)); + ParsesOk(String.format("ALTER TABLE Foo %s SET %s ('abc'='123', 'a'='1')", + part, propType)); + ParsesOk(String.format("ALTER TABLE Foo %s SET %s ('a'='1', 'b'='2', 'c'='3')", + part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s ( )", part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s ('a', 'b')", + part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s ('a'='b',)", + part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s ('a'=b)", part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s (a='b')", part, propType)); + ParserError(String.format("ALTER TABLE Foo %s SET %s (a=b)", part, propType)); + } } } diff --git a/testdata/workloads/functional-query/queries/QueryTest/alter-table.test b/testdata/workloads/functional-query/queries/QueryTest/alter-table.test index e7f4e32b6..e53e9a28a 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/alter-table.test +++ b/testdata/workloads/functional-query/queries/QueryTest/alter-table.test @@ -540,3 +540,38 @@ group by int_col order by 1 limit 100 ---- TYPES int,bigint ==== +---- QUERY +# Show the table stats before altering. +show table stats alltypes_test +---- LABELS +YEAR, MONTH, #ROWS, #FILES, SIZE, FORMAT +---- RESULTS +2009,4,-1,1,regex:.+KB,'SEQUENCE_FILE' +2009,5,-1,1,regex:.+KB,'RC_FILE' +Total,,-1,2,regex:.+KB,'' +---- TYPES +INT, INT, BIGINT, BIGINT, STRING, STRING +==== +---- QUERY +# Test altering the 'numRows' table property of a table. +alter table alltypes_test set tblproperties ('numRows'='200') +---- RESULTS +==== +---- QUERY +# Test altering the 'numRows' table property of a partition. +alter table alltypes_test partition(year=2009, month=4) +set tblproperties ('numRows'='30') +---- RESULTS +==== +---- QUERY +# Show the table stats after altering the table and partition stats. +show table stats alltypes_test +---- LABELS +YEAR, MONTH, #ROWS, #FILES, SIZE, FORMAT +---- RESULTS +2009,4,30,1,regex:.+KB,'SEQUENCE_FILE' +2009,5,-1,1,regex:.+KB,'RC_FILE' +Total,,200,2,regex:.+KB,'' +---- TYPES +INT, INT, BIGINT, BIGINT, STRING, STRING +====