Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24693 )
Change subject: IMPALA-15236: Expose HBO match provenance ...................................................................... Patch Set 7: (5 comments) Thanks for updating the patch! The new annotation looks good to me. http://gerrit.cloudera.org:8080/#/c/24693/7/fe/src/main/java/org/apache/impala/planner/CanonicalizationStrategy.java File fe/src/main/java/org/apache/impala/planner/CanonicalizationStrategy.java: http://gerrit.cloudera.org:8080/#/c/24693/7/fe/src/main/java/org/apache/impala/planner/CanonicalizationStrategy.java@68 PS7, Line 68: carries none nit: isn't it EXPR_REWRITE? http://gerrit.cloudera.org:8080/#/c/24693/7/fe/src/main/java/org/apache/impala/planner/CanonicalizationStrategy.java@69 PS7, Line 69: data nit: IMO, the strategy controls the HBO key string which matches queries, e.g. EXPR_REWRITE matches equivalent queries, IGNORE_PARTITION_CONSTANTS matches queries that only differ in partition equality predicates. Data (sizes) matching is done in finding similar runs in the HBO value lists where each run has the input data stats. E.g. if the same query runs twice but in the second run the underlying table/partition is overwritten with double-size data, the HBO key of EXPR_REWRITE matches but the input stats of runs won't match. In short, HBO key string (and canonicalization strategy) matches queries, and historical runs similarity matches data sizes. Here we can comment that strategy matches equivalent queries don't need caveats. http://gerrit.cloudera.org:8080/#/c/24693/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java File fe/src/main/java/org/apache/impala/planner/PlanNode.java: http://gerrit.cloudera.org:8080/#/c/24693/4/fe/src/main/java/org/apache/impala/planner/PlanNode.java@490 PS4, Line 490: if (hboMatch_ != null && queryOptions.isEnable_explain_hbo()) { > Makes sense for tooling, and I would rather check the shape with you than p I'd prefer cardinality_ / cardinalityBeforeHbo_ for simplicity. http://gerrit.cloudera.org:8080/#/c/24693/7/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/24693/7/fe/src/main/java/org/apache/impala/service/Frontend.java@2145 PS7, Line 2145: : explainStr); This looks awkward.. To simplify the code, we can always show the HBO matching details when explain_level >= 2. Will that break your tool for example? https://github.com/alexandrefimov/Query-Doctor http://gerrit.cloudera.org:8080/#/c/24693/7/tests/query_test/test_hbo.py File tests/query_test/test_hbo.py: http://gerrit.cloudera.org:8080/#/c/24693/7/tests/query_test/test_hbo.py@99 PS7, Line 99: 'explain_level': 2 The query is not an EXPLAIN. I think we don't need to set explain_level here. Then we can use "self.execute_query(exact_query)" directly since self.execute_query() uses self.client which already has these options set on L82. -- To view, visit http://gerrit.cloudera.org:8080/24693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d2deaaf78a2a41353454ba634a247d8c69825bf Gerrit-Change-Number: 24693 Gerrit-PatchSet: 7 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Thu, 27 Aug 2026 05:18:12 +0000 Gerrit-HasComments: Yes
