On Thu, 7 Jul 2022 at 15:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Rowley <dgrowleyml@gmail.com> writes:
> > Anyway, I've no current plans to take the attached any further. I
> > think it'll be better to pursue your NULLable-Var stuff and see if we
> > can do something more generic like remove provably redundant NullTests
> > from baserestrictinfo.
>
> Yeah, I suspect that the way forward is to allow
> preprocess_minmax_aggregates to do what it does now, and then
> remove the IS NOT NULL clause again later when we have the
> info available to let us do that in a generic way.
I started looking at a more generic way to fix this. In the attached
I'm catching quals being added to baserestrictinfo in
distribute_restrictinfo_to_rels() and checking for IS NOT NULL quals
on columns defined with NOT NULL.
I did this by adding a new function add_baserestrictinfo_to_rel()
which can be the place where we add any future logic to ignore other
always-true quals. Perhaps in the future, we can add some logic there
to look for quals on partitions which are always true based on the
partition constraint.
I also took the opportunity here to slightly modernised the Bitmapset
code in this area. We previously called bms_membership() and then
bms_singleton_member(), which is not quite optimal. We invented
bms_get_singleton_member() as a more efficient way of getting that.
The empty set case can just be handled more easily now since you
changed empty sets to always be NULL. If it's not an empty set and not
a singleton, then it must contain multiple members.
I'm quite keen to see some forward progress on improving things for
this bug report. It would be good to take some more measures to stop
the planner being tricked into making silly mistakes. This is one
example of somewhere we could do better.
David