Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24297 )
Change subject: IMPALA-14600: Support HBO for AggregationNode cardinality ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/24297/8/fe/src/main/java/org/apache/impala/analysis/SlotRef.java File fe/src/main/java/org/apache/impala/analysis/SlotRef.java: http://gerrit.cloudera.org:8080/#/c/24297/8/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@341 PS8, Line 341: if (name == null || name.isEmpty()) return name; nit: StringUtils.isEmpty(name) http://gerrit.cloudera.org:8080/#/c/24297/8/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@345 PS8, Line 345: if (open >= 0) return name.substring(open); Ooh, weird situation: Is it even possible to have a naked backquote at the end? I wouldn't think this would compile. Maybe a Preconditions check here would be better for "open >=0"? Or are you worried you might break something in the wild? Editing my own comment: Are we worried about a backslash character like "\`" that might be isolated? Geez, that opens up a can of worms because there could be two escaped backquote characters. Might be best to just use a Preconditions if that is our worry. http://gerrit.cloudera.org:8080/#/c/24297/8/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java: http://gerrit.cloudera.org:8080/#/c/24297/8/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@118 PS8, Line 118: public boolean isCardinalityPreserving() { return true; } Just want to make sure here... If the analytic expression changes the sum expression and the aggregate node has a having clause that looks at the sum expression, does this affect HBO? -- To view, visit http://gerrit.cloudera.org:8080/24297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0fafaf9d827f3bf533b1af7e62fdb2303c126ce Gerrit-Change-Number: 24297 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Wed, 01 Jul 2026 01:41:36 +0000 Gerrit-HasComments: Yes
