Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20460 )
Change subject: IMPALA-12371: Add better cardinality estimation for Iceberg V2 tables with deletes ...................................................................... Patch Set 2: (2 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/20460/2/fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java File fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java: http://gerrit.cloudera.org:8080/#/c/20460/2/fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java@98 PS2, Line 98: is not zero > What happens if it is zero? We don't reach this code then? If there are input files to scan, leftCard will always be greater than 0, so we always report at least one cardinality. This is in line with: https://github.com/apache/impala/blob/0f55e551bc98843c79a9ec82582ddca237aa4fe9/fe/src/main/java/org/apache/impala/planner/PlanNode.java#L747 Actually, if the leftCard is 0 (no input files), then the planner shouldn't even create this node. So alternatively we could probably use a Preconditions.checkState() and a bit simpler formula: cardinality_ = Math.max( 1, leftCard - (long)(leftSelectivity * rightCard)); http://gerrit.cloudera.org:8080/#/c/20460/2/fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java@100 PS2, Line 100: leftCard > For 'leftCard' selectivity has already been taken into account, that's why Yes. -- To view, visit http://gerrit.cloudera.org:8080/20460 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I988dc8d7e1074932c460b3702d3381341e5b23c5 Gerrit-Change-Number: 20460 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 08 Sep 2023 14:28:31 +0000 Gerrit-HasComments: Yes
