Re: BUG #18260: Unexpected error: "negative bitmapset member not allowed" triggered by multiple JOIN

On Wed, Dec 27, 2023 at 8:15 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> On Wed, Dec 27, 2023 at 6:18 PM PG Bug reporting form <
> noreply(at)postgresql(dot)org> wrote:
>
>> My fuzzer finds a bug in Postgres 17devel, which triggers an unexpected
>> error "ERROR: negative bitmapset member not allowed".
>
>
> Thank you for the report. This issue is caused by the way SJE removes
> PHVs. I have run out of time today, but I will look into it tomorrow.
>

I've looked into it further. When removing a useless join, we'd remove
PHVs that are not used at join partner rels or above the join. A PHV
that references the join's relid in ph_eval_at is logically "above" the
join and thus should not be removed. We added a check in 9a2dbc614 for
that:

!bms_is_member(ojrelid, phinfo->ph_eval_at)

During that time, join removal was only performed for left joins, so it
was not possible for 'ojrelid' to be negative. However, with the
introduction of the SJE feature, inner joins can also be removed, and
'ojrelid' is set to -1 in the inner join case. That's how we see this
error.

A straightforward way to fix this error is to skip checking ojrelid for
inner joins:

--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -456,7 +456,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo
*rel,
Assert(sjinfo == NULL || !bms_is_member(relid, phinfo->ph_lateral));
if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
bms_is_member(relid, phinfo->ph_eval_at) &&
- !bms_is_member(ojrelid, phinfo->ph_eval_at))
+ (sjinfo == NULL || !bms_is_member(ojrelid, phinfo->ph_eval_at)))

Alternatively, we can modify bms_is_member() to return false for
negative numbers instead of emitting an error, as suggested by the
comment there.

--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -477,9 +477,9 @@ bms_is_member(int x, const Bitmapset *a)
int wordnum,
bitnum;

- /* XXX better to just return false for x<0 ? */
+ /* negative number cannot be a member of the bitmapset */
if (x < 0)
- elog(ERROR, "negative bitmapset member not allowed");
+ return false;

I prefer the second option, but I'm open to other thoughts.

Attached is a patch for the second option.

FWIW, here is a simplified repro for this error.

create table t (a int primary key, b int);

explain (verbose, costs off)
select 1 from t t1 left join
(lateral (select 1 as x, * from t t2) s1 inner join
(select * from t t3) s2 on s1.a = s2.a)
on true
where s1.x = 1;
ERROR: negative bitmapset member not allowed

Thanks
Richard