Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/23164 )
Change subject: IMPALA-13437 (part 1): Compute processing cost before TupleCachePlanner ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/23164/6/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/23164/6/fe/src/main/java/org/apache/impala/planner/Planner.java@360 PS6, Line 360: This is true for : // COMPUTE_PROCESSING_COST=true and ENABLE_TUPLE_CACHE=true > The phrasing is vague: in english, "for" doesn't really imply a logical sta Ack http://gerrit.cloudera.org:8080/#/c/23164/6/fe/src/main/java/org/apache/impala/planner/TupleCacheNode.java File fe/src/main/java/org/apache/impala/planner/TupleCacheNode.java: http://gerrit.cloudera.org:8080/#/c/23164/6/fe/src/main/java/org/apache/impala/planner/TupleCacheNode.java@55 PS6, Line 55: if (child.getFilteredCardinality() != cardinality_) { > That does seem like it should be true. Is there something that depends on t That's a good point, nothing specific depends on it in my mind. My idea was that it's better to expose an abnormal case like this as early as possible. If an assert feels too aggressive, maybe logging a WARN is also helpful for flagging the abnormal behavior if it happens -- To view, visit http://gerrit.cloudera.org:8080/23164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If78f5d002b0e079eef1eece612f0d4fefde545c7 Gerrit-Change-Number: 23164 Gerrit-PatchSet: 8 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 11 Sep 2025 06:00:33 +0000 Gerrit-HasComments: Yes
