[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. IMPALA-7140 (part 4): support creating descriptors for FS tables This adds the relevant methods to convert LocalFsTable and LocalFsPartition to thrift descriptors for consumption by the backend. Unfortunately we cannot yet enable the planner tests, since they are checking file counts and sizes as part of the explain output, and we haven't yet implemented file info fetching in the LocalCatalog. However, I was able to manually test this change by starting an impalad with --use_local_catalog, connecting to it from the shell, and running various EXPLAIN SELECT queries against tpch and functional tables. The explain output is more or less as expected with the exception of missing file info. Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Reviewed-on: http://gerrit.cloudera.org:8080/10735 Reviewed-by: Vuk Ercegovac Tested-by: Impala Public Jenkins --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java 11 files changed, 227 insertions(+), 105 deletions(-) Approvals: Vuk Ercegovac: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 22 Jun 2018 21:11:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2729/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 22 Jun 2018 17:47:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 22 Jun 2018 17:33:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10735/3/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/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@175 PS3, Line 175: /*includeFileDesc=*/false, > I inlined this call into the above function since it was the only call site actually was wrong about this, thrift *descriptors* never include file info. But I think that's already implied by the name "descriptor". Let me know if you want me to add a comment though. -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 22 Jun 2018 17:24:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10735 to look at the new patch set (#4). Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. IMPALA-7140 (part 4): support creating descriptors for FS tables This adds the relevant methods to convert LocalFsTable and LocalFsPartition to thrift descriptors for consumption by the backend. Unfortunately we cannot yet enable the planner tests, since they are checking file counts and sizes as part of the explain output, and we haven't yet implemented file info fetching in the LocalCatalog. However, I was able to manually test this change by starting an impalad with --use_local_catalog, connecting to it from the shell, and running various EXPLAIN SELECT queries against tpch and functional tables. The explain output is more or less as expected with the exception of missing file info. Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java 11 files changed, 227 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/10735/4 -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java File fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java: http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@295 PS3, Line 295: THdfsPartition > naming does not align with the switch from 'hdfs' to 'fs'. pls add a todo t added a TODO on the Thrift file http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java File fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java: http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java@160 PS3, Line 160: getFilteredHmsParameters > not for this change, but wondering if hms details need to be exposed this h I separated this API rather than filtering at the call site because I'd like to be able to push down the filtering all the way to what we fetch from HMS after it implements something like https://issues.apache.org/jira/browse/HIVE-19715 . We might want to _always_ remove the incremental stats from the parameters and add an entirely separate getter which returns them in decoded format for memory usage - I think Parna is working on something like that. But didn't want to expand this change. I'll add a TODO about this. http://gerrit.cloudera.org:8080/#/c/10735/3/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/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@87 PS3, Line 87: 0; > interesting that this one can be 0, L73 returns empty but flipping L81 to t yea, I was surprised too. It's temporary at least until the next patch in this series. http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@98 PS3, Line 98: -1 > what's the -1? Done http://gerrit.cloudera.org:8080/#/c/10735/3/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/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@175 PS3, Line 175: > add a comment that a slim hdfs table is being constructed (no hdfs info, no I inlined this call into the above function since it was the only call site, and the above function name makes it clearer it's just a descriptor. The "no HDFS info" bit is just temporary until the next patch, not something inherent about the design here (it will include the descriptors) http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@176 PS3, Line 176: get > nit: build instead of get? obviated by above -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 22 Jun 2018 16:59:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10735 ) Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java File fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java: http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@295 PS3, Line 295: THdfsPartition naming does not align with the switch from 'hdfs' to 'fs'. pls add a todo to get these into better alignment. http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java File fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java: http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java@160 PS3, Line 160: getFilteredHmsParameters not for this change, but wondering if hms details need to be exposed this high in the abstraction? http://gerrit.cloudera.org:8080/#/c/10735/3/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/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@87 PS3, Line 87: 0; interesting that this one can be 0, L73 returns empty but flipping L81 to true tests out pruning. http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@98 PS3, Line 98: -1 what's the -1? http://gerrit.cloudera.org:8080/#/c/10735/3/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/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@175 PS3, Line 175: add a comment that a slim hdfs table is being constructed (no hdfs info, not incremental stats). http://gerrit.cloudera.org:8080/#/c/10735/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@176 PS3, Line 176: get nit: build instead of get? -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 21 Jun 2018 16:39:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 4): support creating descriptors for FS tables
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10735 to review the following change. Change subject: IMPALA-7140 (part 4): support creating descriptors for FS tables .. IMPALA-7140 (part 4): support creating descriptors for FS tables This adds the relevant methods to convert LocalFsTable and LocalFsPartition to thrift descriptors for consumption by the backend. Unfortunately we cannot yet enable the planner tests, since they are checking file counts and sizes as part of the explain output, and we haven't yet implemented file info fetching in the LocalCatalog. However, I was able to manually test this change by starting an impalad with --use_local_catalog, connecting to it from the shell, and running various EXPLAIN SELECT queries against tpch and functional tables. The explain output is more or less as expected with the exception of missing file info. Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 --- M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java 10 files changed, 223 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/10735/1 -- To view, visit http://gerrit.cloudera.org:8080/10735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4550612eb6d1e3a324f49a9c4d24b048e45d3738 Gerrit-Change-Number: 10735 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
