From 18653b8bc5ea1f4e55750f15d067e31fef3d5577 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Thu, 8 Oct 2026 09:31:06 +0900 Subject: [PATCH v1] Look through wrapper paths when collecting enforced clause serials get_param_path_clause_serials() handles join paths and Append paths explicitly, and for anything else returns the ppi_serials of the path's ParamPathInfo on the assumption that it is a baserel scan. But Material, Memoize, Projection, Sort, IncrementalSort, Unique, Agg and GroupingSets paths all inherit their subpath's ParamPathInfo, and that assumption fails when the subpath is an Append. The ParamPathInfo of an appendrel is built as for a scan of the parent and so claims the clauses such a scan would enforce, which the children need not be able to enforce. Append paths themselves are judged by intersecting their children, but a wrapper on top of one reported the parent's claim. This could drop a join clause for good. If a LATERAL UNION ALL subquery references an outer relation in an expression that becomes an EquivalenceClass member, the members get no child version of it, so their parameterized scans cannot enforce the derived clause. The appendrel's ParamPathInfo claims it nonetheless, and once a Materialize sat on the Append, create_nestloop_path() dropped the clause from the join above as already enforced, returning rows that fail it. To fix, make get_param_path_clause_serials() recurse through such wrapper paths, since they enforce no clauses of their own. Also use it in get_memoize_path(), which read ppi_serials directly for its inner-unique check. Back-patch to v16, where the serial-based detection of enforced clauses was introduced. Author: Richard Guo Discussion: https://postgr.es/m/ Backpatch-through: 16 --- src/backend/optimizer/path/joinpath.c | 6 ++-- src/backend/optimizer/util/relnode.c | 41 +++++++++++++++++++++++++++ src/include/nodes/pathnodes.h | 5 +++- src/test/regress/expected/join.out | 40 ++++++++++++++++++++++++++ src/test/regress/sql/join.sql | 18 ++++++++++++ 5 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index dfd08e7aeb1..91bac051fcc 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -801,16 +801,16 @@ get_memoize_path(PlannerInfo *root, RelOptInfo *innerrel, */ if (extra->inner_unique) { - Bitmapset *ppi_serials; + Bitmapset *pserials; if (inner_path->param_info == NULL) return NULL; - ppi_serials = inner_path->param_info->ppi_serials; + pserials = get_param_path_clause_serials(inner_path); foreach_node(RestrictInfo, rinfo, extra->restrictlist) { - if (!bms_is_member(rinfo->rinfo_serial, ppi_serials)) + if (!bms_is_member(rinfo->rinfo_serial, pserials)) return NULL; } } diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 34dfb757152..eaf90afd2d7 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -2244,6 +2244,47 @@ get_param_path_clause_serials(Path *path) } else { + Path *subpath; + + /* + * A path that merely wraps another path enforces no clauses of its + * own, so look through it. We can't just use its ppi_serials: that + * describes a baserel scan, and if the wrapped path is an Append, + * what it enforces depends on its children, as computed above. + */ + switch (nodeTag(path)) + { + case T_MaterialPath: + subpath = ((MaterialPath *) path)->subpath; + break; + case T_MemoizePath: + subpath = ((MemoizePath *) path)->subpath; + break; + case T_ProjectionPath: + subpath = ((ProjectionPath *) path)->subpath; + break; + case T_SortPath: + subpath = ((SortPath *) path)->subpath; + break; + case T_IncrementalSortPath: + subpath = ((IncrementalSortPath *) path)->spath.subpath; + break; + case T_UniquePath: + subpath = ((UniquePath *) path)->subpath; + break; + case T_AggPath: + subpath = ((AggPath *) path)->subpath; + break; + case T_GroupingSetsPath: + subpath = ((GroupingSetsPath *) path)->subpath; + break; + default: + subpath = NULL; + break; + } + if (subpath != NULL) + return get_param_path_clause_serials(subpath); + /* * Otherwise, it's a baserel path and we can use the * previously-computed set of serial numbers. diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 1c6d1fe3d04..e1fb5bc8d83 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1913,7 +1913,10 @@ typedef struct PathTarget * ppi_serials is the set of rinfo_serial numbers for quals that are enforced * by this path. As with ppi_clauses, it's only maintained for baserels. * (We could construct it on-the-fly from ppi_clauses, but it seems better - * to materialize a copy.) + * to materialize a copy.) For an appendrel it describes a scan of the + * parent; what an Append path enforces depends on its children, so callers + * should go through get_param_path_clause_serials() rather than use it + * directly. */ typedef struct ParamPathInfo { diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 4ee94d00ade..c34948fbd91 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -10744,6 +10744,46 @@ select * from 4567890123456789 | -4567890123456789 | 4567890123456789 | -4567890123456789 | | | (25 rows) +-- check that a clause the UNION ALL members cannot enforce is not dropped +-- from the join above as already enforced, even through a Materialize +explain (costs off) +select t1.f1 from int4_tbl t1 where t1.f1 in + (select s.v + t1.f1 from + (select q1 - 123 as v from int8_tbl where q2 < t1.f1 + union all + select q1 - 123 from int8_tbl where q2 < t1.f1) s, + int4_tbl t3) +order by 1; + QUERY PLAN +---------------------------------------------------------------- + Sort + Sort Key: t1.f1 + -> Nested Loop Semi Join + Join Filter: (t1.f1 = (((int8_tbl.q1 - 123)) + t1.f1)) + -> Seq Scan on int4_tbl t1 + -> Nested Loop + -> Seq Scan on int4_tbl t3 + -> Materialize + -> Append + -> Seq Scan on int8_tbl + Filter: (q2 < t1.f1) + -> Seq Scan on int8_tbl int8_tbl_1 + Filter: (q2 < t1.f1) +(13 rows) + +select t1.f1 from int4_tbl t1 where t1.f1 in + (select s.v + t1.f1 from + (select q1 - 123 as v from int8_tbl where q2 < t1.f1 + union all + select q1 - 123 from int8_tbl where q2 < t1.f1) s, + int4_tbl t3) +order by 1; + f1 +------------ + 123456 + 2147483647 +(2 rows) + -- lateral can result in join conditions appearing below their -- real semantic level explain (verbose, costs off) diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index bf8153afbcd..50ce03394c4 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -4090,6 +4090,24 @@ select * from on z.q2 = ss.v) on y.q1 = 1; +-- check that a clause the UNION ALL members cannot enforce is not dropped +-- from the join above as already enforced, even through a Materialize +explain (costs off) +select t1.f1 from int4_tbl t1 where t1.f1 in + (select s.v + t1.f1 from + (select q1 - 123 as v from int8_tbl where q2 < t1.f1 + union all + select q1 - 123 from int8_tbl where q2 < t1.f1) s, + int4_tbl t3) +order by 1; +select t1.f1 from int4_tbl t1 where t1.f1 in + (select s.v + t1.f1 from + (select q1 - 123 as v from int8_tbl where q2 < t1.f1 + union all + select q1 - 123 from int8_tbl where q2 < t1.f1) s, + int4_tbl t3) +order by 1; + -- lateral can result in join conditions appearing below their -- real semantic level explain (verbose, costs off) -- 2.37.1 (Apple Git-137.1)