Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24605 )

Change subject: IMPALA-15195: Calcite Planner performance improvements for tpcds
......................................................................


Patch Set 8:

(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:     // If the origin table can be found, get the null percentage 
from there.
> Sorry, one clarification since the numbering shifted: I was reading PS8, wh
Done


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:     // If the origin table can be found, get the null percentage 
from there.
> Ah, I think the normal `TableScan` base case can get us here too. For a rig
Oh, Ok, I get it now.

In my mind, I saw childRel and was assuming it was the child of the node being 
recursed, not the filter RelNode at the top level.

This makes sense and I put in a new test and verified that the bad code 
produced a different value.


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:         // won't match up and thus put in a null value for that 
column.
> Yep, generated nulls are what I meant. `IS NOT NULL` does call the same cod
Ack


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:           // side didn't match. For now, return the default null 
percentage.
> I think we can get a reasonable estimate from the same uniform-distribution
Sounds good, thanks for coming up with this!


http://gerrit.cloudera.org:8080/#/c/24605/8/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/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/FilterSelectivityEstimator.java@267
PS8, Line 267:           return DEFAULT_IS_NULL_PERCENTAGE;
> This changed behaviour for semi/anti joins between PS6 and PS8. It used to
This makes sense.  Reverted the code.



--
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: 8
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 21:24:37 +0000
Gerrit-HasComments: Yes

Reply via email to