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: (3 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()); > It's actually a little harder to make this test fail than it seems. Ah, I think the normal `TableScan` base case can get us here too. For a right-side column of an inner join, line 259 recurses into `join.getInput(1)` with a child-local ref. `getJoinNullPercentageEstimate()` then returns null for the `TableScan`, and we reach the column-origin lookup. With the old `childRel_`, that local ordinal is still resolved against the original join. So I don't think we need an anti/semi join to reproduce it; a right-side column with different null stats should do it. 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(); > I'm not sure I'm totally getting what you're saying here... Yep, generated nulls are what I meant. `IS NOT NULL` does call the same code; the issue with the old test was that expecting 100% non-null also passed when we fell back to the base table's zero-null stats. The PS8 `IS NULL` case with the default estimate covers it. Thanks! 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); > Same problem as the non-equality. I don't know how to determine the one-to I think we can get a reasonable estimate from the same uniform-distribution assumption that `JoinRelationInfo` already uses. Let `overlap = min(leftNdv, rightNdv)`. Then `unmatchedLeft = leftRows * (1 - overlap / leftNdv)`, and likewise for the right side. LEFT output is `innerRows + unmatchedLeft`, RIGHT uses `unmatchedRight`, and FULL includes both. For a right-side column in a LEFT JOIN, the generated-null fraction is `unmatchedLeft / outputRows`. That gives 50% for the `bigint_col` example. If the NDVs are missing, the default still seems right. Does that sound reasonable? -- 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: Thu, 06 Aug 2026 16:01:55 +0000 Gerrit-HasComments: Yes
