Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/8238 )
Change subject: IMPALA-4524: Batch calls to ALTER TABLE...ADD PARTITION. ...................................................................... Patch Set 3: (2 comments) Re testing, a simple python test that adds at least MAX_PARTITION_UPDATES_PER_SEC + 1 partitions and then validates that they were actually added in the HMS (e.g. by doing show partitions) is sufficient. If you want you can convert the analysis test in AnalyzeDDLTest.java to a positive test but it is kind of redundant if you add the python test I just mentioned. http://gerrit.cloudera.org:8080/#/c/8238/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/8238/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@a1954 PS3, Line 1954: Why did you remove this? http://gerrit.cloudera.org:8080/#/c/8238/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1974 PS3, Line 1974: if (hmsSublist.size() != addedHmsPartitions.size()) { : List<Partition> difference = computeDifference(hmsSublist, : addedHmsPartitions); : addedHmsPartitions.addAll( : getPartitionsFromHms(msTbl, msClient, tableName, difference)); : } : : for (Partition partition: addedHmsPartitions) { : // Create and add the HdfsPartition to catalog. Return the table object with an : // updated catalog version. : addHdfsPartition(tbl, partition); : } I think you can refactor this so that you make only one call to getPartitionsFromHms() outside the for loop. -- To view, visit http://gerrit.cloudera.org:8080/8238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95f8221ff08c0f126f951f7d37ff5e57985f855f Gerrit-Change-Number: 8238 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Comment-Date: Thu, 12 Oct 2017 16:59:32 +0000 Gerrit-HasComments: Yes
