Marcel Kornacker has posted comments on this change. Change subject: IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems ......................................................................
Patch Set 28: (3 comments) http://gerrit.cloudera.org:8080/#/c/2574/28/fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java File fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java: Line 144: * file. The file is moved (renamed) to the new location unless the source and update comment, this isn't quite true anymore (you also don't rename if the files are in different file systems). Line 175: (destIsDfs && sameFileSystem) ? sameEncryptionZone : sameFileSystem; i still find it a bit hard to follow when exactly a rename is possible. there seem to be two cases: 1) destIsDfs && sameFileSystem && sameEncryptionZone, 2) !destIsDfs && sameFileSystem if you break this out into explicit if statements, you can add a comment to each case (e.g., which one is the s3 case). Line 194: LOG.info(String.format("Copying '%s' to '%s' between filesystems.", preconditions.checkstate(!sameFileSystem) -- 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: 28 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
