BUG #19560: Wrong results since v16: removable LEFT JOIN silently drops WHERE qual

Started by PG Bug reporting form6 days ago2 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

The following bug has been logged on the website:

Bug reference: 19560
Logged by: Orestis Markou
Email address: orestis@orestis.gr
PostgreSQL version: 19beta2
Operating system: Debian (official Docker images), aarch64
Description:

I got a customer-reported bug and managed to narrow it down with the help of
Claude:

```
CREATE TABLE items (id text, owner text);
CREATE TABLE follows (item_id text, user_id text, UNIQUE (user_id,
item_id));
INSERT INTO items VALUES ('item1', 'alice');

WITH viewer AS (SELECT 'bob' AS id)
SELECT count(*) FROM items
LEFT JOIN follows ON follows.item_id = items.id AND follows.user_id = 'bob'
LEFT JOIN viewer ON TRUE
WHERE items.owner = viewer.id;
```

Expected 0 ('alice' <> 'bob'). Actual: 1. EXPLAIN shows the viewer join and
the WHERE qual gone entirely:

Aggregate
-> Seq Scan on items

Any ONE of these restores correct behavior:
- drop the UNIQUE constraint on follows (blocks its removal as a useless
join)
- WITH viewer AS MATERIALIZED
- WHERE items.owner IS NOT DISTINCT FROM viewer.id (non-strict qual)

So it needs: a removable LEFT JOIN elsewhere + a pulled-up one-row subquery
+ a strict qual on the subquery's column.

This is a regression from 15.18 to 16.0. I tested this exact same repro
across all major versions up until and including 19-beta2.

Related to BUG #19553 but a different defect: built 19beta2 from source,
applied the patch from that thread (find_dependent_phvs phrels
membership-vs-exact-match fix in prepjointree.c) — it fixes #19553's own
repro but does not fix this one (still returns 1, same plan).

Workaround: WHERE items.owner = (SELECT id FROM viewer) — scalar subquery
plans as an InitPlan Param, unaffected.

--

I've asked Claude to try and do an analysis of why this is caused and it
ended up with this (validated within the beta2 source code):

Root cause traced on 19beta2 (debug build; cassert raises nothing —
silent wrong results). Relids: 1=items, 2=follows, 3=follows-OJ,
4=viewer.

The chain:

* Pull-up wraps the CTE's constant in a PlaceHolderVar:
PHV(Const 'bob'), phrels={4} (nullable side of a LEFT JOIN).
* Strict WHERE qual => reduce_outer_joins_pass2() (prepjointree.c
~3501) reduces the viewer join to inner; phnullingrels becomes
empty. PHV is now semantically a plain constant.
* remove_useless_results_recurse() drops the viewer RTE_RESULT;
substitute_phv_relids() relocates phrels to the OTHER join side:
{1,2,3}. Phantom dependency on follows.
* make_eq_member() (equivclass.c ~605) tests
bms_is_empty(pull_varnos()); PHV returns phrels={1,2,3}, so
ec_has_const=false despite the Const inside.
* No-const path of generate_base_implied_equalities() (~1398) skips
non-singleton members => no "items.owner='bob'" base restriction.
The equality survives only as a future join clause at {1,2,3}.
* remove_useless_joins() removes the follows join
(join_is_removable() checks attr_needed/ph_eval_at, not EC relids).
remove_rel_from_eclass() (analyzejoins.c ~773) strips relids 2,3
but nothing re-derives the collapsed equality. Qual gone.

Instrumented EC state at removal (elog in remove_rel_from_eclass):
member Var items.owner em_relids {1}; member PHV{Const 'bob',
phrels {1,2,3}, phnullingrels {}} em_relids {1,2,3};
ec_has_const=false; ec_derives empty.

--

This is all going above my head, and I wouldn't dare propose a patch here,
but hopefully this analysis might save someone some time...

#2Nitin Motiani
nitinmotiani@google.com
In reply to: PG Bug reporting form (#1)
Re: BUG #19560: Wrong results since v16: removable LEFT JOIN silently drops WHERE qual

Hi,

Thanks for this detail. I'm not very familiar with this code but this
analysis helped me find the functions and understand it a little
better. I have started working on a WIP patch which I'm attaching
here. I think changing the PHV to the const if phnullingrels size goes
to zero might fix this.

I'm not sure of this approach because a comment warns against removing
PHV if phnullingrels are removed. But I have only done this in the
cases when `is_pseudo_constant_clause` returns true. I'll spend some
more time on this to understand the complete flow. For now, I'm
sending the WIP patch to get feedback.

Attachments:

WIP-v1-0001-WIP-Convert-PHVs-to-constants-if-phnullringrels-w.patchapplication/x-patch; name=WIP-v1-0001-WIP-Convert-PHVs-to-constants-if-phnullringrels-w.patchDownload+53-7