Henry Robinson has posted comments on this change. Change subject: IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems ......................................................................
Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/2574/11/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 774: InsertStmt Nit: Don't refer to a Java-side class here. Just say "INSERT allow writes..." Line 843: mimick 'mimic' http://gerrit.cloudera.org:8080/#/c/2574/11/be/src/util/hdfs-util.cc File be/src/util/hdfs-util.cc: Line 76: bool FilesystemsMatch(const char* path_a, const char* path_b) { : const char* after_scheme_a = strstr(path_a, ":/"); : const char* after_scheme_b = strstr(path_b, ":/"); : : bool path_a_has_scheme = after_scheme_a != NULL; : bool path_b_has_scheme = after_scheme_b != NULL; : // Currently, our defaultFS is HDFS. If there is no scheme present, then that path is a : // HDFS path. : if (!path_a_has_scheme) return IsHdfsPath(path_b); : if (!path_b_has_scheme) return IsHdfsPath(path_a); : : // By this point we know that both the paths have schemes. : bool scheme_a_len = after_scheme_a - path_a; : bool scheme_b_len = after_scheme_b - path_b; : if (scheme_a_len != scheme_b_len) return false; : return strncmp(path_a, path_b, scheme_a_len) == 0; : } Just realised: this won't work if there are two different HDFS filesystems (because you don't check the authority). That's ok, because multiple HDFS filesystems aren't in scope to support - but if it's easy to do the authority check, might as well do so. -- 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: 11 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: 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
