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

Started by Dmytro Astapov4 months ago3 messagesbugs
Jump to latest
#1Dmytro Astapov
dastapov@gmail.com

Hi!

Merry Christmas, happy New Year, and various seasonal greetings to you all.
I come bearing the bug report for a regression, and hopefully a fix as well.

*Short summary:*

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.

*Environment and PostgreSQL Versions:*

Affected versions: 17.7, 18.1
Not affected versions: 16.11, 15.15, 14.20, 13.23

OS: Red Hat 11.5.0-5

*Steps to Reproduce*

CREATE TABLE test (seqno int NOT NULL);

INSERT INTO test SELECT s FROM generate_series(1, 100000) s;

CREATE UNIQUE INDEX ON test(seqno) WHERE seqno IS NOT NULL;

ANALYZE test;

EXPLAIN SELECT max(seqno) FROM test;

*Expected behavior (observed on PostgreSQL 16.11 and earlier)*

QUERY PLAN

---------------------------------------------------------------------------------------

Result (cost=0.32..0.33 rows=1 width=4)

InitPlan 1

-> Limit (cost=0.29..0.32 rows=1 width=4)

-> Index Only Scan Backward using test_seqno_idx on test
(cost=...)

*Actual behavior (observed on PostgreSQL 17.7 and later)*

QUERY PLAN

------------------------------------------------------------------

Aggregate (cost=1693.00..1693.01 rows=1 width=4)

-> Seq Scan on test (cost=0.00..1443.00 rows=100000 width=4)

*I've tried to find the root cause, and this is my (possibly incorrect)
analysis*

The regression was introduced by commit *b262ad440ed* ("Add better handling
of redundant IS [NOT] NULL quals", 2024-01-23).

This commit added an optimization to remove "IS NOT NULL" restriction
clauses when the column is defined as "NOT NULL", since such clauses are
always true.
This optimization was added to fix Bug #17540 related to poor index choice
in min/max queries.

However, this optimization has an unintended side effect on partial index
matching:

1. The min/max optimization (in "planagg.c") rewrites "SELECT max(col)"
into a subquery with the clause "WHERE col IS NOT NULL" added to handle
NULL values correctly.

2. The new optimization in `add_base_clause_to_rel()` recognizes that the
added clause "col IS NOT NULL" is always true (since "col" is "NOT NULL"
according to the table schema) and discards the clause before adding it to
"baserestrictinfo".

3. Later, "check_index_predicates()" attempts to prove that the partial
index predicate ("WHERE seqno IS NOT NULL") is implied by the query's
clauses in "baserestrictinfo".

4. Since the "IS NOT NULL" clause was discarded, the partial index
predicate cannot be proven, and the index is not marked as usable ("predOK"
remains false).

*Suggested fix*

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

Then fix could be to modify `add_base_clause_to_rel()` to retain `IS NOT
NULL` clauses when they match a partial index predicate on the same column.
This preserves the intended fix for Bug #17540 while restoring partial
index usability.

I am attaching the patch against REL_18_1 which attempts to do this.

*Testing*

- The attached patch resolves the reported issue
- All 228 standard regression tests pass

--
Dmytro Astapov

Attachments:

pg_18_1_fix_restriction_is_always_true.patchapplication/x-patch; name=pg_18_1_fix_restriction_is_always_true.patchDownload+73-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dmytro Astapov (#1)
Re: Regression: partial index with IS NOT NULL predicate not used for min/max optimization on NOT NULL columns

Dmytro Astapov <dastapov@gmail.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

#3Dmytro Astapov
dastapov@gmail.com
In reply to: Tom Lane (#2)
Re: Regression: partial index with IS NOT NULL predicate not used for min/max optimization on NOT NULL columns

On Wed, Dec 24, 2025 at 5:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

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:
<snip the EXPLAIN>

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.

Thank you for such a quick reply. This is truly Christmas come early to see
that this is already fixed in HEAD.

I read e2debb643, and it looks like it fixes the issue I reported with
two-prong approach:

Both query clauses and index predicates go through "eval_const_expressions":
- Query clauses are reduced in "subquery_planner" before "query_planner"
- Index predicates are independently reduced in "get_relation_info"
(plancat.c lines 453-456)

So If a query's "col IS NOT NULL" is reduced to TRUE (because col has a
"NOT NULL" constraint), the partial index predicate "WHERE col IS NOT NULL"
is also reduced to NIL, making the index effectively non-partial (if that
is the right term to use).

I poked around a bit, but was unable to construct a counterexample that
you've hinted at.

Maybe the potential fragility comes from having both query clauses and
index predicates going through the same reduction logic, and a change to
either path could break the "status quo", but seems (to me, at least, which
is not worth a lot :) like no easy way to "break" it exists now.

Anyway, thanks again!