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

On Sun, 11 Jan 2026 at 06:03, Jim Vanns <james(dot)vanns(at)gmail(dot)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