Re-land Decimal filter selectivity estimation (#25790) without the TPC-H Q18 regression #27146

Activity

1. shrivasshankar commented on Nov 5, 2026

   

   shrivasshankar

   on Nov 5, 2026

   Contributor

   More actions

   take
2. github-actions

   assigned

   shrivasshankar

   on Nov 5, 2026
3. shrivasshankar commented on Nov 5, 2026

   

   shrivasshankar

   on Nov 5, 2026

   Contributor

   More actions

   sorry, I looked into it and reproduced it locally.

   in Q18, HAVING sum(l_quantity) > 300 filters the output of an aggregate, so the sum column has no min/max stats. before my PR, decimals weren't supported, so this filter got the default 20% (the ~120M rows in the 55.1 plan). now it goes through interval analysis, but with no bounds it estimates 100%, so 600M rows. that makes it look like the biggest input, and the semi join builds on the wrong side.

   I mentioned this "no stats means 100%" case in the PR description but didn't think about aggregate outputs. I reproduced it at SF1: the join flips and peak memory goes from ~430 MB to ~1.25 GB.

   for the re-land, I'll keep the default selectivity when a decimal column has no min/max stats, and add a Q18 plan test so this can't come back. i've tested that locally: Q18 gets its old plan back and the Q6 improvement stays.

   👍React with 👍1Reacted by Alessandro Solimando
4. bvolpato

   added a commit that references this issue on Nov 6, 2026

   Revert apache#25790 (Decimal filter selectivity) due to TPC-H Q18 reg…

   ...

   8460f4f
5. asolimando commented on Nov 6, 2026

   

   asolimando

   on Nov 6, 2026

   MemberAuthor

   More actions

   sorry, I looked into it and reproduced it locally.

   in Q18, HAVING sum(l_quantity) > 300 filters the output of an aggregate, so the sum column has no min/max stats. before my PR, decimals weren't supported, so this filter got the default 20% (the ~120M rows in the 55.1 plan). now it goes through interval analysis, but with no bounds it estimates 100%, so 600M rows. that makes it look like the biggest input, and the semi join builds on the wrong side.

   I mentioned this "no stats means 100%" case in the PR description but didn't think about aggregate outputs. I reproduced it at SF1: the join flips and peak memory goes from ~430 MB to ~1.25 GB.

   for the re-land, I'll keep the default selectivity when a decimal column has no min/max stats, and add a Q18 plan test so this can't come back. i've tested that locally: Q18 gets its old plan back and the Q6 improvement stays.

   I took a cursory look when the error was reported and came to the same conclusion and what you propose is a reasonable fix for the degenerate cases with no bounds, happy to take a look when ready to get it to a reviewable state for maintainers to approve (FYI I will be mostly off next week with limited connectivity, fully back after that). Thanks for taking a look that quickly, @shrivasshankar!
6. shrivasshankar commented on Nov 6, 2026

   

   shrivasshankar

   on Nov 6, 2026

   Contributor

   More actions

   sorry, I looked into it and reproduced it locally.
   in Q18, HAVING sum(l_quantity) > 300 filters the output of an aggregate, so the sum column has no min/max stats. before my PR, decimals weren't supported, so this filter got the default 20% (the ~120M rows in the 55.1 plan). now it goes through interval analysis, but with no bounds it estimates 100%, so 600M rows. that makes it look like the biggest input, and the semi join builds on the wrong side.
   I mentioned this "no stats means 100%" case in the PR description but didn't think about aggregate outputs. I reproduced it at SF1: the join flips and peak memory goes from ~430 MB to ~1.25 GB.
   for the re-land, I'll keep the default selectivity when a decimal column has no min/max stats, and add a Q18 plan test so this can't come back. i've tested that locally: Q18 gets its old plan back and the Q6 improvement stays.

   I took a cursory look when the error was reported and came to the same conclusion and what you propose is a reasonable fix for the degenerate cases with no bounds, happy to take a look when ready to get it to a reviewable state for maintainers to approve (FYI I will be mostly off next week with limited connectivity, fully back after that). Thanks for taking a look that quickly, @shrivasshankar!

   Thanks! It’s ready for review in #27148 no rush.