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: (2 comments) Didn't get very far with the code review, got stuck on the first 2 files (and Frontend.java), but figured I should get this out here because if we do change this, it's not a trivial change. http://gerrit.cloudera.org:8080/#/c/24297/8/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/24297/8/common/thrift/Frontend.thrift@789 PS8, Line 789: 28: optional list<i32> node_parent_ids Ok, heh, this comment is gonna be long, and I may be misunderstanding something as well, but here we go... This structure seems a bit off to me because this is information that can be placed in the TPlanNode which can be made aware of its parentId? But I got to thinking about this a bit more too, and let me run this by you... I'm also gonna work under the assumption that we can create runtime filters in the thrift structure that aren't "effective"? Not sure if that is true or what that means, but I'm gonna assume that's the case. So I started with the idea of placing the parentId in the TPlanNode, but then I thought: Can each TPlanNode just contain all runtime filter Ids that may be affected? This would eliminate a lot of the new code in the backend, I think. This is my frontend bias, but this seems like information that can be calculated up front as opposed to "effective" runtime filters which information might be only available on the backend. So now if we have the TPlanNode contain a list of runtime filter Ids that could affect it, I think the backend code doesn't change too much? http://gerrit.cloudera.org:8080/#/c/24297/8/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/24297/8/fe/src/main/java/org/apache/impala/service/Frontend.java@2454 PS8, Line 2454: */ nit: prolly either both buildNodeParentIds should be static or neither one? -- 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 00:39:19 +0000 Gerrit-HasComments: Yes
