Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17316 )
Change subject: IMPALA-10658: LOAD DATA INPATH silently fails between HDFS and Azure ABFS ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/17316/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java: http://gerrit.cloudera.org:8080/#/c/17316/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@584 PS1, Line 584: isPathOnFileSystem This patch adds requirement for checking existence. So we can only used it as boolean sameFileSystem = isPathOnFileSystem(sourceFile, destFs); But not boolean sameFileSystem = isPathOnFileSystem(dest, sourceFs); Maybe "isPathExistsOnFileSystem" is a better name. http://gerrit.cloudera.org:8080/#/c/17316/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@588 PS1, Line 588: return fs.exists(path); > My idea was that 'path.equals(qp)' is the fast-path to check if the path is I think it'd be good if we have unit tests in FileSystemUtilTest on this method for all FileSystem implementations. Maybe we can borrow some of test codes from the hadoop repo, e.g. https://github.com/apache/hadoop/blob/d20109c171460f3312a760c1309f95b2bf61e0d3/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemE2E.java#L179 (not mean to do this in this patch. We can add tests in a separate patch if it requires lots of changes) http://gerrit.cloudera.org:8080/#/c/17316/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@590 PS1, Line 590: // Path is not on fs. Maybe it's worth some logs here. So we know what path (whether qualified or not) is checked in debugging. -- To view, visit http://gerrit.cloudera.org:8080/17316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id807e8a200b83283a09d3a917185cabab930017d Gerrit-Change-Number: 17316 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 20 Apr 2021 09:19:41 +0000 Gerrit-HasComments: Yes
