impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems
Date Wed, 20 Apr 2016 20:28:16 GMT
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 <sailesh@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message