Re: Check each of base restriction clauses for constant-FALSE-or-NULL

On Tue, Oct 10, 2023 at 5:10 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Sat, 7 Oct 2023 at 22:44, Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> >
> > In relation_excluded_by_constraints() when we're trying to figure out
> > whether the relation need not be scanned, one of the checks we do is to
> > detect constant-FALSE-or-NULL restriction clauses. Currently we perform
> > this check only when there is exactly one baserestrictinfo entry, and
> > the comment explains this as below.
> >
> > * Regardless of the setting of constraint_exclusion, detect
> > * constant-FALSE-or-NULL restriction clauses. Because const-folding
> will
> > * reduce "anything AND FALSE" to just "FALSE", any such case should
> > * result in exactly one baserestrictinfo entry.
>
> Coincidentally (?), I saw the same thing just a few weeks ago while
> working on [1]. I made the exact same adjustment to the code in
> relation_excluded_by_constraints() as you have.

Haha, I noticed the need of this change while writing v5 patch [1] for
that same thread. That patch generates a new constant-FALSE
RestrictInfo for an IS NULL qual that can be reduced to FALSE, and this
makes the comment in relation_excluded_by_constraints() about 'any such
case should result in exactly one baserestrictinfo entry' not true any
more. Without this change in relation_excluded_by_constraints(), a
query like below would not be able to be marked as dummy.

select * from t where a is null and 'otherquals';

And then the regression test diff after applying this change reminds me
that equivclass.c may also generate new constant-FALSE RestrictInfos on
the fly, so it seems to me that this change may benefit some queries
even without the 'reduce-NullTest' patch.

> I wasn't really expecting the baserestrictinfo list to be excessively
> long, and if it ever was, I think looking at things like selectivity
> estimations would by far drown out looping over the entire list in
> relation_excluded_by_constraints() rather than just looking at the
> first item in the list.

Agreed.

> After making the change, I saw the same regression test change as you
> did, but didn't really feel like it was worth tackling separately from
> the patch that we were working on.

I was thinking that this change may be worthwhile by itself even without
the 'reduce-NullTest' patch, because it can benefit some cases, such as
where EC generates constant-FALSE on the fly. So maybe it's worth a
separate patch? I'm not quite sure.

[1]
https://www.postgresql.org/message-id/CAMbWs4-eNVNTNc94eF%2BO_UwHYKv43vyMurhcdqMV%3DHt5fehcOg%40mail.gmail.com

Thanks
Richard