Re: Wrong results: hashed SubPlan referenced twice after OR-qual extraction reuses a stale hash table (13 to 19beta4)

From: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>
To: rahul(at)rhyadav(dot)dev
Cc: Samuel Olaoye <dapsalmy(at)gmail(dot)com>, Pgsql Bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: Wrong results: hashed SubPlan referenced twice after OR-qual extraction reuses a stale hash table (13 to 19beta4)
Date: 2026-10-16 23:26:54
Message-ID: CAK98qZ29+QuHHYkpuCN5ZrvHyrSAQnuCzC-rVa+54Fp5pMTiRw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi Rahul,

On Fri, Oct 16, 2026 at 3:37 PM Rahul Yadav wrote:
> The attached patch gives each SubPlanState its own flag marking its
> hash table as stale.

Thanks for the patch!

I tested v1-0001 on current master (6b3f8857aa8) and it
looks good to me overall.

One minor question: with hashtablestale in place, do we still need the
"planstate->chgParam != NULL" test in ExecHashSubPlan()?

I also found a related bug in the EPQ recheck code, using the same
query shape. Here is a minimal repro with the table and data from the
original report:

create table t_posts (id int primary key, channel_id int not null, status
text not null);
create table t_follows (channel_id int not null, user_id int not null,
primary key (channel_id, user_id));
create table t_sessions (id int primary key, post_id int not null,
viewer_id int not null);
insert into t_posts values (10, 1, 'published'), (20, 2, 'published');
insert into t_follows values (1, 501), (2, 502);
insert into t_sessions values (1, 10, 501), (2, 20, 502);
analyze t_posts, t_follows, t_sessions;

-- session 1
begin;
update t_posts set status = status;

-- session 2 (blocks)
select s.id,
exists (select 1 from t_posts p
where p.id = s.post_id
and (p.status = 'draft'
or p.channel_id in (select f.channel_id from
t_follows f
where f.user_id = s.viewer_id))
for update of p) as allowed -- FOR UPDATE is variant
from t_sessions s order by s.id;

-- session 1
commit;

Session 2 then returns (1, t), (2, f). Both rows should be true: each
viewer follows the channel of their own session's post. The same
query without the concurrent update returns (2, t), and so does a
concurrent update of post 20 only. I see the same result on master
and with v1 applied. The report in this thread is fixed by v1; it is
this FOR UPDATE variant that is still wrong.

This needs only ONE hashed reference, so it is not the
duplicated-SubPlan bug. The IN stays a hashed SubPlan inside an EPQ
recheck tree. The EXISTS returns after its first row, so the same
recheck tree is reused for the second session row. In the reuse
branch of EvalPlanQualBegin() the new values of all parent params are
copied, but only epqParam is marked changed:

rcplanstate->chgParam = bms_add_member(rcplanstate->chgParam,
epqstate->epqParam);

UpdateChangedParamSet() keeps only the params in each node's allParam,
so the hashed SubPlan's inner plan (allParam (b 0), where param 0 is
s.viewer_id) never sees a change. ExecHashSubPlan() then keeps
probing the table built for viewer 501.

Here is my proposed fix:

diff --git a/src/backend/executor/execMain.c
b/src/backend/executor/execMain.c
index 2519af9e778..2a265a7d381 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -3044,6 +3044,17 @@ EvalPlanQualBegin(EPQState *epqstate)
*/
rcplanstate->chgParam = bms_add_member(rcplanstate->chgParam,
epqstate->epqParam);
+
+ /*
+ * We just overwrote the values of the parent parameters above, so
+ * flag all the external parameters the tree depends on as changed.
+ * Otherwise a node that caches state computed from a parameter, such
+ * as a hashed SubPlan, would keep using the value from the previous
+ * EPQ evaluation. UpdateChangedParamSet() filters this set down to
+ * the parameters each descendant node actually depends on.
+ */
+ rcplanstate->chgParam = bms_add_members(rcplanstate->chgParam,
+ rcplanstate->plan->extParam);
}
}

The FOR UPDATE variant of the reporter's join query contains both
bugs: it returns (2, f) on master, with v1 alone, and with this change
alone, and (2, t) only with both applied.

Do you think this can go together with your fix? Or would you prefer
it as a separate patch? I am happy either way, and I can send the
isolation spec if that is useful.

Best,
Alex

--
Alexandra Wang
EDB: https://www.enterprisedb.com

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Manu 2026-10-17 07:05:31 Re: BUG #19764: ManualBug3 Assert failure in pgstat_setup_shared_refs crashes initdb
Previous Message Venu K 2026-10-16 23:06:13 Re: BUG #19768: JSON_TABLE: row/text comparisons in context item or PASSING give wrong results or spurious collation