impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) IMPALA-2840: Don't store table location in partition location
Date Tue, 15 Mar 2016 21:44:14 GMT
Jim Apple has posted comments on this change.

Change subject: IMPALA-2840: Don't store table location in partition location

Patch Set 4:

File fe/src/main/java/com/cloudera/impala/catalog/

Line 204: public class Location
> Why is this class here instead of HdfsPartition?
I mentioned this in another comment, but it may have been obscured.

It simplifies the code. On reason is that this class doesn't actually use anything from HdfsPartition.

Line 208:     // partition location.
> You also need to comment what happens if the partition location does not fo
I was attempting to do so. The last N directories are stripped, whether they are partition
directories are not.

Line 237: // Combine with some random constants (from uuidgen -r) to make Locations with
        :       // identical 'suffix_'s but different 'prefix_'s hash to different values.
        :       final long m = 0xc6bfaf3a929b49e1L;
        :       final long a = 0xa9591152f59b46d7L;
        :       long long_prefix = prefix_;
        :       long_prefix = (long_prefix * m + a) >>> 32;
        :       int prefix_hash = (int)long_prefix;
        :       return suffix_.hashCode() ^ prefix_hash;
> can't you just use toString().hashCode()?
simpler, but probably more expensive. Do you think it's worth it?

Line 275: What is left is the prefix.
> So, for test-warehouse/functional.db/dimitris/is/great/partition_dir_for_al
I thought that was what we had discussed? What do you think should happen instead?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c67b6ce0f83de2f5277a528a9ce67e47d638adb
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Jim Apple <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-HasComments: Yes

View raw message