Thank you so much, David. I'll give this reply a more thorough read through and address the points you've raised over the next few days or so.
Cheers Jim On Sat, 3 Jan 2026, 00:38 David Rowley, <[email protected]> wrote: > On Sat, 6 Dec 2025 at 03:59, Jim Vanns <[email protected]> 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/ >
