Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18023 )
Change subject: IMPALA-7942: Add query hints for cardinalities and selectivities ...................................................................... Patch Set 9: (6 comments) Looks good to me too. I have only reviewed the code section and will continue tomorrow to cover the tests. Thanks a lot for the rework, Wang Sheng! http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/analysis/Predicate.java File fe/src/main/java/org/apache/impala/analysis/Predicate.java: http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/analysis/Predicate.java@78 PS9, Line 78: is nit. should be a double value in (0, 1]. http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/analysis/TableRef.java File fe/src/main/java/org/apache/impala/analysis/TableRef.java: http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/analysis/TableRef.java@554 PS9, Line 554: // This hint is only valid for hdfs table nit. for this HDFS table reference. On paper, one can assign different # rows for distinct table refs referencing the same table. For example, select * from T <+ TABLE_NUM_ROWS = 1000 >, T <+ TABLE_NUM_ROWS = 10 >; http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/analysis/TableRef.java@555 PS9, Line 555: valueOf It seems we need to handle NumberFormatException thrown from this method. For example, TABLE_NUM_ROWS = 'abc'. http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/planner/ScanNode.java@81 PS9, Line 81: Reserve nit. Store http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/planner/ScanNode.java@81 PS9, Line 81: , nit. "." http://gerrit.cloudera.org:8080/#/c/18023/9/fe/src/main/java/org/apache/impala/planner/ScanNode.java@82 PS9, Line 82: if table does not have stats, or corrupt stats. Otherwise, Impala will : // use table original stats. I wonder if this is still correct. I thought the hint can be used to override any stats, valid or invalid. -- To view, visit http://gerrit.cloudera.org:8080/18023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2776b9bbd878b8a21d9c866b400140a454f59e1b Gerrit-Change-Number: 18023 Gerrit-PatchSet: 9 Gerrit-Owner: wangsheng <[email protected]> Gerrit-Reviewer: Amogh Margoor <[email protected]> Gerrit-Reviewer: Fucun Chu <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Tue, 06 Sep 2022 21:39:02 +0000 Gerrit-HasComments: Yes
