Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24605 )
Change subject: IMPALA-15195: Calcite Planner performance improvements for tpcds ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java: http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@205 PS6, Line 205: RelColumnOrigin originCol = mq_.getColumnOrigin(childRel_, inputRef.getIndex()); This needs to use relNode, not childRel_. After recursing into the right side of a join, inputRef has a child-local ordinal. Resolving that ordinal against the original join can pick the column at the same position from the left input, so e.g. r.c IS NULL may use l's null stats. Please add a right-side join test. http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@212 PS6, Line 212: return getNumNulls(columnNum, table) / table.getRowCount(); This can return NaN for an empty table: getNumNulls() is 0 and table.getRowCount() is 0, so estimateSelectivity() accepts 0/0 instead of falling back. The old code guarded the denominator with Math.max(..., 1). http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@274 PS6, Line 274: Double innerRowCount = info.getRowCount(); This throws for a valid non-equi outer join. With l LEFT JOIN r ON l.x < r.x, JoinRelationInfo has no equalityConjunctions, and getRowCount() fails its checkState. This needs useDefaultRowCount() handling and a focused test with r.x IS NULL. http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@282 PS6, Line 282: Double percentage = Math.min(innerRowCount/nonOuterJoinRowCount, 1.0); This is the matched-row ratio, not the null percentage. With a LEFT JOIN and a right-side column, zero matches returns 0 even though every output value is NULL; a full one-to-one match returns 1 even though the join adds no NULLs. The denominator also needs to reflect the outer-join output for one-to-many matches. http://gerrit.cloudera.org:8080/#/c/24605/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@288 PS6, Line 288: // outer join We cannot pass the same ordinal through an arbitrary Project. A projection can reorder columns or produce an expression, so this can inspect the wrong join side or column. Please map through Project.getProjects().get(columnNum) and only recurse for a direct RexInputRef; otherwise use the fallback. -- To view, visit http://gerrit.cloudera.org:8080/24605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ee6662dc6500ea6ebd9f859de4df1740c51a058 Gerrit-Change-Number: 24605 Gerrit-PatchSet: 6 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Tue, 04 Aug 2026 18:01:55 +0000 Gerrit-HasComments: Yes
