You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/04/03 02:11:09 UTC
[3/9] impala git commit: IMPALA-6739: Exception in ALTER TABLE SET
statements
IMPALA-6739: Exception in ALTER TABLE SET statements
The patch fixes issues with executing ALTER TABLE SET statements when
there are no matching partitions.
The patch also removes incorrect precondition i.e.
(partitionSet == null || !partitionSet.isEmpty()) in ALTER TABLE SET
statements because a partitionSet can be null when PARTITION is not
specified in the ALTER TABLE SET statement and partitionSet can be
empty when there is no matching partition. For example:
Matching partitions (partitionSet != null && !partitionSet.isEmpty()):
> alter table functional.alltypesagg partition(year=2009, month=1)
set fileformat parquet;
No matching partitions (partitionSet != null && partitionSet.isEmpty()):
> alter table functional.alltypesagg partition(year=2009, month=1)
set fileformat parquet;
No partition specified (partitionSet == null):
> alter table functional.alltypesagg set fileformat parquet;
Testing:
- Added a new test
- Ran all front-end tests
Change-Id: I793e827d5cf5b7986bd150dd9706df58da3417f3
Reviewed-on: http://gerrit.cloudera.org:8080/9819
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/c3ab2768
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/c3ab2768
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/c3ab2768
Branch: refs/heads/master
Commit: c3ab27681fbca613d6106567615138777f513b95
Parents: ab5c98c
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Tue Mar 27 09:37:31 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Mon Apr 2 21:05:40 2018 +0000
----------------------------------------------------------------------
.../analysis/AlterTableSetLocationStmt.java | 6 ++-
.../impala/service/CatalogOpExecutor.java | 2 -
.../apache/impala/analysis/AnalyzeDDLTest.java | 12 +++++
.../partition-ddl-predicates-all-fs.test | 52 ++++++++++++++++++++
4 files changed, 68 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/c3ab2768/fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
index f076312..ae31a76 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
@@ -59,8 +59,9 @@ public class AlterTableSetLocationStmt extends AlterTableSetStmt {
params.setAlter_type(TAlterTableType.SET_LOCATION);
TAlterTableSetLocationParams locationParams =
new TAlterTableSetLocationParams(location_.toString());
- if (getPartitionSet() != null) {
- List<List<TPartitionKeyValue>> tPartitionSet = getPartitionSet().toThrift();
+ PartitionSet partitionSet = getPartitionSet();
+ if (partitionSet != null && !partitionSet.getPartitions().isEmpty()) {
+ List<List<TPartitionKeyValue>> tPartitionSet = partitionSet.toThrift();
Preconditions.checkState(tPartitionSet.size() == 1);
locationParams.setPartition_spec(tPartitionSet.get(0));
}
@@ -80,6 +81,7 @@ public class AlterTableSetLocationStmt extends AlterTableSetStmt {
if (getPartitionSet() != null) {
// Targeting a partition rather than a table.
List<HdfsPartition> partitions = getPartitionSet().getPartitions();
+ if (partitions.isEmpty()) { return; }
if (partitions.size() != 1) {
// Sort the partitions to get a consistent error reporting.
List<HdfsPartition> sortedPartitions = Lists.newArrayList(partitions);
http://git-wip-us.apache.org/repos/asf/impala/blob/c3ab2768/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
----------------------------------------------------------------------
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 9ccfa50..184a839 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -2238,7 +2238,6 @@ public class CatalogOpExecutor {
Reference<Long> numUpdatedPartitions)
throws ImpalaException {
Preconditions.checkState(tbl.getLock().isHeldByCurrentThread());
- Preconditions.checkState(partitionSet == null || !partitionSet.isEmpty());
boolean reloadFileMetadata = false;
if (partitionSet == null) {
org.apache.hadoop.hive.metastore.api.Table msTbl =
@@ -2275,7 +2274,6 @@ public class CatalogOpExecutor {
Reference<Long> numUpdatedPartitions)
throws ImpalaException {
Preconditions.checkState(tbl.getLock().isHeldByCurrentThread());
- Preconditions.checkState(partitionSet == null || !partitionSet.isEmpty());
Preconditions.checkArgument(tbl instanceof HdfsTable);
boolean reloadFileMetadata = false;
RowFormat rowFormat = RowFormat.fromThrift(tRowFormat);
http://git-wip-us.apache.org/repos/asf/impala/blob/c3ab2768/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
----------------------------------------------------------------------
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 e4a90dd..8f9e384 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -535,6 +535,18 @@ public class AnalyzeDDLTest extends FrontendTestBase {
"'sort.columns'='ID, foo')",
"Could not find SORT BY column 'foo' in table.");
+ // Table is partitioned, but it has no matching partitions.
+ AnalyzesOk("alter table functional.alltypesagg partition(year=2009, month=1) " +
+ "set location 'hdfs://localhost:20500/test-warehouse/new_table'");
+ AnalyzesOk("alter table functional.alltypesagg partition(year=2009, month=1) " +
+ "set fileformat parquet");
+ AnalyzesOk("alter table functional.alltypesagg partition(year=2009, month=1) " +
+ "set tblproperties ('key'='value')");
+ AnalyzesOk("alter table functional.alltypesagg partition(year=2009, month=1) " +
+ "set serdeproperties ('key'='value')");
+ AnalyzesOk("alter table functional.alltypesagg partition(year=2009, month=1) " +
+ "set row format delimited fields terminated by '|'");
+
{
// Check that long_properties fail at the analysis layer
String long_property_key = "";
http://git-wip-us.apache.org/repos/asf/impala/blob/c3ab2768/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test b/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
index 17113aa..f90dcff 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
@@ -153,3 +153,55 @@ show partitions p1
---- TYPES
STRING, STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING
====
+---- QUERY
+# Tests no matching partition.
+alter table p1 partition (j=100) set location '$FILESYSTEM_PREFIX/test-warehouse/newtable';
+---- RESULTS
+# TODO: IMPALA-6775
+==== QUERY
+# Check nothing was updated.
+show partitions p1
+---- RESULTS
+'NULL','g',-1,0,regex:.+,regex:.+,regex:.+,regex:.+,regex:.+,regex:.*/test-warehouse/.+/p1/j=__HIVE_DEFAULT_PARTITION__/k=g
+'2','D',-1,0,regex:.+,regex:.+,regex:.+,'TEXT',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=D
+'2','E',-1,0,regex:.+,regex:.+,regex:.+,'TEXT',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=E
+'2','F',-1,0,regex:.+,regex:.+,regex:.+,'TEXT',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=F
+'2','d',-1,0,regex:.+,regex:.+,regex:.+,'PARQUET',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=d
+'2','e',-1,0,regex:.+,regex:.+,regex:.+,'PARQUET',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=e
+'2','f',-1,0,regex:.+,regex:.+,regex:.+,'PARQUET',regex:.+,regex:.*/test-warehouse/.+/p1/j=2/k=f
+'Total','',3,0,regex:.+,regex:.+,'','','',''
+---- TYPES
+STRING, STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING
+====
+---- QUERY
+# Tests no matching partition.
+alter table p1 partition (j=100) set fileformat parquet;
+---- RESULTS
+'Updated 0 partition(s).'
+---- TYPES
+STRING
+====
+---- QUERY
+# Tests no matching partition.
+alter table p1 partition (j=100) set tblproperties ('key'='value');
+---- RESULTS
+'Updated 0 partition(s).'
+---- TYPES
+STRING
+====
+---- QUERY
+# Tests no matching partition.
+alter table p1 partition (j=100) set serdeproperties ('key'='value');
+---- RESULTS
+'Updated 0 partition(s).'
+---- TYPES
+STRING
+====
+---- QUERY
+# Tests no matching partition.
+alter table p1 partition (j=100) set row format delimited fields terminated by '|';
+---- RESULTS
+'Updated 0 partition(s).'
+---- TYPES
+STRING
+====
\ No newline at end of file