vlsi commented on PR #2330: URL: https://github.com/apache/calcite/pull/2330#issuecomment-6341052190
Thanks for the work on this, @liyafan82. I'm closing the PR because it no longer applies to `main`, but the idea is still open in [CALCITE-4465](https://issues.apache.org/jira/browse/CALCITE-4465). `RelMdDistinctRowCount.getDistinctRowCount(Filter, …)` on `main` still forwards the predicate to the input without deriving a bound from it, so the improvement is not implemented elsewhere. The PR itself does not compile against `main`: `Sarg.containsNull` was removed in bf56743554 in favor of `Sarg.nullAs`, and four of the six files conflict. The `Project` and `Aggregate` handlers in `RelMdDistinctRowCount` have also changed since (CALCITE-4511, CALCITE-7083, CALCITE-7793). A new PR would need to: - compute the bound from `Sarg.nullAs` (a null counts as a distinct value only for `RexUnknownAs.TRUE`); - handle other discrete types, such as `DATE` and `DECIMAL` with scale 0, or state why they are left out (the JIRA discussion asks for special handling of discrete types); - fix the `BOOLEAN` case in `RangeSets.numDistinctVals`, which never runs: it sits inside the `instanceof Number` check, and a `BOOLEAN` Sarg holds `Boolean` endpoints; - apply the bound in one place rather than repeating `RexUtil.estimateColumnsNdv` in every handler, since `Filter`, `Sort`, `Exchange`, `TableModify`, `Union`, and `RelSubset` pass the predicate to an input that applies the bound again; - include the plan changes in existing tests, because a lower NDV changes row-count estimates for an `Aggregate` above a `Filter` with an `IN` list or a range. Anyone who picks up the JIRA is welcome to reuse the tests from this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
