impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Amos Bird (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4033,IMPALA-4105: Improvements of partition DDL.
Date Sat, 19 Nov 2016 02:26:34 GMT
Amos Bird has posted comments on this change.

Change subject: IMPALA-4033,IMPALA-4105: Improvements of partition DDL.

Patch Set 1:


Ah, sorry. I saw there are other commits targeting more than one issues. What should I do
now? abandon this?
File bin/

Line 54:   export JAVA_TOOL_OPTIONS="-agentlib:jdwp=transport=dt_socket,address=${JVM_DEBUG_PORT},server=y,suspend=${JVM_SUSPEND}
> what's the motivation for this change?
It's for remote debuging catalog's jvm. This is like what does.
File common/thrift/CatalogService.thrift:

Line 192: 
> replace "some" with "the"
File fe/src/main/java/org/apache/impala/analysis/

Line 163
> this fix needs a test
File fe/src/main/java/org/apache/impala/analysis/

Line 73
> The toLowerCase() in this file seem correct to me, why did you remove them?
File fe/src/main/java/org/apache/impala/analysis/

Line 40:   // not null when refreshing some partition(s)
> replace "some" with "a set of"

Line 47:     this.tableName_ = name;
> can remove 'this' everywhere in this constructor

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
File fe/src/main/java/org/apache/impala/catalog/

Line 1256:             hmsPartition = msClient.getHiveClient().getPartition(
> use the bulk getPartitions() API to get all partitions in one RPC

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f4e46ec0a63b46e485141290268d019c3dd15c7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Amos Bird <>
Gerrit-HasComments: Yes

View raw message