Re: [PATCH] Add support for SAOP in the optimizer for partial index paths

Just adding the same patch rebased atop today's master at
89eafad297a9b01ad77cfc1ab93a433e0af894b0  before I attempt my first
commit-fest entry. I've been sitting on this for a year now but I
think I missed the PG19 boat? So I targeted PG20 in the CF entry.

Jim


On Sun, 1 Mar 2026 at 11:22, Jim Vanns <james.vanns@gmail.com> wrote:
>
> OK, I've had a considerable refactor which hopefully addresses all your points.
>
> The function now proceeds as follows;
>
> 1) Preparatory/rudimentary checks outside of main loop (OR-based SAOP
> clause, array dimensionality, element count etc.)
> 2) Pre-filter bitmap indices for partials or planner implied
> 3) For each element IN() now look for best choice index via
> compare_path_costs (not just first as before)
> 3a) Check clause fits general index structure (affecting all elements)
> 3b) Check index matches predicate value/element
> 4) Build bitmap OR path for this candidate
> 5) Tests now moved to existing bitmapopts sql/out
>
> I've extended the existing test too to include the multiple-choice
> path for the planner as you suggested, which this patch should now
> handle (before it was greedy).
>
> It's rebased on top of the current master
> (aecc558666ad62fbecb08ff7af1394656811a581) to remain
> relevant/up-to-date.
>
> I've not yet tested the performance of it (preferring
> correctness/acceptance first!) in the face of many indexes and 100
> elements in the array but before I do so, is there a best place for
> this kind of test in the source tree? Somewhere beneath src/test
> again? I looked but couldn't see an obvious place (bar the regression
> tests).
>
> Cheers,
>
> Jim
>
>
> Jim
>
> On Mon, 12 Jan 2026 at 00:54, David Rowley <dgrowleyml@gmail.com> wrote:
> >
> > On Sun, 11 Jan 2026 at 06:03, Jim Vanns <james.vanns@gmail.com> wrote:
> > > Before I continue with the other suggestions of consolidating the test
> > > and benchmarking, I've made the code change you suggested and used a
> > > bitmap for recording positions in the list of candidate indexes. Can
> > > you check and make sure I'm on the right track?
> >
> > Just a quick look;
> >
> > 1. There doesn't seem to be any consideration that there may be many
> > partial indexes which are suitable for the SAOP element:
> >
> > drop table if exists t;
> > create table t (a int);
> > insert into t select x/1000 from generate_series(1,1000000)x;
> > create index t_eq_1 on t (a) where a = 1;
> > create index t_eq_2 on t (a) where a = 2;
> > create index t_eq_3 on t (a) where a = 3;
> >
> > create index t_le_2 on t (a) where a <= 2;
> >
> > explain select * from t where a in(1,2,3); -- Uses t_le_2 twice rather
> > than the other two more suitable indexes.
> >
> > drop index t_le_2;
> > explain select * from t where a in(1,2,3); -- What I'd expect the
> > above query to produce.
> >
> > See: compare_path_costs_fuzzily()
> >
> > 2. Is there any point in trying the index again when this condition is
> > true: if (!clauseset.nonempty). Since you'll be looking for the same
> > column for the next element, shouldn't you do bms_del_member() on that
> > index? Then put an "if (bms_is_empty(suitable_indexes)) break;" before
> > the while loop so that you don't needlessly process the entire SAOP
> > array when you run out of suitable indexes.
> >
> > 3. Styistically, instead of using int index_pos, you can use
> > foreach_current_index(idx_lc).
> >
> > 4. I think the following code puts too much faith into there only
> > being 1 path produced. From a quick skim of the current code in
> > build_index_paths(), because you're requesting ST_BITMAPSCAN, we don't
> > seem to ever produce more than 1 path, but if that changed, then your
> > code would make the list contain too many paths.
> >
> > per_saop_paths = list_concat(per_saop_paths, indexpaths);
> >
> > 5. Minor detail, but there's a bit of inconsistency in how you're
> > checking for empty Lists. The preferred way is: list != NIL.
> >
> > 6. Are you sure you want to use predOK == true indexes? Do you have a
> > case where this new code can produce a better plan than if the predOK
> > index was used directly by the existing Path generation code? If so,
> > please provide examples.
> >
> > David