lipeng...@sensorsdata.cn has posted comments on this change. ( http://gerrit.cloudera.org:8080/18574 )
Change subject: IMPALA-11279: Optimize plain count(*) queries for Iceberg tables ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@464 PS2, Line 464: public static TResultSet getIcebergTotalRecordsProp(FeIcebergTable table) { > how about `s/feTable/table/` to keep consistent with other functions? Done http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@486 PS2, Line 486: } : // When total is 0, no optimization is still very fast > Seems like the comment explains the opposite of the following code block, w Done http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2158 PS2, Line 2158: optimizeQueryForIcebergTable(planCtx, analysisResult); > I think we can move this to #2027 since we need not to create the plan Because to consider retrieving TOTAL_RECORDS_PROP from the Iceberg Snapshot summary failure, creating a query plan is required. http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2213 PS2, Line 2213: if (stmt.hasGroupByClause()) return; > I think the statement can't has 'havingClause' too and it's better to add s Great advice http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2228 PS2, Line 2228: if (!funcCallExpr.getFnName().getFunction().equ > how about also optimizing the case of `select count(constant) from tbl` sin Thx for your cr. As in the 'https://gerrit.cloudera.org/#/c/18574/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-is-null-predicate-push-down.test', 'select count(constant) from TBL' is rewritten to 'count(*)', so this optimization works. The relevant code is`org.apache.impala.rewrite.NormalizeCountStarRule`. http://gerrit.cloudera.org:8080/#/c/18574/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2459 PS2, Line 2459: request.getTable_name()); > I noticed that `optimizeQueryForIcebergTable()` has guaranteed that the tab Be consistent with other functions named "doGetXXX" and double check table type. -- To view, visit http://gerrit.cloudera.org:8080/18574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e9c48bbba7ab2320fa80915e7001ce54f1ef6d9 Gerrit-Change-Number: 18574 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward <lipeng...@sensorsdata.cn> Gerrit-Reviewer: Anonymous Coward <lipeng...@sensorsdata.cn> Gerrit-Reviewer: Gergely Fürnstáhl <gfurnst...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jian Zhang <zjsar...@gmail.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Xianqing He <hexianqing...@126.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 31 May 2022 05:28:23 +0000 Gerrit-HasComments: Yes
