Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24693 )

Change subject: IMPALA-15236: Expose HBO match provenance
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/24693/3/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/24693/3/be/src/service/query-options.h@54
PS3, Line 54: CTE_THRESHOLD
This should be ENABLE_EXPLAIN_HBO now.


http://gerrit.cloudera.org:8080/#/c/24693/3/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/24693/3/common/thrift/Query.thrift@837
PS3, Line 837: ;
nit: don't need semicomma


http://gerrit.cloudera.org:8080/#/c/24693/3/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/3/fe/src/main/java/org/apache/impala/planner/PlanNode.java@490
PS3, Line 490:             
.append(PrintUtils.printEstCardinality(cardinalityBeforeHbo_))
Showing the accuracy rate would be helpful for external tools so they don't 
need to parse the human-readable numbers like "7.30K".

 (1 - |cardinality_ - cardinalityBeforeHbo_| / cardinalityBeforeHbo_) * 100

We will be interesting in nodes with low accuracy rates which means HBO 
corrects the estimate a lot. If cardinalityBeforeHbo_ is -1, we can show "N/A". 
Otherwise, show a string like "85%".


http://gerrit.cloudera.org:8080/#/c/24693/2/tests/query_test/test_hbo.py
File tests/query_test/test_hbo.py:

http://gerrit.cloudera.org:8080/#/c/24693/2/tests/query_test/test_hbo.py@63
PS2, Line 63:       # To avoid the test being fragile, we only compare the 
cardinality lines.
            :       actual_cardinality_lines = [line for line in 
actual_plan_lines
            :                                   if 'cardinality' in line]
            :       expected_cardinality_lines = [line for line in 
expected_plan_lines
            :                                     if 'cardinality' in line]
> The details line carries the hash key, which changes whenever the key forma
Fine to me. The original cardinality could also change.


http://gerrit.cloudera.org:8080/#/c/24693/3/tests/query_test/test_hbo.py
File tests/query_test/test_hbo.py:

http://gerrit.cloudera.org:8080/#/c/24693/3/tests/query_test/test_hbo.py@92
PS3, Line 92: {'explain_level': 2, 'enable_explain_hbo': True}
I thought these just impact EXPLAIN outputs. Do they also impact the plan shown 
in query profile? I think the plan in the query profile should always be the 
most verbose version, i.e. with HBO details lines, without setting any of these 
query options.


http://gerrit.cloudera.org:8080/#/c/24693/3/tests/query_test/test_hbo.py@106
PS3, Line 106: ?!unavailable
Will the original estimate be unavailable?



--
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: 3
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: Sun, 23 Aug 2026 02:34:49 +0000
Gerrit-HasComments: Yes

Reply via email to