Amos Bird has posted comments on this change. Change subject: IMPALA-4033,IMPALA-4105: Improvements of partition DDL. ......................................................................
Patch Set 1: (8 comments) Ah, sorry. I saw there are other commits targeting more than one issues. What should I do now? abandon this? http://gerrit.cloudera.org:8080/#/c/5137/1/bin/start-catalogd.sh File bin/start-catalogd.sh: Line 54: export JAVA_TOOL_OPTIONS="-agentlib:jdwp=transport=dt_socket,address=${JVM_DEBUG_PORT},server=y,suspend=${JVM_SUSPEND} ${JAVA_TOOL_OPTIONS}" > what's the motivation for this change? It's for remote debuging catalog's jvm. This is like what start-impalad.sh does. http://gerrit.cloudera.org:8080/#/c/5137/1/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: Line 192: > replace "some" with "the" Done http://gerrit.cloudera.org:8080/#/c/5137/1/fe/src/main/java/org/apache/impala/analysis/PartitionSet.java File fe/src/main/java/org/apache/impala/analysis/PartitionSet.java: Line 163 > this fix needs a test Done http://gerrit.cloudera.org:8080/#/c/5137/1/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java: Line 73 > The toLowerCase() in this file seem correct to me, why did you remove them? Done http://gerrit.cloudera.org:8080/#/c/5137/1/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java: Line 40: // not null when refreshing some partition(s) > replace "some" with "a set of" Done Line 47: this.tableName_ = name; > can remove 'this' everywhere in this constructor Done Line 74: TableRef tableRef = new TableRef(tableName_.toPath(), null, Privilege.ALTER); > This is a pretty drastic behavioral change. Look at the comment on L62 foll yeah, it's really a dillema here. My rough idea is to loop in the old partitionSpec as a special case. http://gerrit.cloudera.org:8080/#/c/5137/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: Line 1256: hmsPartition = msClient.getHiveClient().getPartition( > use the bulk getPartitions() API to get all partitions in one RPC Done -- To view, visit http://gerrit.cloudera.org:8080/5137 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f4e46ec0a63b46e485141290268d019c3dd15c7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Amos Bird <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Amos Bird <[email protected]> Gerrit-HasComments: Yes
