Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12282 )
Change subject: IMPALA-6050: Query profiles should indicate storage layer(s) used ...................................................................... Patch Set 4: (5 comments) I'm re-running the full suite of tests to ensure there are no test failures introduced by rebasing this patch. Will post an update when the tests complete. http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/FsType.java File fe/src/main/java/org/apache/impala/catalog/FsType.java: http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/FsType.java@25 PS4, Line 25: * Represents the type of filesystem being used. Typically associated with a > This is somewhat redundant with the is*Filesystem() logic in FileSystemUtil Moved it to FileSystemUtil.java so all the code is in one place. The use of FsType is slightly different from the is*FileSystem methods. FsType allows grouping multiple FileSystems into a single logic type, which is useful for cloud stores like ADLS because it supports multiple file system connectors: abfs, abfss, adl. It also also mapping a filesystem scheme to a more user friendly string - e.g. s3a to S3. I added some more Javadocs to make it clearer when to use FsType vs. when to use is*FileSystem. http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java: http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@89 PS4, Line 89: return FsType.LOCAL; > See comment on LocalFsTable Done http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@276 PS4, Line 276: return FsType.LOCAL; > Is this right? I think this is a "Local" table in the sense that it's the " Yeah, thanks for pointing that out. Updated the implementation so its pretty much the same as what HdfsTable / HdfsPartition does. http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/test/java/org/apache/impala/planner/ExplainTest.java File fe/src/test/java/org/apache/impala/planner/ExplainTest.java: http://gerrit.cloudera.org:8080/#/c/12282/4/fe/src/test/java/org/apache/impala/planner/ExplainTest.java@81 PS4, Line 81: partitions.add(createMockHdfsPartition("abfs://dummy...@dummy-account.dfs.core" > This is cool Thanks :) http://gerrit.cloudera.org:8080/#/c/12282/4/testdata/workloads/functional-planner/queries/PlannerTest/scan-node-fs-scheme.test File testdata/workloads/functional-planner/queries/PlannerTest/scan-node-fs-scheme.test: http://gerrit.cloudera.org:8080/#/c/12282/4/testdata/workloads/functional-planner/queries/PlannerTest/scan-node-fs-scheme.test@22 PS4, Line 22: S3 partitions=1/0 files=0 size=0B > What's the deal with the 1/0? It's due to the fact that s3_tbl isn't a real table, it's a "test table" created using FrontendTestBase#addTestTable. 't_nopart' in 'insert-sort-by.test' is another example of a table that has the '1/0' behavior. -- To view, visit http://gerrit.cloudera.org:8080/12282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b1b4a1bc1a24e9614e3b4dc5a61dc96d075d1c3 Gerrit-Change-Number: 12282 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 09 Apr 2019 21:15:16 +0000 Gerrit-HasComments: Yes
