impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems
Date Fri, 29 Apr 2016 05:02:06 GMT
Sailesh Mukil has posted comments on this change.

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

Patch Set 23:

File be/src/util/

Line 81: This is a helper function and is not declared in the header file. It returns
> Returns

Line 88: file
> Let's check for "file:" since there could conceivable be a scheme called fi

Line 88: file
> And please add a test case for this (e.g. file:blah vs file2:blah should be

Line 90: after_scheme + 3
> this is missing the dereference.  please add a test case to your unit test 

Line 94: after_authority - path
> is this off by one? i.e. hdfs://n/ should return 7, right?
hdfs://n/ should return '8':
4 for 'hdfs', 3 for '://' and 1 for 'n'. (4 + 3 + 1)

At this point after_authority would point to the '/' after 'n'.

I've added a test case for hdfs://namenode, hdfs://namenode/ and it works as expected, i.e.
they match.

To view, visit
To unsubscribe, visit

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

View raw message