Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/25001 )
Change subject: IMPALA-15059: Use HBO row counts in Calcite scans ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/25001/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java: http://gerrit.cloudera.org:8080/#/c/25001/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@96 PS1, Line 96: private boolean hboStatsLoaded_; Question: I didn't look at the hbo code closely, but are there cases where the stats would not get loaded in HdfsScanNode? If we're always going to load in stats in HdfsScanNode and we make the other change I suggested below about sharing code, then this should be done at initialization time and no need for non-final variables. But I do want to be sure about that. http://gerrit.cloudera.org:8080/#/c/25001/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@281 PS1, Line 281: for (FeFsPartition partition : fsTable.loadAllPartitions()) { I'm a little concerned about duplication of running this for loop. If I'm not mistaken, these calculations will also be done in HdfsScanNode. Perhaps we can supply these calculations to HdfsScanNode via the ImpalaHdfsScanNode and rearrange HdfsScanNode so it doesn't recalculate if it is already supplied? Would be nice if the calculation was in a shared method that can be called from both places. Any other avoidance of duplication of code would be good too. http://gerrit.cloudera.org:8080/#/c/25001/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdRowCount.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdRowCount.java: http://gerrit.cloudera.org:8080/#/c/25001/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdRowCount.java@56 PS1, Line 56: public Double getRowCount(TableScan scan, RelMetadataQuery mq) { Do we need this method? Can table.getHboRowCount() be called from table.getRowCount()? -- To view, visit http://gerrit.cloudera.org:8080/25001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f530098d34869cda5ff36a06576bb4854ad7f4f Gerrit-Change-Number: 25001 Gerrit-PatchSet: 1 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 05 Oct 2026 18:55:05 +0000 Gerrit-HasComments: Yes
