[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 24: Verified+1 -- 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: 24 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionDef class to capture the repeatable part of the statement. TPartitionDef is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Reviewed-on: http://gerrit.cloudera.org:8080/4144 Reviewed-by: Dimitris Tsirogiannis Tested-by: Impala Public Jenkins --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 858 insertions(+), 207 deletions(-) Approvals: Impala Public Jenkins: Verified Dimitris Tsirogiannis: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 25 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 24: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/235/ -- 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: 24 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 24: Code-Review+2 -- 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: 24 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 23: Code-Review+2 -- 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: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 23: Yes, looking at it now. -- 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: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Michael Ho has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 23: Dimitris, can you please take a quick pass again to see if there is anything else which needs to be addressed ? Alex has +1'ed it already. -- 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: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Alex Behm has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 23: Code-Review+1 -- 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: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#23). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionDef class to capture the repeatable part of the statement. TPartitionDef is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 858 insertions(+), 207 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/23 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4144 to look at the new patch set (#23). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionDef class to capture the repeatable part of the statement. TPartitionDef is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 858 insertions(+), 207 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/23 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 23 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 22: Code-Review+1 (2 comments) Catalog changes look good. Thanks again Attila. 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) 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 > These cases should be covered by AuthorizationTest already. I don't think w I agree. Not all of them are needed. I'd like to keep the ones where we grant/revoke URI privileges and then try to add at partitions on these location. -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Alex Behm 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) Thanks for the clarifications. Change looks pretty good to me. 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 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: 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 were absent from both the catalog cache and the HMS Line 1861: private Table alterTableAddPartitions(Table tbl, Since there are some interesting details with existing partitions (in cache or HMS), I think it would be nice to return the number of partitions actually created to the user. I'm ok with doing that in a follow-on patch. If you decide to go that route, please file a JIRA. 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 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 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 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 analysis fails, it's clear no state should have changed (because the stmt does not go into runtime). Line 1076: # IMPALA-1670: If 'IF NOT EXISTS' is used ALTER TABLE ADD PARTITION works with preexisting Nice! 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. 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 These cases should be covered by AuthorizationTest already. I don't think we need to test grant/revoking and then performing this ALTER TABLE specifically. What do you think, Dimitris? -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#22). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 960 insertions(+), 207 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/22 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 22 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4144 to look at the new patch set (#22). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 960 insertions(+), 207 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/22 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 22 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#21). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 960 insertions(+), 210 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/21 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 21 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4144 to look at the new patch set (#21). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 960 insertions(+), 210 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/21 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 21 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#20). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 953 insertions(+), 219 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/20 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 20 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 20:
(18 comments)
I'm sorry it took this long to answer your comments. I've worked on other stuff
for a couple of weeks.
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
File
fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java:
Line 51: p.setTableName(tableName);
> style: extra after 'partitions')
What do you mean? It is an underscore not a space after 'partitions' and there
is only one space after the closing ')'
PS19, Line 73: TAlterTableParams params = super.toThrift();
: params.setAlter_type(TAlterTableType.ADD_PARTITION);
> I think you can simply use TAlterTableParams.AddToPartition_params(). It al
Done
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/PartitionParams.java
File fe/src/main/java/org/apache/impala/analysis/PartitionParams.java:
Line 31
> Confusing leading sentence (what's a partial SQL statement?). How about som
Done
Line 36
> "Params" is pretty generic and could easily be confused with other "Partiti
Done
Line 38
> nit: order members as in the SQL clause, i.e, spec, loc, cacheOp
Done
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java:
Line 208
> toCanonicalString()?
Done
PS19, Line 211:
> nit: this is just for comparison purposes, no need to pretty print (save a
Done
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1767: if (!iterator.hasNext()) {
> Do we even need this step of checking the partitions against the local (pos
This step is necessary because in L1813-1817 we want to add only those
partitions to catalog_ that are not in there yet.
Consider a scenario when a partition exists in catalog_ but it does not exist
in HMS yet. If we skip this check in L1767, then addHdfsPartition() in L1816
would add the partition to catalog_ again. Note that addHdfsPartition() does
not check for duplicate partitions and it will add the same partition again to
catalog_ if we don't filter out duplicates beforehand.
Line 1793: org.apache.hadoop.hive.metastore.api.Table msTbl =
tbl.getMetaStoreTable().deepCopy();
> Please file a JIRA to adhere to MAX_PARTITION_UPDATES_PER_RPC in this batch
Created IMPALA-4524 and added the check to AlterTableAddPartitionStmt.analyze()
Line 1801: String partitionSpecStr = Joiner.on(",
").join(partitionSpec);
> Weird that we have two round trips to the HMS here. Is it reasonable to get
In alterTableCachePartitions() we call
HdfsCachingUtil.submitCachePartitionDirective() for each partition that needs
to be cached. This submits a new caching directive for the specified cache pool
name, path and replication. It also adds the resulting directive ID to the
corresponding partition. HdfsCachingUtil.submitCachePartitionDirective() should
be called for newly added partitions only; that is, partitions that did not
exist in catalog_ or HMS before.
If we want to get everything done in a single add_partitions() call, the
HdfsCachingUtil.submitCachePartitionDirective() calls have to be made before
add_partitions() because we need to know the directive IDs. Unfortunately at
that point we don't know yet which partitions are preexisting (in HMS) and
which are not and thus we might end-up submitting cache directives for the
wrong path.
I've added a short comment.
Line 1806: LOG.debug(String.format("Skipping partition creation because
(%s) already " +
> I understand that this is necessary, but it's weird in combination with L17
We need the check in L1767 because in L1813-1817 we should add only those
partitions to catalog_ that are not in there yet to avoid duplicates.
In L1767 we check if there are any conflicts with partitions stored in
catalog_. In L1807 we check if there are any conflicts with partitions stored
in HMS.
In L1807-1811 we are loading those partitions from HMS that exist in HMS, but
don't exist in catalog_ yet. We have to load these partitions because we want
to add them to catalog_ in L1813-1817.
PS19, Line 1810:
> Let's try to be a bit more defensive here. If getPartiionsFromHms() throws
I've changed the wording of ImpalaRuntimeException in L1850
Line 1846:
> Fetch them in bulk using getPartitionsByNames()
Done
http://gerrit.cloudera.org:8080/#/c/4144/19/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:
PS19, Line 963: TYPES
> You don'
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4144 to look at the new patch set (#20). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 953 insertions(+), 219 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/20 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 20 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Alex Behm has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 19:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
File
fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java:
Line 51: for (PartitionParams p: partitions_) {
style: extra after 'partitions')
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/PartitionParams.java
File fe/src/main/java/org/apache/impala/analysis/PartitionParams.java:
Line 31: * Represents the partial SQL statement of specifying a table partition
Confusing leading sentence (what's a partial SQL statement?). How about
something like this:
Represents a partition definition used in ALTER TABLE ADD PARTITION consisting
of partition key-value pairs and an optional location and optional caching
options.
Line 36: public class PartitionParams implements ParseNode {
"Params" is pretty generic and could easily be confused with other "Partition*"
stuff. How about PartitionDef?
Line 38: private final PartitionSpec partitionSpec_;
nit: order members as in the SQL clause, i.e, spec, loc, cacheOp
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java:
Line 208: public String getComparableStringRep() {
toCanonicalString()?
http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1767: if (catalog_.containsHdfsPartition(tableName.getDb(),
tableName.getTbl(),
Do we even need this step of checking the partitions against the local
(possibly stale) cache? This might cause the table to be loaded which is
expensive and seems unnecessary.
We eventually have to deal with the "truly" added partitions based on the RPC
response from the HMS anyway.
Line 1793: addedHmsPartitions =
msClient.getHiveClient().add_partitions(hmsPartitionsToAdd,
Please file a JIRA to adhere to MAX_PARTITION_UPDATES_PER_RPC in this batched
RPC to the HMS. We've seen problems in the past when updating a lot of metadata
in one RPC to the HMS.
For now, please enforce during analysis that one ALTER TABLE ADD PARTITION can
only add at most MAX_PARTITION_UPDATES_PER_RPC partitions.
Line 1801: alterTableCachePartitions(msTbl, msClient, tableName,
addedHmsPartitions,
Weird that we have two round trips to the HMS here. Is it reasonable to get the
work done in a single add_partitions() RPC?
If not, it would be good to leave a brief comment or TODO.
I don't think we need to hold up the patch for this, I just want to understand
the reasoning/issue :)
Line 1806: // catalog the partitions that already exist in HMS but aren't
in the catalog yet.
I understand that this is necessary, but it's weird in combination with L1767
where we may just have loaded the entire table metadata, but can still be
inconsistent down here. I think that strengthens the case for getting rid of
the check in L1767.
Also do we even need to load those partitions? Is the expectation that after
the ALTER all the affected partitions are loaded?
Line 1846: part =
msClient.getHiveClient().getPartition(part.getDbName(),
Fetch them in bulk using getPartitionsByNames()
--
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: 19
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 19: Code-Review+1 (8 comments) Minor fixes in the tests. I'll ask Alex to see if he can make a final pass. http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java: PS19, Line 73: List partParams = Lists.newArrayList(); : for (PartitionParams p: partitions_) partParams.add(p.toThrift()); I think you can simply use TAlterTableParams.AddToPartition_params(). It already handles the initialization of the list and can save you a few precious lines :) http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java: PS19, Line 211: nit: this is just for comparison purposes, no need to pretty print (save a few bytes :)). http://gerrit.cloudera.org:8080/#/c/4144/19/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: PS19, Line 1810: addedHmsPartitions.addAll(getPartitionsFromHms(msClient, difference)); Let's try to be a bit more defensive here. If getPartiionsFromHms() throws an exception we should inform the user that the some metadata inconsistency has occurred and the user should run INVALIDATE METADATA to recover. http://gerrit.cloudera.org:8080/#/c/4144/19/testdata/workloads/functional-query/queries/QueryTest/alter-table.test File testdata/workloads/functional-query/queries/QueryTest/alter-table.test: PS19, Line 963: drop table if exists i1670_alter; You don't need the drop table statements. Plz remove PS19, Line 987: Duplicate partition spec: (j=11, n='alex', i=1) Isn't this an analysis exception? I believe it will be an ImpalaRuntimeException which you can catch here if "IF NOT EXISTS" is not used. PS19, Line 990: # IMPALA-1670: IF NOT EXISTS works as expected: : # - ignores existing partitions and adds the rest. : # - location and caching options of existing partitions are not modified. : # - if adding one partition fails, the entire statement fails. The query that follows this comment does not use the IF NOT EXISTS clause which is confusing. You may want to remove this and add a comment before each ALTER TABLE stmt that indicates what's being currently tested. http://gerrit.cloudera.org:8080/#/c/4144/19/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test: PS19, Line 242: # IMPALA-1670: Revoke privileges, so that user would not have privileges to run ALTER TABLE nit: long line http://gerrit.cloudera.org:8080/#/c/4144/19/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS19, Line 733: rows nit: with_data -- 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: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#19). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 940 insertions(+), 215 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/19 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 18: Reordered import statements -- 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: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#19). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 940 insertions(+), 215 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/19 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#18). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 949 insertions(+), 224 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/18 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#18). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 949 insertions(+), 224 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/18 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#18). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 952 insertions(+), 224 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/18 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS14, Line 714: # Sometimes metadata load is triggered here, so compare only the first three : # returned partitions. > Thanks for looking into it. Yes, the explanation makes sense. Let's remove 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: I'll make a final pass at the new patch. Nice work! -- 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS14, Line 714: # Sometimes metadata load is triggered here, so compare only the first three : # returned partitions. > I did some additional investigation. You are correct that setting SYNC_DDL Thanks for looking into it. Yes, the explanation makes sense. Let's remove the partition x=4 from the hive call and remove the comment that triggered this :). You can leave L716-719 as is. -- 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS14, Line 714: # Sometimes metadata load is triggered here, so compare only the first three : # returned partitions. > I am not sure this explanation and the fix is correct. SYNC_DDL causes a me I did some additional investigation. You are correct that setting SYNC_DDL is not directly related to the issue, just made it easier to reproduce. (A) The issue can be reproduced manually with Hive and Impala shells, so this is definitely not a bug in the python tests. Steps to reproduce: [0] Start Impala and Hive shells side-by-side. [1] impala> create table ptbl (a int) partitioned by (x int); [2] impala> show partitions ptbl; [3] hive> alter table ptbl add partition (x=1); [4] impala> show partitions ptbl; [5] impala> set sync_ddl=1; (OR wait for few seconds between [6] and [7]) [6] impala> alter table ptbl add partition (x=2) cached in 'testPool'; [7] impala> show partitions ptbl; In [2] and [4] the list of partitions is empty. In [7] the list of partitions should contain (x=2) only, but it contains both (x=1) and (x=2). (A.1) This behavior can be reproduced without the IMPALA-1670 related changes, so this is not something that IMPALA-1670 introduced. (A.2) Setting SYNC_DDL in [5] is not necessary, we get the same behavior if we wait for a few seconds instead between [6] and [7]. (A.3) If we run [6] without any caching options, Impala works as expected and in [7] the list of partitions contains only (x=2) (B) In asf-gerrit/master branch (without the IMPALA-1670 change-set): CatalogOpExecutor.alterTableAddPartition() calls CatalogServiceCatalog.watchCacheDirs() with cache directive IDs (L1704). The comments for CatalogServiceCatalog.watchCacheDirs() state: "Adds a list of cache directive IDs for the given table name. Asynchronously refreshes the table metadata once all cache directives complete." So, this is what triggers the metadata load. How do you think I should proceed? -- 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS14, Line 714: # Sometimes metadata load is triggered here, so compare only the first three : # returned partitions. > The non-deterministic behavior was caused by the SYNC_DDL query option, whi I am not sure this explanation and the fix is correct. SYNC_DDL causes a metadata operation (DDL statement) to wait until metadata have been broadcasted to all the Impalad nodes in the cluster. It doesn't trigger a metadata load from HMS. So, whether a partition that was added through HIve is visible in impala shouldn't in theory depend on SYNC_DDL. -- 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#17). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 949 insertions(+), 222 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/17 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#17). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 16 files changed, 949 insertions(+), 222 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/17 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(3 comments)
> Attila, any updates on this one? Let me know if you need any help
> with the tests?
Sorry for the late response, I was on PTO for a week. Uploading the new
change-set now.
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java:
PS14, Line 211: new Comparator() {
: @Override
: public int compare(PartitionKeyValue t, PartitionKeyValue
o) {
: return t.getColName().compareTo(o.getColName());
: }
> Thanks for the explanation. Can you please consider moving this comparison
Moved it to PartitionKeyVale.getColNameComparator()
http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
PS14, Line 714: # Sometimes metadata load is triggered here, so compare only
the first three
: # returned partitions.
> I am sorry but we can't just leave it like this. a) we need to understand i
The non-deterministic behavior was caused by the SYNC_DDL query option, which
is set to 1 in the previous tests (in ImpalaDbWrapper.__exit__()).
Setting SYNC_DDL to 0 in ImpalaDbWrapper.__enter__() fixed the issue.
PS14, Line 770: def test_partitions_exist_after_refresh(self, vector):
> I don't think this is an integration test. You may want to move it to test_
This test and the INVALIDATE METADATA case was moved to alter-table.test
(L1059-L1132).
--
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: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 16: Attila, any updates on this one? Let me know if you need any help with the tests? -- 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: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Michael Ho has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java:
PS14, Line 211: new Comparator() {
: @Override
: public int compare(PartitionKeyValue t, PartitionKeyValue
o) {
: return t.getColName().compareTo(o.getColName());
: }
> I was thinking about that too, but comparing the value part of PartitionKey
Thanks for the explanation. Can you please consider moving this comparison
logic to PartionKeyValue as this seems to be where this function belongs ?
--
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: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: PS14, Line 714: # Sometimes metadata load is triggered here, so compare only the first three : # returned partitions. > Changed the comment, hope it makes more sense now. I am sorry but we can't just leave it like this. a) we need to understand if there is a bug here (most likely this is not the case), b) if the result depends on some previous invocation of another test or if it is sensitive to the order the tests are executed, it means that we need to fix these tests. -- 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: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#16). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py M tests/metadata/test_refresh_partition.py 16 files changed, 951 insertions(+), 221 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/16 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java:
PS14, Line 89: = false;
> No need for the assignment as this variable is assigned in all paths to lin
Done
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java:
PS14, Line 211: new Comparator() {
: @Override
: public int compare(PartitionKeyValue t, PartitionKeyValue
o) {
: return t.getColName().compareTo(o.getColName());
: }
> What do you think about implementing the Comparable interface for Partition
I was thinking about that too, but comparing the value part of
PartitionKeyValue objects is not that straightforward. To simplify things, I
could just compare the PartitionKeyValue.toString() values, but that might not
be the ordering that users of PartitionKeyValue expect.
In PartitionSpec class, analyze() ensures that there are no duplicate keys,
therefore comparing keys only is sufficient and the issue of comparing values
can be avoided.
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS13, Line 1634: IF NOT EXISTS is used, conflicts are hand
> Same comment as above. I suppose this one is "IF NOT EXISTS is used" and th
Done
PS13, Line 1720:
> nit: long line
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: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#15). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py M tests/metadata/test_refresh_partition.py 16 files changed, 951 insertions(+), 221 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/15 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 15: (21 comments) http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: PS14, Line 1629: > " or null if the table is not altered because all the partitions already ex Done Line 1630 > nit: this line seems to wrap a bit early. We allow up to 90 chars per line. Done PS14, Line 1635: : > remove Done PS14, Line 1658: > Is this needed ? If so, will "IF NOT EXISTS was specified" be more appropr Done PS14, Line 1694: > Does the HMS call throw an exception if "ifNotExits" is false and some part Yes, it does. In that case, alterTableAddPartitions() throws an exception too on L1679. PS14, Line 1694: > nit: "If" Done PS14, Line 1706: > Do you know how this function synchronizes against concurrent query adding DDL operations follow the locking protocol described on L173-181. addHdfsPartition() is only called from alterTableRecoverPartitions() and alterTableAddPartitions(). At the beggining of these functions we check that the current thread holds the table lock. PS14, Line 1714: : > I don't think this function is making use or enforcing this assumption anyw Done PS14, Line 1724: : > nit: one line Done PS14, Line 1753: > nit: maybe it's better to say "maps partitions (identified by their partiti Done PS14, Line 1800: : : : : : : > Should this be factored out to a function similar to applyAlterPartition() Factored it out to applyAlterHmsPartitions() function. applyAlterPartition() now calls applyAlterHmsPartitions(). http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java: PS14, Line 220: : : : : : > I think this case simply enforces the unique partition specs (similar to th Done Line 246 > Also you may want to add a test case where one of the partitions is cached Done http://gerrit.cloudera.org:8080/#/c/4144/14/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: PS14, Line 172: def get_impala_partition_info(self, table_name, *include_ > A few thoughts on this function: Done PS14, Line 174: returned by a S > maybe: "returned by a SHOW PARTITIONS statement." Done http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: Line 269: Passes 'command' to 'engine' callable (e.g. execute method of a BeeswaxConnection > Thanks for adding the comment. Maybe an example of an 'engine' callable (in Done Line 701: "partition (x=3) cached in 'testPool' with replication=3" % table_name, > You may want to add an assert here to verify that no partitions were added Done PS14, Line 713: > Even partitions need to be left alone from time to time :). Maybe "was not Done PS14, Line 714: # Impala sees the partition that has already existed in HMS (x=2) and the newly : # added partitions (x= > Not sure I follow this comment. Can you explain the non-deterministic behav Changed the comment, hope it makes more sense now. Additional details: 1. If I'm running all the tests in test_hms_integration.py, impala_partitions() on L719 returns all four partitions (x=1,2,3,4). 2. If I only run test_add_overlapping_partitions(), impala_partitions() returns three partitions (x=1,2,3). I'm not really sure what is going on here, looks like a metadata load is somehow triggered in the 1st scenario. PS14, Line 770: # Impala sees the partitions > I don't think this is an integration test. You may want to move it to test_ Done Line 800: # Hive sees the partitions > Also try to add some new and existing partitions using Impala (maybe with l This scenario is already tested in alter-table.test L987-L1037. -- 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: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#15). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionParams.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py M tests/metadata/test_refresh_partition.py 16 files changed, 951 insertions(+), 221 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/15 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Michael Ho has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java:
PS14, Line 89: = false;
No need for the assignment as this variable is assigned in all paths to line 96.
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java:
PS14, Line 211: new Comparator() {
: @Override
: public int compare(PartitionKeyValue t, PartitionKeyValue
o) {
: return t.getColName().compareTo(o.getColName());
: }
What do you think about implementing the Comparable interface for
PartitionKeyValue ? That would make the comparator reusable else where too and
it would be consistent across all callsites.
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS13, Line 1634: IF NOT EXISTS is used, conflicts are hand
Same comment as above. I suppose this one is "IF NOT EXISTS is used" and the
one above "IF NOT EXISTS is not used".
PS13, Line 1720:
nit: long line
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
Line 1630:* If IF NOT EXISTS is not used and there is a conflict with the
nit: this line seems to wrap a bit early. We allow up to 90 chars per line.
PS14, Line 1658: and ifNotExists is true.
Is this needed ? If so, will "IF NOT EXISTS was specified" be more appropriate
?
PS14, Line 1694: 'ifNotExists' is true
Does the HMS call throw an exception if "ifNotExits" is false and some
partitions already exist in HMS ?
PS14, Line 1694: if
nit: "If"
PS14, Line 1706: addHdfsPartition(tbl, partition);
Do you know how this function synchronizes against concurrent query adding the
same set of partitions ?
PS14, Line 1724: if (!bSet.contains(a.getValues())) {
: diffList.add(a);
nit: one line
PS14, Line 1800: try {
:
msClient.getHiveClient().alter_partitions(tableName.getDb(), tableName.getTbl(),
: hmsPartitionsToCache);
: } catch (TException e) {
: throw new ImpalaRuntimeException(
: String.format(HMS_RPC_ERROR_FORMAT_STR,
"alter_partitions"), e);
: }
Should this be factored out to a function similar to applyAlterPartition() ? In
fact, is it possible to somehow combine this with applyAlterPartition() by
passing a list with single entry ?
--
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: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(14 comments)
Very nice! Minor nits in the code, some comments in testing.
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS14, Line 1629: .
" or null if the table is not altered because all the partitions already exist
and IF NOT EXISTS is specified".
PS14, Line 1635: If all partitions exist in
:* catalog cache, return null.
remove
PS14, Line 1714: It is assumed that all Partition objects in 'aList' and
'bList' belong to the same
:* table.
I don't think this function is making use or enforcing this assumption
anywhere. So, you may want to remove this sentence.
PS14, Line 1753: lists of partition values
nit: maybe it's better to say "maps partitions (identified by their partition
values) to their corresponding HDFS caching ops."
http://gerrit.cloudera.org:8080/#/c/4144/14/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java:
PS14, Line 220: AnalysisError("alter table functional.alltypes add " + cl +
: " partition(year=2050, month=10)" +
: " partition(year=2050, month=11)" +
: " partition(Month=10, YEAR=2050) location" +
: "
'hdfs://localhost:20500/test-warehouse/alltypes/year=2010/month=10'",
: "Duplicate partition spec: (month=10, year=2050)");
I think this case simply enforces the unique partition specs (similar to the
one in L215). So I am not sure if LOCATION adds anything more in terms of test
coverage in this case. Maybe just keep the previous case?
Line 246: }
Also you may want to add a test case where one of the partitions is cached in a
non-existent pool.
http://gerrit.cloudera.org:8080/#/c/4144/14/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
PS14, Line 172: def impala_partitions(self, table_name, *include_fields):
A few thoughts on this function:
1. The field positions seem to be hardcoded based on the current output of SHOW
PARTITIONS. Any change in the output will break the tests that rely on this
function.
2. "impala_partitions" doesn't really indicate what to expect from this
function. Maybe "get_impala_partitions" or "get_impala_partition_info" is more
appropriate.
3. I find it awkward that by default it returns the partition values and all
other fields are optional. Personally, I'd change it so that, if include_fields
is not specified, return all fields. Otherwise, return only those fields in
'include_fields'.
PS14, Line 174: Impala sees them
maybe: "returned by a SHOW PARTITIONS statement."
http://gerrit.cloudera.org:8080/#/c/4144/14/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
Line 269: Passes 'command' to 'engine' callable and makes sure that it
raises an exception.
Thanks for adding the comment. Maybe an example of an 'engine' callable (in
terms of an existing Python class that can be used) would also help future
readers understand how to use this.
Line 701: "Partition already exists")
You may want to add an assert here to verify that no partitions were added to
Impala.
PS14, Line 713: was left alone
Even partitions need to be left alone from time to time :). Maybe "was not
modified."
PS14, Line 714: # Sometimes metadata load is triggered here, so compare only
the first three
: # returned partitions.
Not sure I follow this comment. Can you explain the non-deterministic behavior
wrt metadata load?
PS14, Line 770: def test_partitions_exist_after_refresh(self, vector):
I don't think this is an integration test. You may want to move it to
test_partition_metadata.py and also cover the 'invalidate metadata' case. Also,
another integration test case that is currently not covered is adding
partitions using Impala and checking that they are accessible through Hive.
Line 800: table_name).get_data().split('\n')
Also try to add some new and existing partitions using Impala (maybe with
location and cached properties) and verify that new partitions are added
correctly and existing partitions are not modified. You may want to test w and
w/o IF NOT EXISTS.
--
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: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Bh
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 14:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS13, Line 1630: IF NOT EXISTS is not used and there is a c
> "IF NOT EXISTS is used "
Done
PS13, Line 1683:
> "fail"
Done
PS13, Line 1684: tions,
> "In that case, we"
Done
PS13, Line 1687: try {
: updateLastDdlTime(msTbl, msClient);
: } catch (TException e) {
: throw new ImpalaRuntimeException(
: String.format(HMS_RPC_ERROR_FORMAT_STR,
"updateLastDdlTime"), e);
: }
> The semantics of getPartitionFromHMS() (L1720) and the way it is used here
Done
PS13, Line 1715: table. Partition objects are distinguished by partition values
only.
:*/
: private List computeDifference(List
aList,
: List bList) {
: Set> bSet = Sets.newHashSet();
: for (Partition b: bList) bSet.add(b.getValues());
:
> This function and the way it is used is kind of confusing. What you essenti
Done
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java:
PS13, Line 205:
: @Test
: public void TestAlterTableAddMultiplePartitions() {
: for (String cl: new String[]{"if not exists", ""}) {
: // Add multiple partitions.
: AnalyzesOk("alter table functional.alltypes add " + cl +
: " partition(year=2050, month=10)" +
: " partition(year=2050, month=11)" +
: " partition(year=2050, month=12)");
: // Duplicate partition specifications.
: AnalysisError("alter table functional.alltypes add " + cl
+
: " partition(year=2050, month=10)" +
: " partition(year=2050, month=11)" +
: " partition(Month=10, YEAR=2050)",
: "Duplicate partition spec: (month=10, year=2050)");
: AnalysisError("alter table functional.alltypes add " + cl
+
: " partition(year=2050, month=10)" +
: " partition(year=2050, month=11)" +
: " partition(Month=10, YEAR=2050) location" +
: "
'hdfs://localhost:20500/test-warehouse/alltypes/year=2010/month=10'",
: "Duplicate partition spec: (month=10, year=2050)");
:
: // Multiple partitions with locations and caching
: AnalyzesOk("alter table functional.alltypes add " + cl +
: " partition(year=2050, month=10) location" +
: " '/test-warehouse/alltypes/y2050m10' cached in
'testPool'" +
: " partition(year=2050, month=11) location" +
: "
'hdfs://localhost:20500/test-warehouse/alltypes/y2050m11'" +
: " cached in 'testPool' with replication = 7" +
: " partition(year=2050, month=12) location" +
: " 'file:///test-warehouse/alltypes/y2050m12'
uncached");
: // One of the partitions points to an invalid URI
: AnalysisError("alter table functional.alltypes add " + cl
+
: " partition(year=2050, month=10) location" +
: " '/test-warehouse/alltypes/y2050m10' cached in
'testPool'" +
: " partition(year=2050, month=11) location" +
: "
'hdfs://localhost:20500/test-warehouse/alltypes/y2050m11'" +
: " cached in 'testPool' with replication = 7" +
: " partition(year=2050, month=12) location" +
: " 'fil:///test-warehouse/alltypes/y2050m12' uncached",
: "No FileSystem for scheme: fil");
: }
> One easy way to increase test coverage could be to parameterize these test
Done. Moved new test cases to TestAlterTableAddMultiplePartitions() function
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java:
Line 1236:
> Also, can you add a test case where the user doesn't have privileges to add
There is already a test like that on L1255
http://gerrit.cloudera
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#14). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 15 files changed, 888 insertions(+), 231 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/14 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#14). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 15 files changed, 888 insertions(+), 231 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/14 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER
TABLE ADD PARTITION
..
Patch Set 13:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS13, Line 1630: 'addPartParams.isIf_not_exists()' is false
"IF NOT EXISTS is used "
PS13, Line 1683: have failed
"fail"
PS13, Line 1684: So, we need to
"In that case, we"
PS13, Line 1687: if (hmsPartitionsToAdd.size() == addedHmsPartitions.size()) {
: hmsPartitionsToAdd = addedHmsPartitions;
: } else {
: hmsPartitionsToAdd = getPartitionsFromHMS(msClient,
tableName, hmsPartitionsToAdd,
: addedHmsPartitions);
: }
The semantics of getPartitionFromHMS() (L1720) and the way it is used here is
kind of confusing. What you essentially need in the else case is to compute the
set difference between hmsPartitionsToAdd and addHmsPartitions and retrieve
from HMS the partiions in the result. The partitions can then be added to
addedHmsPartitions list which is the one to process in the following operations
(L1695-1709).
L1687-1692 could be rewritten into something like:
if (hmsPartitionsToAdd.size() != addedHmsPartitions.size()) {
List difference = computeDifference(hmsPartitionsToAdd,
addedHmsPartittions);
addedHmsPartitions.addAll(getPartitionsFromHms(client, tableName,
difference));
}
The names of the functions/variables can be improved but I think you get the
idea.
PS13, Line 1715: Returns a list of partitions retrieved from HMS for each
'hmsPartitionsToAdd' element.
:* - If 'addedHmsPartitions' contains a partition with the
same partition values, no need
:* to get the partition object again from HMS. Use the one
from 'addedHmsPartitions' instead.
:* - Otherwise, get the new Partition object directly from
HMS.
:*/
: private List getPartitionsFromHMS(MetaStoreClient
msClient, TableName tableName,
: List hmsPartitionsToAdd, List
addedHmsPartitions)
This function and the way it is used is kind of confusing. What you essentially
need is to compute the difference (set) between hmsPartitionsToAdd and
addHmsPartitions and then retrieve these partitions from hms.
http://gerrit.cloudera.org:8080/#/c/4144/13/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java:
PS13, Line 205: // Add multiple partitions.
: AnalyzesOk("alter table functional.alltypes add " +
: "partition(year=2050, month=10) " +
: "partition(year=2050, month=11) " +
: "partition(year=2050, month=12)");
: // Duplicate partition specifications.
: AnalysisError("alter table functional.alltypes add " +
: "partition(year=2050, month=10) " +
: "partition(year=2050, month=11) " +
: "partition(Month=10, YEAR=2050)",
: "Duplicate partition spec: (month=10, year=2050)");
: AnalysisError("alter table functional.alltypes add if not
exists " +
: "partition(year=2050, month=10) " +
: "partition(year=2050, month=11) " +
: "partition(Month=10, YEAR=2050)",
: "Duplicate partition spec: (month=10, year=2050)");
: AnalysisError("alter table functional.alltypes add " +
: "partition(year=2050, month=10) " +
: "partition(year=2050, month=11) " +
: "partition(Month=10, YEAR=2050) location " +
:
"'hdfs://localhost:20500/test-warehouse/alltypes/year=2010/month=10'",
: "Duplicate partition spec: (month=10, year=2050)");
:
: // Multiple partitions with locations and caching
: AnalyzesOk("alter table functional.alltypes add " +
: "partition(year=2050, month=10) location " +
: "'/test-warehouse/alltypes/y2050m10' cached in
'testPool' " +
: "partition(year=2050, month=11) location " +
:
"'hdfs://localhost:20500/test-warehouse/alltypes/y2050m11' " +
: "cached in 'testPool' with replication = 7 "+
: "partition(year=2050, month=12) location " +
: "'file:///test-warehouse/alltypes/y2050m12' uncached");
: // One of the partitions points to an invalid URI
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#13). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py 15 files changed, 716 insertions(+), 206 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/13 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#13). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py 15 files changed, 716 insertions(+), 206 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/13 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 12: (10 comments) http://gerrit.cloudera.org:8080/#/c/4144/12/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: PS12, Line 1635: add partitions not yet in catalog cache and HMS to HMS > That is not a conflict resolution process, this is expected to happen by de Correct, I fixed the comment. PS12, Line 1657: spec > remove Done Line 1683: > Here you may want to add a brief comment explaining that if if_not_exists i Done PS12, Line 1685: addedHmsPartitions > How do you know that the order of the partitions as returned by the add_par I checked the implementation of add_partitions() (in MetaStoreClient.java) and add_partitions_core() (in MetaStore.java) and found that add_partitions() returns partitions in the same order as it receives them. On the other hand it is not documented anywhere, so probably it is not safe to take this behavior for granted. I moved back to using the map. PS12, Line 1718: hmsPartitions > hmsPartitionsToAdd Done Line 1720: Map, Partition> addedPartitionMap = > Add a preconditions check that hmsPartitions.size() > addedHmsPartitions.si Done PS12, Line 1726: p > nit: Plz refrain from using short names for variables that show up several Done PS12, Line 1746: Store > remove Done PS12, Line 1747: 'partitionCachingOpMap' maps lists of partition values to their corresponding :* HDFS caching ops. > Comment is not up-to-date. Moved back to using partitionCachingOpMap map. PS12, Line 1758: cacheHmsPartitions > maybe rename to 'hmsPartitionsToCache'? 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: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 12: (10 comments) http://gerrit.cloudera.org:8080/#/c/4144/12/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: PS12, Line 1635: add partitions not yet in catalog cache and HMS to HMS That is not a conflict resolution process, this is expected to happen by default. You may want to focus on the special cases here: 1. partition exists in cache, ignore 2. partition exists in HMS but not in cache, reload partition from HMS Also remove the comment about HDFS caching, it's not part of resolving conflicts, it's the expected behavior, no? PS12, Line 1657: spec remove Line 1683: Here you may want to add a brief comment explaining that if if_not_exists is true, we may fail to add all the partitions to HMS because some of them may already exist there. So, we need to load in the catalog the partitions that already exist in HMS but aren't in the catalog. PS12, Line 1685: addedHmsPartitions How do you know that the order of the partitions as returned by the add_partitions call is the same as the order of the partitions and their associated cache op specified it in L1668? You can keep the map that you initially had, my original comment was that you don't need to populate it with all the partitions, just with those that have a cache op specified. PS12, Line 1718: hmsPartitions hmsPartitionsToAdd Line 1720: Map, Partition> addedPartitionMap = Add a preconditions check that hmsPartitions.size() > addedHmsPartitions.size(). PS12, Line 1726: p nit: Plz refrain from using short names for variables that show up several statements away from the place they are defined. For example, using 'ap' in L1722 is ok. PS12, Line 1746: Store remove PS12, Line 1747: 'partitionCachingOpMap' maps lists of partition values to their corresponding :* HDFS caching ops. Comment is not up-to-date. PS12, Line 1758: cacheHmsPartitions maybe rename to 'hmsPartitionsToCache'? -- 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: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#12). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py 15 files changed, 715 insertions(+), 206 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/12 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#12). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py 15 files changed, 715 insertions(+), 206 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/12 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has uploaded a new patch set (#11). Change subject: IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION .. IMPALA-1670,IMPALA-4141: Support multiple partitions in ALTER TABLE ADD PARTITION Just like Hive, Implala should support multiple partitions in ALTER TABLE ADD PARTITION statements. The syntax is as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec1 [location_spec1] [cache_spec1] PARTITION partition_spec2 [location_spec2] [cache_spec2] ... Grammar was modified to handle the new syntax. Introduced PartitionParams class to capture the repeatable part of the statement. TPartitionParams is the name of the corresponding thrift class. AlterTableAddPartitionStmt and CatalogOpExecutor classes were also modified to work with a list of partitions. Duplicate partition specs are rejected in AlterTableAddPartitionStmt.analyze(). Added FE, E2E and integration tests. Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/com/cloudera/impala/analysis/PartitionParams.java M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java M fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java M fe/src/test/java/com/cloudera/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_partition_metadata.py 15 files changed, 715 insertions(+), 206 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/4144/11 -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
