Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10543 )

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
......................................................................


Patch Set 12:

(4 comments)

Looks pretty close, just some clarifying questions.

http://gerrit.cloudera.org:8080/#/c/10543/12/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/10543/12/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1227
PS12, Line 1227: partitions != null
Isn't this more like a precondition? Also you could just say

return partitions.size() > 1;


http://gerrit.cloudera.org:8080/#/c/10543/12/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/10543/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2245
PS12, Line 2245:             
((HdfsTable)tbl).isLocationSharedWithOtherPartitions(part)) {
Should we check if the table is "managed" / "external" in Hive? I'm guessing 
this only applies to managed tables since HMS can't drop data partitions for 
external tables.


http://gerrit.cloudera.org:8080/#/c/10543/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2247
PS12, Line 2247:
Could you format the string to add the partition data here.
Also should we include the the following information?

- Location that is causing the conflict
- List and count(or atleast a subset of them) of conflicting partitions? 
Joiner.On(list.subset())...

Seeing this error message, the next steps for the user would be to probably 
figure out the conflicting partitions, may be we should include that 
information?


http://gerrit.cloudera.org:8080/#/c/10543/12/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:

http://gerrit.cloudera.org:8080/#/c/10543/12/tests/metadata/test_partition_metadata.py@206
PS12, Line 206:     self.client.execute("insert into table %s partition(j=3) 
select 3" % FQ_TBL_NAME)
May be just call refresh instead?



--
To view, visit http://gerrit.cloudera.org:8080/10543
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 12
Gerrit-Owner: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Mon, 18 Jun 2018 23:46:39 +0000
Gerrit-HasComments: Yes

Reply via email to