Marcel Kornacker has posted comments on this change.

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


Patch Set 27:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/2574/27/common/thrift/ImpalaInternalService.thrift
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?


http://gerrit.cloudera.org:8080/#/c/2574/27/fe/src/main/java/com/cloudera/impala/analysis/LoadDataStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/LoadDataStmt.java:

Line 153:               "INPATH location '%s' cannot contain non-hidden 
subdirectories.", sourceDataPath_));
long line (i know, not your fault)


http://gerrit.cloudera.org:8080/#/c/2574/27/fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java
File fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java:

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

not a fun name either way


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


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

Path = ... \n<4 spaces>? ... : ...


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


Line 189:       LOG.info(String.format(
precede w/ preconditions.checkstate(!doRename))


Line 193:       LOG.info(String.format("Copying '%s' to '%s' between 
filesystems.",
couldn't this be !isDestDfs && sameFileSystem?


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


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

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

Reply via email to