Repository navigation
Optimize count(col) using table statistics #904
Description
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
If both the number of rows and the number of nulls are available as statistic, we can compute the value based on statistics only when reading.
row_count - null_count(row)
Describe the solution you'd like
Add an optimization rule that matches this count expression with column and use the statistics to replace it with the value.
Describe alternatives you've considered
Additional context
Activity
[-]Optimize `coun(col)` using table statistics[/-][+]Optimize `count(col)` using table statistics[/+]matthewmturner commentedon Sep 9, 2021
@Dandandan i wanted to look into this but saw that it was mentioned in #962 as having potential merge conflicts. Is it ok for me to start looking into this? Or should I hold off for now?
Dandandan commentedon Sep 9, 2021
Maybe best to consult @rdettai about what is timing-wise the best way to implement this particular optimization.
rdettai commentedon Sep 9, 2021
in #965 I have moved that optimization to the physical plan, so if the PR gets accepted, this optimization should definitively be done there instead of the logical plan
matthewmturner commentedon Sep 9, 2021
@rdettai ok, will wait for that then.
just for my understanding, i thought this was a new optimization rule, so how was it moved to the physical plan already?
houqp commentedon Sep 10, 2021
@matthewmturner @rdettai was saying he moved all existing cost based optimizers (not this particular one) into physical plane in that PR :)
rdettai commentedon Sep 10, 2021
actually, If I'm not mistaken this optimization would go into the
AggregateStatisticsrule, which is one of the CBOs that has moved from being anOptimizerRuleto aPhysicalOptimizerRule😃matthewmturner commentedon Sep 10, 2021
@rdettai @houqp thx both for info. so my understanding is that this is an optimization that has not yet been implemented, but it will ultimately belong to the
AggregateStatisticsrule. Once the CBO have been moved to the physical level I can work on this?rdettai commentedon Sep 10, 2021
Exactly! You can already take a try at it from my PR. But you would take the risk that if it does not get merged the work has to be done again on the
OptimizerRuleversion 🙂matthewmturner commentedon Sep 17, 2021
@rdettai glad your PR got merged :) now that it is, is it a good time to try working on this?
rdettai commentedon Sep 17, 2021
Yes sure!
matthewmturner commentedon Sep 20, 2021
@Dandandan @rdettai i was going to add this to the existing
take_optimizable_countrule as it seems sufficiently similar. do you agree this is right place or is a new rule needed?rdettai commentedon Sep 20, 2021
I would write a separate
take_optimizable_non_null_count, or something like that, but if you have a better idea to structure this part of the code feel free to propose it in a PRmatthewmturner commentedon Sep 20, 2021
@rdettai got it, thanks. Im still just working on getting an understanding of the existing rules. Would you be able to provide some more details on why there is a
lit_exprfor the count rule and its equal to&ScalarValue::UInt8(Some(1))? My naive expectation was to have something similar to the max_min where acol_exprwas made and thenum_rows/null_countfrom theColumnStatisticscould be used.thx in advance.
chore: Close dictionary provider when iterator is closed (apache#904)