Обсуждение: [PATCH] Add support for SAOP in the optimizer for partial index paths

Hi Postgres hackers,

This is my first patch to the project and I've been sitting on it for 6 months!

This patch was produced via:

git diff -p -U 4d936c3fff1ac8dead2cc240ba3da2ed6337257c

The branch point was 4d936c3fff1ac8dead2cc240ba3da2ed6337257c (master
as of 05/12/2025 1445 GMT)

The patch, though a single diff, was generated from 7 logically
distinct commits (feature, tests, expected output etc.).

I hope I've read the submission guides sufficiently. The code change
was based heavily on the existing code in indxpath.c.

Here's a summary of the feature:

    Prior to this patch, only BitmapOr paths were considered for partial
    indexes. With this patch, we now support ScalarArrayOpExpr clauses
    too (i.e. ANY() and IN()).

    I found no entry for this feature in the TODO list here;
    - https://wiki.postgresql.org/wiki/Todo

    However, it has previously been reported/raised here;
    -
https://www.postgresql.org/message-id/flat/c128bd06-a246-4129-914c-3dee7b13417a%40vondra.me#5b3f3b7e90d6de8c39a095afaea6b460

    The new function, generate_bitmap_saop_paths, was largely based on the
    existing generate_bitmap_or_paths() function while also glancing at
    other array handling code such as that found in backend/utils/adt/xml.c
    plus some additional false-starts in backend/optimizer/util/predtest.c

    The C code was formatted via;
    src/tools/pgindent/pgindent --indent=src/tools/pg_bsd_indent/pg_bsd_indent

Cheers,

Jim Vanns

Hello again,

Hope you don't mind me bumping this a little, but I wondered if I
should have requested a shepherd/mentor in getting this patch through
the review process? Or await a commitfest?

Cheers,

Jim

On Fri, 5 Dec 2025 at 14:59, Jim Vanns <james.vanns@gmail.com> wrote:
>
> Hi Postgres hackers,
>
> This is my first patch to the project and I've been sitting on it for 6 months!
>
> This patch was produced via:
>
> git diff -p -U 4d936c3fff1ac8dead2cc240ba3da2ed6337257c
>
> The branch point was 4d936c3fff1ac8dead2cc240ba3da2ed6337257c (master as of 05/12/2025 1445 GMT)
>
> The patch, though a single diff, was generated from 7 logically distinct commits (feature, tests, expected output
etc.).
>
> I hope I've read the submission guides sufficiently. The code change was based heavily on the existing code in
indxpath.c.
>
> Here's a summary of the feature:
>
>     Prior to this patch, only BitmapOr paths were considered for partial
>     indexes. With this patch, we now support ScalarArrayOpExpr clauses
>     too (i.e. ANY() and IN()).
>
>     I found no entry for this feature in the TODO list here;
>     - https://wiki.postgresql.org/wiki/Todo
>
>     However, it has previously been reported/raised here;
>     -
https://www.postgresql.org/message-id/flat/c128bd06-a246-4129-914c-3dee7b13417a%40vondra.me#5b3f3b7e90d6de8c39a095afaea6b460
>
>     The new function, generate_bitmap_saop_paths, was largely based on the
>     existing generate_bitmap_or_paths() function while also glancing at
>     other array handling code such as that found in backend/utils/adt/xml.c
>     plus some additional false-starts in backend/optimizer/util/predtest.c
>
>     The C code was formatted via;
>     src/tools/pgindent/pgindent --indent=src/tools/pg_bsd_indent/pg_bsd_indent
>
> Cheers,
>
> Jim Vanns

Just another gentle nudge in the home somebody might bite. I rebased
it into master again this morning and that all worked fine, so I don't
think there is any clashing code, yet. I'm happy to address feedback
etc. where guided.

Cheers

Jim

On Fri, 5 Dec 2025 at 14:59, Jim Vanns <james.vanns@gmail.com> wrote:
>
> Hi Postgres hackers,
>
> This is my first patch to the project and I've been sitting on it for 6 months!
>
> This patch was produced via:
>
> git diff -p -U 4d936c3fff1ac8dead2cc240ba3da2ed6337257c
>
> The branch point was 4d936c3fff1ac8dead2cc240ba3da2ed6337257c (master
> as of 05/12/2025 1445 GMT)
>
> The patch, though a single diff, was generated from 7 logically
> distinct commits (feature, tests, expected output etc.).
>
> I hope I've read the submission guides sufficiently. The code change
> was based heavily on the existing code in indxpath.c.
>
> Here's a summary of the feature:
>
>     Prior to this patch, only BitmapOr paths were considered for partial
>     indexes. With this patch, we now support ScalarArrayOpExpr clauses
>     too (i.e. ANY() and IN()).
>
>     I found no entry for this feature in the TODO list here;
>     - https://wiki.postgresql.org/wiki/Todo
>
>     However, it has previously been reported/raised here;
>     -
https://www.postgresql.org/message-id/flat/c128bd06-a246-4129-914c-3dee7b13417a%40vondra.me#5b3f3b7e90d6de8c39a095afaea6b460
>
>     The new function, generate_bitmap_saop_paths, was largely based on the
>     existing generate_bitmap_or_paths() function while also glancing at
>     other array handling code such as that found in backend/utils/adt/xml.c
>     plus some additional false-starts in backend/optimizer/util/predtest.c
>
>     The C code was formatted via;
>     src/tools/pgindent/pgindent --indent=src/tools/pg_bsd_indent/pg_bsd_indent
>
> Cheers,
>
> Jim Vanns

On Sat, 6 Dec 2025 at 03:59, Jim Vanns <james.vanns@gmail.com> wrote:
> This is my first patch to the project and I've been sitting on it for 6 months!

Welcome!

> Here's a summary of the feature:
>
>     Prior to this patch, only BitmapOr paths were considered for partial
>     indexes. With this patch, we now support ScalarArrayOpExpr clauses
>     too (i.e. ANY() and IN()).
>
>     I found no entry for this feature in the TODO list here;
>     - https://wiki.postgresql.org/wiki/Todo
>
>     However, it has previously been reported/raised here;
>     -
https://www.postgresql.org/message-id/flat/c128bd06-a246-4129-914c-3dee7b13417a%40vondra.me#5b3f3b7e90d6de8c39a095afaea6b460
>
>     The new function, generate_bitmap_saop_paths, was largely based on the
>     existing generate_bitmap_or_paths() function while also glancing at
>     other array handling code such as that found in backend/utils/adt/xml.c
>     plus some additional false-starts in backend/optimizer/util/predtest.c

I had a quick look and the idea seems reasonable.

A couple of things:

1. It's probably worth having generate_bitmap_saop_paths() do a
precheck for suitable partial and bitmap supporting indexes before
looping over each element of the SOAP array. Maybe just before the
"elem_type = ARR_ELEMTYPE(arrayval);" where the more expensive stuff
starts to happen. You could also record the List's array element
indexes of the possibly suitable partial indexes in a Bitmapset and
loop over those ones with a bms_next_member() loop rather than all
'indexes'. I think partial indexes are rare enough to warrant the
short circuit before getting in too deep. Also, not having to re-find
the indexes you're interested in for each SOAP array element seems
worthwhile.

2. For your tests, I think you can lump all these new tests into
bitmapops.sql. Please shrink the row counts down to much smaller than
10k rows. There's probably no need for any rows if you disable
enable_seqscan and enable_indexscan. The existing test in that file
has to have quite a large row count as it's testing lossy bitmaps.

I would expect this extra processing to add quite a bit of overhead in
certain scenarios. Can you test this and include the SQL scripts you
used to test that? We need to establish the performance of a
reasonable worst-case for this doesn't unreasonably slow the planner
down. Perhaps a few dozen indexes and test with a 100-element SOAP
array and extract the average planning time from EXPLAIN (SUMMARY ON)
with and without the patch.

If you do see quite a bit of overhead, then that might also trigger
you to consider what other short-circuits are possible.

Also, please register the patch in [1]. Unfortunately, the January CF
has started now, but if you get it in March's then it shouldn't get
forgotten.

David

[1] https://commitfest.postgresql.org/58/