Re: [PROPOSAL] Expand OR clauses in joins to UNION ALL paths

From: Ayoub Kazar <ayoub(dot)kazar(at)data-bene(dot)io>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PROPOSAL] Expand OR clauses in joins to UNION ALL paths
Date: 2026-10-28 13:39:04
Message-ID: 01bd4175-8346-4c1a-a362-27bb699b9ae8@data-bene.io
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 28/10/2026 02:32, Ayoub Kazar wrote:

> On 27/10/2026 23:30, Zsolt Parragi wrote:
>> Hello! v5 fails the basic test suite for me in an assert enabled build
> Apologies, i had inadvertently run the previous test cycle on a build
> without asserts and all TAP tests.
>
> The assertion failure tripped at:
>
> TRAP: failed Assert("list_length(dest_tlist) ==
> list_length(src_tlist)"), File: "tlist.c", Line: 332
> This occurred in queries with upper operations (aggregates, COUNT(*),
> or GROUP BY) atop an OR-expanded join. while or_expansion_align_path()
> projected the subpath to match the joinrel's targetlist,
> arm->subroot->processed_tlist still retained the upper query's
> targetlist. so the mismatch between the subplan's targetlist and
> subroot->processed_tlist triggered the assert.
>
> The fix for v5 is  just to synchronize the subroot's targetlist right
> after alignment:
>
>
> +   /* Synchronize subroot tlist with the arm's actual output target */
> +   arm->subroot->processed_tlist =
> +  arm->subroot->parse->targetList =
> +  make_tlist_from_pathtarget(aligned->pathtarget);
> With this fix in place, all assertion checks pass. I have attached a
> fixed v5.
>
> Later i found something else with pg_plan_advice story ; so v5 still
> fails in one test here:
>
> In v5, create_append_path() passed joinrel as the parent.
> Consequently, Append.apprelids contained the join's base relations
> (e.g., {a, b} or {generate_series, b}). This caused two problems for
> plan inspection:
>
> So this is the reason we see Partitionwise, if the transformation was
> doing something clean like an upper UNION ALL query with subqueries as
> relids, it would have the same rtekind for each branch and would be a
> normal Append from pg_plan_advice's perspective.
> E.g: joining a table with a function scan or VALUES (e.g.,
> generate_series(1, 4) g, table b WHERE g = b.x OR g = b.y),
> pg_plan_advice threw:
> ERROR: rtekind mismatch: 3 vs. 0
>
> And as mentioned, an equivalent manual UNION ALL query does not suffer
> from this, because a manual UNION ALL Append has an apprelids
> consisting of just subquery RTIs (RTE_SUBQUERY).
>
> So i thought of doing Set-Operation for AppendPath.
>
> Instead of creating the AppendPath with joinrel as its parent, we
> build it on an UPPERREL_SETOP upper relation keyed by the candidate
> arm subqueries' RTIs, and then add it to joinrel->pathlist:
>
> Now i'm not sure whether this is fine to do or not, especially given
> that this kind of transformations has never been done in any other
> place and this comes at an intermediate state of planning. At this
> point any issue with pg_plan_advice is gone, because the patch has a
> more standard-ish planning state than before.
>
> what do you think ?
>
>
> Regards,
> Ayoub

Here's a v6 (moving forward with this) with minor fixes, shouldn't fail
CI no more i think (had older gcc used without noticing).

Regards,
Ayoub

Attachment Content-Type Size
v6-0001-Expand-OR-clauses-in-joins-to-UNION-ALL-Append-paths.patch text/x-patch 103.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2026-10-28 13:53:05 Re: PoC: true in-place major version upgrades
Previous Message Greg Burd 2026-10-28 13:29:39 Re: WAL record CRC calculated incorrectly because of underlying buffer modification