Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION ......................................................................
Patch Set 22: (10 comments) http://gerrit.cloudera.org:8080/#/c/4144/22/fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java: Line 92: throw new AnalysisException( > Add a test for this in AnalyzeDDLTest Done http://gerrit.cloudera.org:8080/#/c/4144/22/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: PS22, Line 221: Used when persisting the results of COMPUTE STATS statements. > Update the comment (now it is used for the add partition statement) Done Line 1859: * 2. If a partition exists in HMS but not in catalog cache, reload partition from HMS. > mention that caching directives are only applied to new partitions that wer Done Line 1861: private Table alterTableAddPartitions(Table tbl, > Since there are some interesting details with existing partitions (in cache Filed IMPALA-4844 http://gerrit.cloudera.org:8080/#/c/4144/22/testdata/workloads/functional-query/queries/QueryTest/alter-table.test File testdata/workloads/functional-query/queries/QueryTest/alter-table.test: Line 1023: # IMPALA-1670: Cannot add duplicate partition specs. > remove, this is covered by an analyzer test Done Line 1033: # IMPALA-1670: Cannot add duplicate partition specs, not even if 'IF NOT EXISTS' is used. > remove, this is covered by an analyzer test Done Line 1056: # IMPALA-1670: If 'IF NOT EXISTS' is not used, ALTER TABLE ADD PARTITION cannot add a > This should be covered by an analyzer test instead Done Line 1066: # IMPALA-1670: If adding one partition fails, the entire statement fails. > Not really accurate. There are also modes of partial failure/success. If an Removed this section. The test case between L1056-1063 was moved to AnalyzeDDLTest. Line 1150: # IMPALA-1670: After REFRESH Impala can access previously added partitions and partition > Seems a little excessive to me, not sure if we need this test. I agree, we don't need to test the same scenario both with INVALIDATE METADATA and REFRESH. Removed this section. http://gerrit.cloudera.org:8080/#/c/4144/22/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test: Line 242: # IMPALA-1670: Revoke privileges, so that user would not have privileges to run ALTER > I agree. Not all of them are needed. I'd like to keep the ones where we gra Done -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 22 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
