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: (9 comments) Thanks Marcel. 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? 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. 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) Done 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? Done Line 149: * preserving the the existing file extension. If renameIfAlreadyExists is false, an > the the Done Line 157: Path destFile = destFs.isDirectory(dest) ? new Path(dest, sourceFile.getName()) : > better: Done Line 167: boolean isDestDfs = isDistributedFileSystem(destFs); > destIsDfs Done Line 189: LOG.info(String.format( > precede w/ preconditions.checkstate(!doRename)) Done Line 193: LOG.info(String.format("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 ... Done -- 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
