Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24153 )

Change subject: IMPALA-14597: Initial HBO support
......................................................................


Patch Set 1:

(21 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/common/global-flags.cc@231
PS1, Line 231:     "Two runs are considered similar if the relative difference 
is within this threshold. "
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/common/global-flags.cc@244
PS1, Line 244:     "testing). Higher values allow more concurrent access but 
may increase memory overhead.");
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/service/impala-server.cc@1456
PS1, Line 1456: vector<const TPlanNode*> 
ImpalaServer::GetPlanNodesWithHboKeys(const TExecRequest& exec_request) {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/be/src/service/impala-server.cc@1458
PS1, Line 1458:   for (const TPlanExecInfo& plan_exec_info: 
exec_request.query_exec_request.plan_exec_info) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/common/thrift/HBO.thrift
File common/thrift/HBO.thrift:

http://gerrit.cloudera.org:8080/#/c/24153/1/common/thrift/HBO.thrift@56
PS1, Line 56:   // List of plan node execution stats with their corresponding 
HBO hash strings for different
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/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/24153/1/fe/src/main/java/org/apache/impala/planner/CanonicalizationStrategy.java@52
PS1, Line 52:   // Lower values indicate higher accuracy, higher values 
indicate more aggressive matching
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/planner/ExprCanonicalizer.java
File fe/src/main/java/org/apache/impala/planner/ExprCanonicalizer.java:

http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/planner/ExprCanonicalizer.java@36
PS1, Line 36:  * - IGNORE_PARTITION_CONSTANTS: Additionally removes constants 
from partition column predicates
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/planner/ExprCanonicalizer.java@38
PS1, Line 38:  * Note: Most expression normalization (e.g., `1 = a` → `a = 1`, 
`(a=1 OR a=2)` → `a IN (1,2)`)
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/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/24153/1/fe/src/main/java/org/apache/impala/planner/PlanNode.java@1006
PS1, Line 1006:    * Generates a single hash string using the specified stats 
type and canonicalization strategy.
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/planner/PlanNode.java@1021
PS1, Line 1021:    * statistics type. Each string corresponds to a different 
canonicalization strategy, ordered
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/planner/PlanNode.java@1033
PS1, Line 1033:       LOG.info("{} HBO Strategy {} statsType {} hash: {}", 
getDisplayLabel(), strategy, statsType, hashString);
line too long (111 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/HistoricalStats.java
File fe/src/main/java/org/apache/impala/service/HistoricalStats.java:

http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@188
PS1, Line 188:           LOG.debug("HBO cache miss for {} using strategy level 
{} (key: {}, scanInputRows: {}). No similar run",
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@192
PS1, Line 192:         LOG.debug("HBO cache miss for {} using strategy level {} 
(key: {}, scanInputRows: {}). Hash key not found",
line too long (115 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java
File fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java:

http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@41
PS1, Line 41:     // String: object header (16) + hash fields (8) + char[] ref 
(8) + char[] header (16) + padding.
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@48
PS1, Line 48:     // ArrayList: object header (16) + size (4) + modCount (4) + 
elementData ref (8) + padding = 40
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@54
PS1, Line 54:     // TPlanNodeRun: object header (16) + isset byte (1) + 
padding (7) + 5 longs (40) + list ref (8) = 72
line too long (105 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@57
PS1, Line 57:     // ArrayList<Long> for scan_input_rows: base (40) + refs 
(8*N) + Long objects (24*N) = 40 + 32*N
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@69
PS1, Line 69:     private static int weighHistoricalStatsValue(String key, 
HistoricalStatsValue<?> value) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24153/1/tests/query_test/test_hbo.py
File tests/query_test/test_hbo.py:

http://gerrit.cloudera.org:8080/#/c/24153/1/tests/query_test/test_hbo.py@30
PS1, Line 30: class TestHBO(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/24153/1/tests/query_test/test_hbo.py@40
PS1, Line 40: a
flake8: W504 line break after binary operator


http://gerrit.cloudera.org:8080/#/c/24153/1/tests/query_test/test_hbo.py@85
PS1, Line 85: d
flake8: E306 expected 1 blank line before a nested definition, found 0



--
To view, visit http://gerrit.cloudera.org:8080/24153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ff60a8bd22c13c0ecad1198934cc96249b1015e
Gerrit-Change-Number: 24153
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Tue, 31 Mar 2026 10:16:00 +0000
Gerrit-HasComments: Yes

Reply via email to