Re: Regression: partial index with IS NOT NULL predicate not used for min/max optimization on NOT NULL columns

Dmytro Astapov <dastapov(at)gmail(dot)com> writes:
> When a column is defined as "NOT NULL" and a partial index exists with a
> predicate "WHERE column IS NOT NULL", the min/max aggregate optimization
> fails to use the partial index. This is a regression introduced in
> PostgreSQL 17.

Ugh.

> *Suggested fix*
> I think that when clause in the query matches a predicate in the partial
> index, it should not be discarded.

I don't like that proposal a bit. It makes the behavior more complex
and less consistent, and probably re-introduces the problem complained
of in bug #17540.

The real problem here is that the operations are being done in the
wrong order; in particular making add_base_clause_to_rel responsible
for discarding qual conditions was a bad idea. There are at least
two ways we could make it work less poorly:

1. postpone discarding of constant NOT NULL conditions till after
we check index predicates;

2. move discarding of constant NOT NULL conditions into
eval_const_expressions, which can fix the problem because that
is also applied to index predicates.

I think #2 is the better answer ... and, as it happens, that got done
recently (in e2debb643). So HEAD no longer exhibits the problem you
show:

regression=# EXPLAIN SELECT max(seqno) FROM test;
QUERY PLAN
---------------------------------------------------------------------------------------------------------------
Result (cost=0.32..0.33 rows=1 width=4)
Replaces: MinMaxAggregate
InitPlan minmax_1
-> Limit (cost=0.29..0.32 rows=1 width=4)
-> Index Only Scan Backward using test_seqno_idx on test (cost=0.29..3050.29 rows=100000 width=4)
(5 rows)

However, there is still a check for constant-true conditions
in add_base_clause_to_rel, because the author argued that there
are edge cases that still justify it. I am wondering though if
your example can be modified so that it still misbehaves in HEAD.
That would be ammunition to remove the check altogether, which
I still think is what we should do. It's a fundamental structural
error to do this there.

regards, tom lane