From 767c598954bbf72e0535f667e2e0667765604b2a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 12 Mar 2023 14:20:34 -0400 Subject: [PATCH] Work around implementation restriction in adjust_appendrel_attrs. adjust_appendrel_attrs can't transfer nullingrel labeling to a non-Var translation expression (mainly because it's too late to wrap such an expression in a PlaceHolderVar). I'd supposed in commit 2489d76c4 that that restriction was unreachable because we'd not attempt to push problematic clauses down to an appendrel child relation. I forgot that set_append_rel_size blindly converts all the parent rel's joininfo clauses to child clauses, and that list could well contain clauses from above a nulling outer join. We might eventually have to devise a direct fix for this implementation restriction, but for now it seems enough to filter out troublesome clauses while constructing the child's joininfo list. Such clauses are certainly not useful while constructing paths for the child rel; they'll have to be applied later when we join the completed appendrel to something else. So we don't need them here, and omitting them from the list should save a few cycles while processing the child rel. Per bug #17832 from Marko Tiikkaja. Discussion: https://postgr.es/m/17832-d0a8106cdf1b722e@postgresql.org --- src/backend/optimizer/path/allpaths.c | 28 +++++++++++++++++++++++---- src/test/regress/expected/join.out | 22 +++++++++++++++++++++ src/test/regress/sql/join.sql | 7 +++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 4a330192f2..479a694bca 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -979,8 +979,10 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, int childRTindex; RangeTblEntry *childRTE; RelOptInfo *childrel; + List *childrinfos; ListCell *parentvars; ListCell *childvars; + ListCell *lc; /* append_rel_list contains all append rels; ignore others */ if (appinfo->parent_relid != parentRTindex) @@ -1021,6 +1023,28 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, * Constraint exclusion failed, so copy the parent's join quals and * targetlist to the child, with appropriate variable substitutions. * + * We skip join quals that came from above outer joins that can null + * this rel, since they would be of no value while generating paths + * for the child. This saves some effort while processing the child + * rel, and it also avoids an implementation restriction in + * adjust_appendrel_attrs (it can't apply nullingrels to a non-Var). + */ + childrinfos = NIL; + foreach(lc, rel->joininfo) + { + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + + if (!bms_overlap(rinfo->clause_relids, rel->nulling_relids)) + childrinfos = lappend(childrinfos, + adjust_appendrel_attrs(root, + (Node *) rinfo, + 1, &appinfo)); + } + childrel->joininfo = childrinfos; + + /* + * Now for the child's targetlist. + * * NB: the resulting childrel->reltarget->exprs may contain arbitrary * expressions, which otherwise would not occur in a rel's targetlist. * Code that might be looking at an appendrel child must cope with @@ -1028,10 +1052,6 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, * PlaceHolderVars.) XXX we do not bother to update the cost or width * fields of childrel->reltarget; not clear if that would be useful. */ - childrel->joininfo = (List *) - adjust_appendrel_attrs(root, - (Node *) rel->joininfo, - 1, &appinfo); childrel->reltarget->exprs = (List *) adjust_appendrel_attrs(root, (Node *) rel->reltarget->exprs, diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index e293de03c0..5d59ed7890 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3613,6 +3613,28 @@ from int4_tbl t1 One-Time Filter: false (3 rows) +-- Test handling of qual pushdown to appendrel members with non-Var outputs +explain (verbose, costs off) +select * from int4_tbl left join ( + select text 'foo' union all select text 'bar' +) ss(x) on true +where ss.x is null; + QUERY PLAN +----------------------------------------- + Nested Loop Left Join + Output: int4_tbl.f1, ('foo'::text) + Filter: (('foo'::text) IS NULL) + -> Seq Scan on public.int4_tbl + Output: int4_tbl.f1 + -> Materialize + Output: ('foo'::text) + -> Append + -> Result + Output: 'foo'::text + -> Result + Output: 'bar'::text +(12 rows) + -- -- test inlining of immutable functions -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 5c0328ed76..a630f58b57 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1189,6 +1189,13 @@ from int4_tbl t1 left join information_schema.column_udt_usage on null) on null; +-- Test handling of qual pushdown to appendrel members with non-Var outputs +explain (verbose, costs off) +select * from int4_tbl left join ( + select text 'foo' union all select text 'bar' +) ss(x) on true +where ss.x is null; + -- -- test inlining of immutable functions -- -- 2.30.2