impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems
Date Sat, 30 Apr 2016 02:29:21 GMT
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems

Patch Set 27:


Thanks Marcel.
File common/thrift/ImpalaInternalService.thrift:

Line 400:   // Fully qualified URI with the scheme to the base directory for this partition.
> what does "with the scheme to the base directory" mean?
it was actually supposed to be "URI with the scheme, to the base directory". But I forgot
to remove "with the scheme" when I added "Fully qualified". Changed it now.
File fe/src/main/java/com/cloudera/impala/analysis/

Line 153:               "INPATH location '%s' cannot contain non-hidden subdirectories.",
> long line (i know, not your fault)
File fe/src/main/java/com/cloudera/impala/common/

Line 88:   private static boolean arePathsInSameEncryptionZone(FileSystem fs, Path p1,
> arePathsInSameHdfsEncryptionZone?

Line 149:    * preserving the the existing file extension. If renameIfAlreadyExists is false,
> the the

Line 157:     Path destFile = destFs.isDirectory(dest) ? new Path(dest, sourceFile.getName())
> better:

Line 167:     boolean isDestDfs = isDistributedFileSystem(destFs);
> destIsDfs

Line 189:
> precede w/ preconditions.checkstate(!doRename))

Line 193:"Copying '%s' to '%s' between filesystems.",
> couldn't this be !isDestDfs && sameFileSystem?
No, that would always make doRename = true, and so this code wouldn't get executed. Do you
think I should make the else into an else if() so it's more readable?

Line 286:    * Returns true iff the filesystem is a S3AFileSystem.
> if the path is on an ...

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I94e15ad67752dce21c9b7c1dced6e114905a942d
Gerrit-PatchSet: 27
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-HasComments: Yes

View raw message