Re: Removing const-false IS NULL quals and redundant IS NOT NULL quals

On Thu, 28 Dec 2023 at 00:38, Andy Fan <zhihuifan1213(at)163(dot)com> wrote:
> I also want to add notnullattnums for the UniqueKey stuff as well, by
> comparing your implementation with mine, I found you didn't consider
> the NOT NULL generated by filter. After apply your patch:
>
> create table a(a int);
> explain (costs off) select * from a where a > 3 and a is null;
> QUERY PLAN
> -------------------------------------
> Seq Scan on a
> Filter: ((a IS NULL) AND (a > 3))
> (2 rows)

> [1]
> https://www.postgresql.org/message-id/attachment/151254/v1-0001-uniquekey-on-base-relation-and-used-it-for-mark-d.patch

I believe these are two different things and we should not mix the two up.

Looking at your patch, I see you have:

+ /* The not null attrs from catalogs or baserestrictinfo. */
+ Bitmapset *notnullattrs;

Whereas, I have:

/* zero-based set containing attnums of NOT NULL columns */
Bitmapset *notnullattnums;

I'm a bit worried that your definition of notnullattrs could lead to
confusion about which optimisations will be possible.

Let's say for example I want to write some code that optimises the
expression evaluation code to transform EEOP_FUNCEXPR_STRICT into
EEOP_FUNCEXPR when all function arguments are Vars that have NOT NULL
constraints and are not nullable by any outer join. With my
definition, it should be safe to do this, but with your definition, we
can't trust we'll not see any NULLs as if the strict function is
evaluated before the strict base qual that filters the NULLs then the
strict function could be called with NULL.

Perhaps we'd want another Bitmapset that has members for strict OpExrs
that filter NULLs and we could document that it's only safe to assume
there are no NULLs beyond the scan level.... but I'd say that's
another patch and I don't want to feed you design ideas here and
derail this patch.

David