public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v8 4/8] Propagate changes to indisclustered to child/parents
6+ messages / 3 participants
[nested] [flat]
* [PATCH v8 4/8] Propagate changes to indisclustered to child/parents
@ 2020-10-07 03:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw)
---
src/backend/commands/cluster.c | 109 ++++++++++++++++----------
src/backend/commands/indexcmds.c | 2 +
src/test/regress/expected/cluster.out | 46 +++++++++++
src/test/regress/sql/cluster.sql | 11 +++
4 files changed, 125 insertions(+), 43 deletions(-)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index cb4fc350c6..5c08f0642e 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
+static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index);
static List *get_tables_to_cluster(MemoryContext cluster_context);
static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
Oid indexOid);
@@ -516,66 +517,88 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
index_close(OldIndex, NoLock);
}
+/*
+ * Helper for mark_index_clustered
+ * Mark a single index as clustered or not.
+ * pg_index is passed by caller to avoid repeatedly re-opening it.
+ */
+static void
+set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index)
+{
+ HeapTuple indexTuple;
+ Form_pg_index indexForm;
+
+ indexTuple = SearchSysCacheCopy1(INDEXRELID,
+ ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(indexTuple))
+ elog(ERROR, "cache lookup failed for index %u", indexOid);
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+ /* this was checked earlier, but let's be real sure */
+ if (isclustered && !indexForm->indisvalid)
+ elog(ERROR, "cannot cluster on invalid index %u", indexOid);
+
+ indexForm->indisclustered = isclustered;
+ CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
+ heap_freetuple(indexTuple);
+}
+
/*
* mark_index_clustered: mark the specified index as the one clustered on
*
- * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
+ * With indexOid == InvalidOid, mark all indexes of rel not-clustered.
+ * Otherwise, mark children of the clustered index as clustered, and parents of
+ * other indexes as unclustered.
+ * We wish to maintain the following properties:
+ * 1) Only one index on a relation can be marked clustered at once
+ * 2) If a partitioned index is clustered, then all its children must be
+ * clustered.
*/
void
mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
{
- HeapTuple indexTuple;
- Form_pg_index indexForm;
- Relation pg_index;
- ListCell *index;
-
- /*
- * If the index is already marked clustered, no need to do anything.
- */
- if (OidIsValid(indexOid))
- {
- if (get_index_isclustered(indexOid))
- return;
- }
+ ListCell *lc, *lc2;
+ List *indexes;
+ Relation pg_index = table_open(IndexRelationId, RowExclusiveLock);
+ List *inh = find_all_inheritors(RelationGetRelid(rel), ShareRowExclusiveLock, NULL);
/*
* Check each index of the relation and set/clear the bit as needed.
+ * Iterate over the relation's children rather than the index's children
+ * since we need to unset cluster for indexes on intermediate children,
+ * too.
*/
- pg_index = table_open(IndexRelationId, RowExclusiveLock);
-
- foreach(index, RelationGetIndexList(rel))
+ foreach(lc, inh)
{
- Oid thisIndexOid = lfirst_oid(index);
-
- indexTuple = SearchSysCacheCopy1(INDEXRELID,
- ObjectIdGetDatum(thisIndexOid));
- if (!HeapTupleIsValid(indexTuple))
- elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
- indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+ Oid inhrelid = lfirst_oid(lc);
+ Relation thisrel = table_open(inhrelid, ShareRowExclusiveLock);
- /*
- * Unset the bit if set. We know it's wrong because we checked this
- * earlier.
- */
- if (indexForm->indisclustered)
+ indexes = RelationGetIndexList(thisrel);
+ foreach (lc2, indexes)
{
- indexForm->indisclustered = false;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
- else if (thisIndexOid == indexOid)
- {
- /* this was checked earlier, but let's be real sure */
- if (!indexForm->indisvalid)
- elog(ERROR, "cannot cluster on invalid index %u", indexOid);
- indexForm->indisclustered = true;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
+ bool isclustered;
+ Oid thisIndexOid = lfirst_oid(lc2);
+ List *parentoids = get_rel_relispartition(thisIndexOid) ?
+ get_partition_ancestors(thisIndexOid) : NIL;
- InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
- InvalidOid, is_internal);
+ /*
+ * A child of the clustered index must be set clustered;
+ * indexes which are not children of the clustered index are
+ * set unclustered
+ */
+ isclustered = (thisIndexOid == indexOid) ||
+ list_member_oid(parentoids, indexOid);
+ Assert(OidIsValid(indexOid) || !isclustered);
+ set_indisclustered(thisIndexOid, isclustered, pg_index);
+
+ InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
+ InvalidOid, is_internal);
+ }
- heap_freetuple(indexTuple);
+ list_free(indexes);
+ table_close(thisrel, ShareRowExclusiveLock);
}
+ list_free(inh);
table_close(pg_index, RowExclusiveLock);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..4ca1ffbfa4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -25,6 +25,7 @@
#include "catalog/catalog.h"
#include "catalog/index.h"
#include "catalog/indexing.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_inherits.h"
@@ -32,6 +33,7 @@
#include "catalog/pg_opfamily.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
+#include "commands/cluster.h"
#include "commands/comment.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index c74cfa88cc..1d436dfaae 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -495,6 +495,52 @@ Indexes:
"clstrpart_idx" btree (a) CLUSTER
Number of partitions: 3 (Use \d+ to list them.)
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a)
+
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
+ Partitioned table "public.clstrpart1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart FOR VALUES FROM (1) TO (10)
+Partition key: RANGE (a)
+Indexes:
+ "clstrpart1_a_idx" btree (a)
+ "clstrpart1_idx_2" btree (a) CLUSTER
+Number of partitions: 2 (Use \d+ to list them.)
+
-- Test CLUSTER with external tuplesorting
create table clstr_4 as select * from tenk1;
create index cluster_sort on clstr_4 (hundred, thousand, tenthous);
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 9bcc77695c..0ded2be1ca 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio
CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
\d clstrpart
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
-- Test CLUSTER with external tuplesorting
--
2.17.0
--O5XBE6gyVG5Rl6Rj
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0005-Invalidate-parent-indexes.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Remove no-op PlaceHolderVars
@ 2024-09-03 02:53 Richard Guo <[email protected]>
2024-09-03 03:31 ` Re: Remove no-op PlaceHolderVars Tom Lane <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2024-09-03 02:53 UTC (permalink / raw)
To: pgsql-hackers
In [1] there was a short discussion about removing no-op
PlaceHolderVars. This thread is for continuing that discussion.
We may insert PlaceHolderVars when pulling up a subquery that is
within the nullable side of an outer join: if subquery pullup needs to
replace a subquery-referencing Var that has nonempty varnullingrels
with an expression that is not simply a Var, we may need to wrap the
replacement expression into a PlaceHolderVar. However, if the outer
join above is later reduced to an inner join, the PHVs would become
no-ops with no phnullingrels bits.
I think it's always desirable to remove these no-op PlaceHolderVars
because they can constrain optimization opportunities. The issue
reported in [1] shows that they can block subexpression folding, which
can prevent an expression from being matched to an index. I provided
another example in that thread which shows that no-op PlaceHolderVars
can force join orders, potentially forcing us to resort to nestloop
join in some cases.
As explained in the comment of remove_nulling_relids_mutator, PHVs are
used in some cases to enforce separate identity of subexpressions. In
other cases, however, it should be safe to remove a PHV if its
phnullingrels becomes empty.
Attached is a WIP patch that marks PHVs that need to be kept because
they are serving to isolate subexpressions, and removes all other PHVs
in remove_nulling_relids_mutator if their phnullingrels bits become
empty. One problem with it is that a PHV's contained expression may
not have been fully preprocessed. For example if the PHV is used as a
qual clause, its contained expression is not processed for AND/OR
flattening, which could trigger the Assert in make_restrictinfo that
the clause shouldn't be an AND clause.
For example:
create table t (a boolean, b boolean);
select * from t t1 left join
lateral (select (t1.a and t1.b) as x, * from t t2) s on true
where s.x;
The other problem with this is that it breaks 17 test cases. We need
to modify them one by one to ensure that they still test what they are
supposed to, which is not a trivial task.
Before delving into these two problems, I'd like to know whether this
optimization is worthwhile, and whether I'm going in the right
direction.
[1] https://postgr.es/m/CAMbWs4-9dYrF44pkpkFrPoLXc_YH15DL8xT8J-oHggp_WvsLLA@mail.gmail.com
Thanks
Richard
Attachments:
[application/octet-stream] v1-0001-Remove-no-op-PlaceHolderVars.patch (7.2K, ../../CAMbWs48biJp-vof82PNP_LzzFkURh0W+RKt4phoML-MyYavgdg@mail.gmail.com/2-v1-0001-Remove-no-op-PlaceHolderVars.patch)
download | inline diff:
From 4cc4c985141c302a1579514472eb38942637464d Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Mon, 2 Sep 2024 17:01:20 +0900
Subject: [PATCH v1] Remove no-op PlaceHolderVars
We may insert PlaceHolderVars when pulling up a subquery that is
within the nullable side of an outer join. However, if the outer join
is later reduced to an inner join, the PHVs would become no-ops with
no phnullingrels bits.
It's always desirable to remove these no-op PlaceHolderVars because
they can constrain optimization opportunities, such as blocking
subexpression folding, or forcing join order. There is one case where
we need to keep a PHV even if its phnullingrels becomes empty: the PHV
is used to enforce separate identity of subexpressions.
This patch removes no-op PlaceHolderVars by marking PHVs that need to
be kept because they are serving to isolate subexpressions.
---
src/backend/optimizer/prep/prepjointree.c | 17 +++++++++++++++++
src/backend/optimizer/util/placeholder.c | 7 ++++---
src/backend/optimizer/util/restrictinfo.c | 9 +++++++--
src/backend/rewrite/rewriteManip.c | 15 ++++++++++++---
src/include/nodes/pathnodes.h | 7 +++++++
5 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 34fbf8ee23..25a4e826c5 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2442,6 +2442,14 @@ pullup_replace_vars_callback(Var *var,
make_placeholder_expr(rcon->root,
(Expr *) newnode,
bms_make_singleton(rcon->varno));
+
+ /*
+ * If the PHV is used to isolate subexpressions, mark it as
+ * needing to be kept.
+ */
+ if (rcon->wrap_non_vars)
+ ((PlaceHolderVar *) newnode)->needkeep = true;
+
/* cache it with the PHV, and with phlevelsup etc not set yet */
rcon->rv_cache[InvalidAttrNumber] = copyObject(newnode);
}
@@ -2462,6 +2470,7 @@ pullup_replace_vars_callback(Var *var,
if (need_phv)
{
bool wrap;
+ bool isolate_exprs = false;
if (newnode && IsA(newnode, Var) &&
((Var *) newnode)->varlevelsup == 0)
@@ -2494,6 +2503,7 @@ pullup_replace_vars_callback(Var *var,
{
/* Caller told us to wrap all non-Vars in a PlaceHolderVar */
wrap = true;
+ isolate_exprs = true;
}
else
{
@@ -2541,6 +2551,13 @@ pullup_replace_vars_callback(Var *var,
(Expr *) newnode,
bms_make_singleton(rcon->varno));
+ /*
+ * If the PHV is used to isolate subexpressions, mark it as
+ * needing to be kept.
+ */
+ if (isolate_exprs)
+ ((PlaceHolderVar *) newnode)->needkeep = true;
+
/*
* Cache it if possible (ie, if the attno is in range, which
* it probably always should be).
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index 81abadd6db..5c678db22d 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -44,9 +44,9 @@ static bool contain_placeholder_references_walker(Node *node,
* phrels is the syntactic location (as a set of relids) to attribute
* to the expression.
*
- * The caller is responsible for adjusting phlevelsup and phnullingrels
- * as needed. Because we do not know here which query level the PHV
- * will be associated with, it's important that this function touches
+ * The caller is responsible for adjusting phlevelsup, phnullingrels and
+ * needkeep as needed. Because we do not know here which query level the
+ * PHV will be associated with, it's important that this function touches
* only root->glob; messing with other parts of PlannerInfo would be
* likely to do the wrong thing.
*/
@@ -60,6 +60,7 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
phv->phnullingrels = NULL; /* caller may change this later */
phv->phid = ++(root->glob->lastPHId);
phv->phlevelsup = 0; /* caller may change this later */
+ phv->needkeep = false; /* caller may change this later */
return phv;
}
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 0b406e9334..8ec579e86e 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -87,8 +87,13 @@ make_restrictinfo(PlannerInfo *root,
incompatible_relids,
outer_relids);
- /* Shouldn't be an AND clause, else AND/OR flattening messed up */
- Assert(!is_andclause(clause));
+ /*
+ * XXX Normally it shouldn't be an AND clause, else AND/OR flattening
+ * messed up. An exception occurs if the clause was initially wrapped in
+ * a PlaceHolderVar and the PlaceHolderVar is removed afterward. In this
+ * case the clause may not have been processed for AND/OR flattening by
+ * preprocess_expression.
+ */
return make_restrictinfo_internal(root,
clause,
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index b20625fbd2..58b3e67a49 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1281,9 +1281,10 @@ remove_nulling_relids_mutator(Node *node,
{
/*
* Note: it might seem desirable to remove the PHV altogether if
- * phnullingrels goes to empty. Currently we dare not do that
- * because we use PHVs in some cases to enforce separate identity
- * of subexpressions; see wrap_non_vars usages in prepjointree.c.
+ * phnullingrels goes to empty. Currently we only dare to do that
+ * if the PHV is not marked as needing to be kept, because we use
+ * PHVs in some cases to enforce separate identity of
+ * subexpressions; see wrap_non_vars usages in prepjointree.c.
*/
/* Copy the PlaceHolderVar and mutate what's below ... */
phv = (PlaceHolderVar *)
@@ -1297,6 +1298,14 @@ remove_nulling_relids_mutator(Node *node,
phv->phrels = bms_difference(phv->phrels,
context->removable_relids);
Assert(!bms_is_empty(phv->phrels));
+
+ /*
+ * Remove the PHV altogether if it is not marked as needing to be
+ * kept and phnullingrels goes to empty.
+ */
+ if (!phv->needkeep && bms_is_empty(phv->phnullingrels))
+ return (Node *) phv->phexpr;
+
return (Node *) phv;
}
/* Otherwise fall through to copy the PlaceHolderVar normally */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 540d021592..ab9758587a 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2758,6 +2758,10 @@ typedef struct MergeScanSelCache
* level of a PlaceHolderVar might be a join rather than a base relation.
* Likewise, phnullingrels corresponds to varnullingrels.
*
+ * needkeep indicates whether the PHV needs to be kept when its phnullingrels
+ * becomes empty. This is set true in cases where the PHV is used to isolate
+ * subexpressions; see wrap_non_vars usages in prepjointree.c.
+ *
* Although the planner treats this as an expression node type, it is not
* recognized by the parser or executor, so we declare it here rather than
* in primnodes.h.
@@ -2796,6 +2800,9 @@ typedef struct PlaceHolderVar
/* > 0 if PHV belongs to outer query */
Index phlevelsup;
+
+ /* true if PHV needs to be kept */
+ bool needkeep;
} PlaceHolderVar;
/*
--
2.43.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Remove no-op PlaceHolderVars
2024-09-03 02:53 Remove no-op PlaceHolderVars Richard Guo <[email protected]>
@ 2024-09-03 03:31 ` Tom Lane <[email protected]>
2024-09-03 08:50 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2024-09-03 03:31 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: pgsql-hackers
Richard Guo <[email protected]> writes:
> Attached is a WIP patch that marks PHVs that need to be kept because
> they are serving to isolate subexpressions, and removes all other PHVs
> in remove_nulling_relids_mutator if their phnullingrels bits become
> empty. One problem with it is that a PHV's contained expression may
> not have been fully preprocessed.
Yeah. I've been mulling over how we could do this, and the real
problem is that the expression containing the PHV *has* been fully
preprocessed by the time we get to outer join strength reduction
(cf. file header comment in prepjointree.c). Simply dropping the PHV
can break various invariants that expression preprocessing is supposed
to establish, such as "no RelabelType directly above another" or "no
AND clause directly above another". I haven't thought of a reliable
way to fix that short of re-running eval_const_expressions afterwards,
which seems like a mighty expensive answer. We could try to make
remove_nulling_relids_mutator preserve all these invariants, but
keeping it in sync with what eval_const_expressions does seems like
a maintenance nightmare.
> The other problem with this is that it breaks 17 test cases.
I've not looked into that, but yeah, it would need some tedious
analysis to verify whether the changes are OK.
> Before delving into these two problems, I'd like to know whether this
> optimization is worthwhile, and whether I'm going in the right
> direction.
I believe this is an area worth working on. I've been wondering
whether it'd be better to handle the expression-identity problems by
introducing an "expression wrapper" node type that is distinct from
PHV and has the sole purpose of demarcating a subexpression we don't
want to be folded into its parent. I think that doesn't really move
the football in terms of fixing the problems mentioned above, but
perhaps it's conceptually cleaner than adding a bool field to PHV.
Another thought is that grouping sets provide one of the big reasons
why we need an "expression wrapper" or equivalent functionality.
So maybe we should try to move forward on your other patch to change
the representation of those before we spend too much effort here.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Remove no-op PlaceHolderVars
2024-09-03 02:53 Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2024-09-03 03:31 ` Re: Remove no-op PlaceHolderVars Tom Lane <[email protected]>
@ 2024-09-03 08:50 ` Richard Guo <[email protected]>
2026-01-16 02:49 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2024-09-03 08:50 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Tue, Sep 3, 2024 at 11:31 AM Tom Lane <[email protected]> wrote:
> Yeah. I've been mulling over how we could do this, and the real
> problem is that the expression containing the PHV *has* been fully
> preprocessed by the time we get to outer join strength reduction
> (cf. file header comment in prepjointree.c). Simply dropping the PHV
> can break various invariants that expression preprocessing is supposed
> to establish, such as "no RelabelType directly above another" or "no
> AND clause directly above another".
Yeah, this is what the problem is.
> I haven't thought of a reliable
> way to fix that short of re-running eval_const_expressions afterwards,
> which seems like a mighty expensive answer.
This seems likely to result in a lot of duplicate work.
> We could try to make
> remove_nulling_relids_mutator preserve all these invariants, but
> keeping it in sync with what eval_const_expressions does seems like
> a maintenance nightmare.
Yeah, and it seems that we also need to know the EXPRKIND code for the
expression containing the to-be-dropped PHV in remove_nulling_relids
to know how we should preserve all these invariants, which seems to
require a lot of code changes.
Looking at the routines run in preprocess_expression, most of them
recurse into PlaceHolderVars and preprocess the contained expressions.
The two exceptions are canonicalize_qual and make_ands_implicit. I
wonder if we can modify them to also recurse into PlaceHolderVars.
Will this resolve our problem here?
> I believe this is an area worth working on. I've been wondering
> whether it'd be better to handle the expression-identity problems by
> introducing an "expression wrapper" node type that is distinct from
> PHV and has the sole purpose of demarcating a subexpression we don't
> want to be folded into its parent. I think that doesn't really move
> the football in terms of fixing the problems mentioned above, but
> perhaps it's conceptually cleaner than adding a bool field to PHV.
>
> Another thought is that grouping sets provide one of the big reasons
> why we need an "expression wrapper" or equivalent functionality.
> So maybe we should try to move forward on your other patch to change
> the representation of those before we spend too much effort here.
Hmm, in the case of grouping sets, we have a similar situation where
we do not want a subexpression that is part of grouping items to be
folded into its parent, because otherwise we may fail to match these
subexpressions to lower target items.
For grouping sets, this problem is much easier to resolve, because
we've already replaced such subexpressions with Vars referencing the
GROUP RTE. As a result, during expression preprocessing, these
subexpressions will not be folded into their parents.
An ensuing effect of this approach is that a HAVING clause may contain
expressions that are not fully preprocessed if they are part of
grouping items. This is not an issue as long as the clause remains in
HAVING, because these expressions will be matched to lower target
items in setrefs.c. However, if the clause is moved or copied into
WHERE, we need to re-preprocess these expressions. This should not be
too expensive, as we only need to re-preprocess the HAVING clauses
that are moved to WHERE, not the entire tree. The v13 patch in that
thread implements this approach.
Thanks
Richard
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Remove no-op PlaceHolderVars
2024-09-03 02:53 Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2024-09-03 03:31 ` Re: Remove no-op PlaceHolderVars Tom Lane <[email protected]>
2024-09-03 08:50 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
@ 2026-01-16 02:49 ` Richard Guo <[email protected]>
2026-01-21 08:18 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2026-01-16 02:49 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Tue, Sep 3, 2024 at 5:50 PM Richard Guo <[email protected]> wrote:
> On Tue, Sep 3, 2024 at 11:31 AM Tom Lane <[email protected]> wrote:
> > Yeah. I've been mulling over how we could do this, and the real
> > problem is that the expression containing the PHV *has* been fully
> > preprocessed by the time we get to outer join strength reduction
> > (cf. file header comment in prepjointree.c). Simply dropping the PHV
> > can break various invariants that expression preprocessing is supposed
> > to establish, such as "no RelabelType directly above another" or "no
> > AND clause directly above another".
> Yeah, this is what the problem is.
While fixing some performance issues caused by PHVs recently, it
struck me again that we should be removing "no-op" PHVs whenever
possible, because PHVs can be optimization barriers in several cases.
I still do not have a good idea for ensuring that removing the PHV
wrapper does not break the expression tree invariants. But maybe we
can use a conservative approach: we only strip the PHV if the
contained expression is known to be safe (for example, a simple Var).
I've also realized that we cannot remove no-op PHVs in every case.
For example, deconstruct_distribute_oj_quals() relies on
remove_nulling_relids() to temporarily strip out the nullingrels bits
that are later restored as we crawl up the join stack. If the PHV
were removed, we would be unable to restore its nullingrels bits for
next level up. To handle this, I added a new parameter to
remove_nulling_relids() so the caller can decide whether to allow
removal or not.
Attached is a draft patch for the code changes. It currently causes
plan changes in 28 regression test queries, which is a surprisingly
high number. We'll have to go through these tests one by one, but
before doing that, I would like to hear others' thoughts on this
patch.
- Richard
Attachments:
[application/octet-stream] v2-0001-Remove-no-op-PlaceHolderVars.patch (19.8K, ../../CAMbWs4-_fDPOiYJWj330pJS++kKbTw_EfO5-pC8nPKHUAScMWw@mail.gmail.com/2-v2-0001-Remove-no-op-PlaceHolderVars.patch)
download | inline diff:
From a41c27b140000a3b72f1b277dd21b2a3e93f5517 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Thu, 15 Jan 2026 11:43:17 +0900
Subject: [PATCH v2] Remove no-op PlaceHolderVars
When pulling up a subquery, we may need to wrap its targetlist items
in PlaceHolderVars if the subquery is under an outer join. This
ensures that they are evaluated at the right place and are forced to
NULL when the outer join should do so. However, if the outer join is
later reduced to an inner join, these PlaceHolderVars effectively
become no-ops regarding nullability.
It is desirable to remove these no-op PlaceHolderVars because they can
constrain optimization opportunities, such as blocking subexpression
folding or forcing join order. Previously, however, we did not try to
remove them because PHVs are also used to enforce separate identity of
subexpressions, and we could not distinguish between the two cases.
Additionally, simply removing the PHV wrapper could expose the
contained expression in a way that violates various invariants
established by expression preprocessing, such as creating nested AND
clauses or stacked RelabelType nodes.
This patch removes no-op PlaceHolderVars by marking PHVs as preserved
if they are used to enforce subexpression identity. Furthermore, to
ensure that stripping the PHV wrapper does not violate expression tree
invariants, we only do so when the contained expression is known to be
safe to expose to the surrounding expression structure.
---
src/backend/optimizer/path/equivclass.c | 4 +-
src/backend/optimizer/path/pathkeys.c | 2 +-
src/backend/optimizer/plan/initsplan.c | 6 +-
src/backend/optimizer/plan/planner.c | 4 +-
src/backend/optimizer/plan/setrefs.c | 4 +-
src/backend/optimizer/prep/prepjointree.c | 41 +++++++++++---
src/backend/optimizer/util/placeholder.c | 11 ++--
src/backend/optimizer/util/relnode.c | 4 +-
src/backend/optimizer/util/tlist.c | 2 +-
src/backend/rewrite/rewriteManip.c | 69 +++++++++++++++++++++--
src/backend/utils/adt/selfuncs.c | 8 ++-
src/include/nodes/pathnodes.h | 8 +++
src/include/rewrite/rewriteManip.h | 3 +-
13 files changed, 132 insertions(+), 34 deletions(-)
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index a4fcfcc86c8..d1dd6ba725f 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -2471,8 +2471,8 @@ reconsider_full_join_clause(PlannerInfo *root, OuterJoinClauseInfo *ojcinfo)
* representation for FULL JOIN USING output columns, this
* wouldn't be needed?)
*/
- cfirst = remove_nulling_relids(cfirst, fjrelids, NULL);
- csecond = remove_nulling_relids(csecond, fjrelids, NULL);
+ cfirst = remove_nulling_relids(cfirst, fjrelids, NULL, false);
+ csecond = remove_nulling_relids(csecond, fjrelids, NULL, false);
if (equal(leftvar, cfirst) && equal(rightvar, csecond))
{
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 5eb71635d15..0b610e46df8 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -1408,7 +1408,7 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
sortkey = (Expr *)
remove_nulling_relids((Node *) sortkey,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
pathkey = make_pathkey_from_sortop(root,
sortkey,
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 97ea95a4eb8..071b39df0d4 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2643,13 +2643,15 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
*
* We first strip out all the nullingrels bits corresponding to
* commuting joins below this one, and then successively put them back
- * as we crawl up the join stack.
+ * as we crawl up the join stack. Note that we do not allow stripping
+ * no-op PlaceHolderVars here; if a PHV were removed, we would be
+ * unable to restore its nullingrels bits later.
*/
quals = jtitem->oj_joinclauses;
if (!bms_is_empty(joins_below))
quals = (List *) remove_nulling_relids((Node *) quals,
joins_below,
- NULL);
+ NULL, false);
/*
* We'll need to mark the lower versions of the quals as not safe to
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7a6b8b749f2..9b897721de6 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5584,7 +5584,7 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target)
expr = (Expr *)
remove_nulling_relids((Node *) expr,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
add_column_to_pathtarget(input_target, expr, sgref);
}
@@ -5628,7 +5628,7 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target)
non_group_vars = (List *)
remove_nulling_relids((Node *) non_group_vars,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
add_new_columns_to_pathtarget(input_target, non_group_vars);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 16d200cfb46..e63d52f0d8c 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2523,11 +2523,11 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
plan->targetlist = (List *)
remove_nulling_relids((Node *) plan->targetlist,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
plan->qual = (List *)
remove_nulling_relids((Node *) plan->qual,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
output_targetlist = NIL;
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index c80bfc88d82..9e93775d91c 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2725,8 +2725,9 @@ pullup_replace_vars_callback(Var *var,
* references to the same subquery output as being equal(). So it's worth
* a bit of extra effort to avoid it.
*
- * The cached items have phlevelsup = 0 and phnullingrels = NULL; we'll
- * copy them and adjust those values for this reference site below.
+ * The cached items have phlevelsup = 0, phnullingrels = NULL, and
+ * phpreserved = false. We'll copy them and adjust phpreserved here; the
+ * other values are adjusted for this reference site below.
*/
if (need_phv &&
varattno >= InvalidAttrNumber &&
@@ -2735,6 +2736,13 @@ pullup_replace_vars_callback(Var *var,
{
/* Just copy the entry and fall through to adjust phlevelsup etc */
newnode = copyObject(rcon->rv_cache[varattno]);
+
+ /*
+ * Mark the PlaceHolderVar as preserved if it's used for
+ * identification purposes.
+ */
+ if (rcon->wrap_option != REPLACE_WRAP_NONE)
+ ((PlaceHolderVar *) newnode)->phpreserved = true;
}
else
{
@@ -2920,6 +2928,13 @@ pullup_replace_vars_callback(Var *var,
if (varattno >= InvalidAttrNumber &&
varattno <= list_length(rcon->targetlist))
rcon->rv_cache[varattno] = copyObject(newnode);
+
+ /*
+ * Mark the PlaceHolderVar as preserved if it's used for
+ * identification purposes.
+ */
+ if (rcon->wrap_option != REPLACE_WRAP_NONE)
+ ((PlaceHolderVar *) newnode)->phpreserved = true;
}
}
}
@@ -3196,23 +3211,28 @@ reduce_outer_joins(PlannerInfo *root)
* remove references to those joins as nulling rels. This is handled as
* an additional pass, for simplicity and because we can handle all
* fully-reduced joins in a single pass over the parse tree.
+ *
+ * We allow stripping no-op PlaceHolderVars here, as this is a
+ * tree-simplification pass and we want to remove useless wrappers.
*/
if (!bms_is_empty(state2.inner_reduced))
{
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
state2.inner_reduced,
- NULL);
+ NULL, true);
/* There could be references in the append_rel_list, too */
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
state2.inner_reduced,
- NULL);
+ NULL, true);
}
/*
* Partially-reduced full joins have to be done one at a time, since
* they'll each need a different setting of except_relids.
+ *
+ * As above, we allow stripping no-op PlaceHolderVars.
*/
foreach(lc, state2.partial_reduced)
{
@@ -3222,11 +3242,13 @@ reduce_outer_joins(PlannerInfo *root)
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
full_join_relids,
- statep->unreduced_side);
+ statep->unreduced_side,
+ true);
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
full_join_relids,
- statep->unreduced_side);
+ statep->unreduced_side,
+ true);
}
}
@@ -3680,18 +3702,21 @@ remove_useless_result_rtes(PlannerInfo *root)
* know that such an outer join wouldn't really have nulled anything.) We
* don't do this during the main recursion, for simplicity and because we
* can handle all such joins in a single pass over the parse tree.
+ *
+ * We allow stripping no-op PlaceHolderVars here, as this is a
+ * tree-simplification pass and we want to remove useless wrappers.
*/
if (!bms_is_empty(dropped_outer_joins))
{
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
dropped_outer_joins,
- NULL);
+ NULL, true);
/* There could be references in the append_rel_list, too */
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
dropped_outer_joins,
- NULL);
+ NULL, true);
}
/*
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index e1706363c88..4279fd85117 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -44,11 +44,11 @@ static bool contain_placeholder_references_walker(Node *node,
* phrels is the syntactic location (as a set of relids) to attribute
* to the expression.
*
- * The caller is responsible for adjusting phlevelsup and phnullingrels
- * as needed. Because we do not know here which query level the PHV
- * will be associated with, it's important that this function touches
- * only root->glob; messing with other parts of PlannerInfo would be
- * likely to do the wrong thing.
+ * The caller is responsible for adjusting phlevelsup, phnullingrels, and
+ * phpreserved as needed. Because we do not know here which query level
+ * the PHV will be associated with, it's important that this function
+ * touches only root->glob; messing with other parts of PlannerInfo
+ * would be likely to do the wrong thing.
*/
PlaceHolderVar *
make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
@@ -58,6 +58,7 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
phv->phexpr = expr;
phv->phrels = phrels;
phv->phnullingrels = NULL; /* caller may change this later */
+ phv->phpreserved = false; /* caller may change this later */
phv->phid = ++(root->glob->lastPHId);
phv->phlevelsup = 0; /* caller may change this later */
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index f57631e876f..2d4574ce916 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -2264,11 +2264,11 @@ have_partkey_equi_join(PlannerInfo *root, RelOptInfo *joinrel,
if (bms_overlap(rel1->relids, root->outer_join_rels))
expr1 = (Expr *) remove_nulling_relids((Node *) expr1,
root->outer_join_rels,
- NULL);
+ NULL, false);
if (bms_overlap(rel2->relids, root->outer_join_rels))
expr2 = (Expr *) remove_nulling_relids((Node *) expr2,
root->outer_join_rels,
- NULL);
+ NULL, false);
}
/*
diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 752ea9222f6..ca66b07009e 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -1161,7 +1161,7 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
sanitized_node =
remove_nulling_relids(node,
bms_make_singleton(context->root->group_rtindex),
- NULL);
+ NULL, true);
}
/*
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 6fa174412f2..16c295b2c14 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -53,6 +53,7 @@ typedef struct
const Bitmapset *removable_relids;
const Bitmapset *except_relids;
int sublevels_up;
+ bool remove_noop_phvs;
} remove_nulling_relids_context;
static bool contain_aggs_of_level_walker(Node *node,
@@ -68,6 +69,7 @@ static Node *add_nulling_relids_mutator(Node *node,
add_nulling_relids_context *context);
static Node *remove_nulling_relids_mutator(Node *node,
remove_nulling_relids_context *context);
+static bool is_safe_to_strip_payload(Node *node);
/*
@@ -1323,17 +1325,22 @@ add_nulling_relids_mutator(Node *node,
* in Var.varnullingrels and PlaceHolderVar.phnullingrels fields within
* the given expression, except in nodes belonging to rels listed in
* except_relids.
+ *
+ * If remove_noop_phvs is true, we also check if PlaceHolderVars become
+ * no-ops and strip them if so.
*/
Node *
remove_nulling_relids(Node *node,
const Bitmapset *removable_relids,
- const Bitmapset *except_relids)
+ const Bitmapset *except_relids,
+ bool remove_noop_phvs)
{
remove_nulling_relids_context context;
context.removable_relids = removable_relids;
context.except_relids = except_relids;
context.sublevels_up = 0;
+ context.remove_noop_phvs = remove_noop_phvs;
return query_or_expression_tree_mutator(node,
remove_nulling_relids_mutator,
&context,
@@ -1371,11 +1378,15 @@ remove_nulling_relids_mutator(Node *node,
!bms_overlap(phv->phrels, context->except_relids))
{
/*
- * Note: it might seem desirable to remove the PHV altogether if
- * phnullingrels goes to empty. Currently we dare not do that
- * because we use PHVs in some cases to enforce separate identity
- * of subexpressions; see wrap_option usages in prepjointree.c.
+ * If phnullingrels goes to empty, the PHV is no longer needed for
+ * outer-join nulling. We can remove it if we are allowed to
+ * remove no-op PHVs, provided that it is not marked as preserved
+ * (which indicates that it is needed to enforce separate identity
+ * of subexpressions) and that the contained expression is safe to
+ * pull up without breaking various invariants established by
+ * expression preprocessing.
*/
+
/* Copy the PlaceHolderVar and mutate what's below ... */
phv = (PlaceHolderVar *)
expression_tree_mutator(node,
@@ -1388,6 +1399,14 @@ remove_nulling_relids_mutator(Node *node,
phv->phrels = bms_difference(phv->phrels,
context->removable_relids);
Assert(!bms_is_empty(phv->phrels));
+
+ /* Strip the PHV if it's safe; see comment above */
+ if (context->remove_noop_phvs &&
+ !phv->phpreserved &&
+ bms_is_empty(phv->phnullingrels) &&
+ is_safe_to_strip_payload((Node *) phv->phexpr))
+ return (Node *) phv->phexpr;
+
return (Node *) phv;
}
/* Otherwise fall through to copy the PlaceHolderVar normally */
@@ -1408,6 +1427,46 @@ remove_nulling_relids_mutator(Node *node,
return expression_tree_mutator(node, remove_nulling_relids_mutator, context);
}
+/*
+ * is_safe_to_strip_payload
+ *
+ * Check whether the given node is safe to pull up into the surrounding
+ * expression structure.
+ *
+ * This is used to remove a PlaceHolderVar. Even if a PlaceHolderVar is no
+ * longer needed, we cannot simply remove it if doing so would expose the
+ * contained expression to a parent node in a way that breaks various
+ * invariants established by earlier expression preprocessing.
+ *
+ * Since we don't know what the parent node is, we need to be conservative.
+ */
+static bool
+is_safe_to_strip_payload(Node *node)
+{
+ if (node == NULL)
+ return false;
+
+ switch (nodeTag(node))
+ {
+ case T_Var:
+ case T_Param:
+ case T_Const:
+ case T_PlaceHolderVar:
+ /* Atomic nodes should be safe to expose */
+ return true;
+
+ case T_OpExpr:
+ case T_FuncExpr:
+ case T_CoalesceExpr:
+ return true;
+
+ default:
+ return false;
+ }
+
+ return false;
+}
+
/*
* replace_rte_variables() finds all Vars in an expression tree
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 29fec655593..fdf328c0fac 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -3655,7 +3655,7 @@ add_unique_group_var(PlannerInfo *root, List *varinfos,
* extended statistics (see estimate_multivariate_ndistinct). So strip
* them out first.
*/
- var = remove_nulling_relids(var, root->outer_join_rels, NULL);
+ var = remove_nulling_relids(var, root->outer_join_rels, NULL, false);
foreach(lc, varinfos)
{
@@ -4236,7 +4236,8 @@ estimate_multivariate_bucketsize(PlannerInfo *root, RelOptInfo *inner,
* Clear nullingrels to correctly match hash keys. See
* add_unique_group_var()'s comment for details.
*/
- expr = remove_nulling_relids(expr, root->outer_join_rels, NULL);
+ expr = remove_nulling_relids(expr, root->outer_join_rels,
+ NULL, false);
/*
* Detect and exclude exact duplicates from the list of hash
@@ -5762,7 +5763,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
* extended statistics. So strip them out first.
*/
if (bms_overlap(varnos, root->outer_join_rels))
- node = remove_nulling_relids(node, root->outer_join_rels, NULL);
+ node = remove_nulling_relids(node, root->outer_join_rels,
+ NULL, false);
foreach(ilist, onerel->indexlist)
{
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 449885b9319..c4671ef362a 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2985,6 +2985,11 @@ typedef struct MergeScanSelCache
* level of a PlaceHolderVar might be a join rather than a base relation.
* Likewise, phnullingrels corresponds to varnullingrels.
*
+ * phpreserved indicates whether the PlaceHolderVar needs to be preserved
+ * when its phnullingrels becomes empty. This is set true in cases where
+ * the PlaceHolderVar is used to enforce the separate identity of the
+ * contained expression.
+ *
* Although the planner treats this as an expression node type, it is not
* recognized by the parser or executor, so we declare it here rather than
* in primnodes.h.
@@ -3018,6 +3023,9 @@ typedef struct PlaceHolderVar
/* RT indexes of outer joins that can null PHV's value */
Relids phnullingrels;
+ /* true if PHV enforces separate identity */
+ bool phpreserved pg_node_attr(equal_ignore);
+
/* ID for PHV (unique within planner run) */
Index phid;
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index f8216c22fb7..05245faa49c 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -89,7 +89,8 @@ extern Node *add_nulling_relids(Node *node,
const Bitmapset *added_relids);
extern Node *remove_nulling_relids(Node *node,
const Bitmapset *removable_relids,
- const Bitmapset *except_relids);
+ const Bitmapset *except_relids,
+ bool remove_noop_phvs);
extern Node *replace_rte_variables(Node *node,
int target_varno, int sublevels_up,
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Remove no-op PlaceHolderVars
2024-09-03 02:53 Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2024-09-03 03:31 ` Re: Remove no-op PlaceHolderVars Tom Lane <[email protected]>
2024-09-03 08:50 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2026-01-16 02:49 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
@ 2026-01-21 08:18 ` Richard Guo <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Richard Guo @ 2026-01-21 08:18 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Fri, Jan 16, 2026 at 11:49 AM Richard Guo <[email protected]> wrote:
> Attached is a draft patch for the code changes. It currently causes
> plan changes in 28 regression test queries, which is a surprisingly
> high number. We'll have to go through these tests one by one, but
> before doing that, I would like to hear others' thoughts on this
> patch.
I went through those tests one by one, and AFAICS the plan changes are
all positive.
However, these plan changes resulted in a loss of coverage in tests
specifically designed to verify the planner's handling of PHVs. So
I've modified the affected queries to force the preservation of PHVs
by changing the target expressions into forms that the new logic
considers "unsafe to strip". Please see attached.
- Richard
Attachments:
[application/octet-stream] v3-0001-Remove-no-op-PlaceHolderVars.patch (19.8K, ../../CAMbWs4-qJGVHLonakB+5C_DvhA0i7Wua4StzG4n0F42i+dB+oA@mail.gmail.com/2-v3-0001-Remove-no-op-PlaceHolderVars.patch)
download | inline diff:
From 760685f6f4ff4bee693af3297edbcaeeedc2cda6 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Thu, 15 Jan 2026 11:43:17 +0900
Subject: [PATCH v3 1/2] Remove no-op PlaceHolderVars
When pulling up a subquery, we may need to wrap its targetlist items
in PlaceHolderVars if the subquery is under an outer join. This
ensures that they are evaluated at the right place and are forced to
NULL when the outer join should do so. However, if the outer join is
later reduced to an inner join, these PlaceHolderVars effectively
become no-ops regarding nullability.
It is desirable to remove these no-op PlaceHolderVars because they can
constrain optimization opportunities, such as blocking subexpression
folding or forcing join order. Previously, however, we did not try to
remove them because PHVs are also used to enforce separate identity of
subexpressions, and we could not distinguish between the two cases.
Additionally, simply removing the PHV wrapper could expose the
contained expression in a way that violates various invariants
established by expression preprocessing, such as creating nested AND
clauses or stacked RelabelType nodes.
This patch removes no-op PlaceHolderVars by marking PHVs as preserved
if they are used to enforce subexpression identity. Furthermore, to
ensure that stripping the PHV wrapper does not violate expression tree
invariants, we only do so when the contained expression is known to be
safe to expose to the surrounding expression structure.
---
src/backend/optimizer/path/equivclass.c | 4 +-
src/backend/optimizer/path/pathkeys.c | 2 +-
src/backend/optimizer/plan/initsplan.c | 6 +-
src/backend/optimizer/plan/planner.c | 4 +-
src/backend/optimizer/plan/setrefs.c | 4 +-
src/backend/optimizer/prep/prepjointree.c | 41 +++++++++++---
src/backend/optimizer/util/placeholder.c | 11 ++--
src/backend/optimizer/util/relnode.c | 4 +-
src/backend/optimizer/util/tlist.c | 2 +-
src/backend/rewrite/rewriteManip.c | 67 +++++++++++++++++++++--
src/backend/utils/adt/selfuncs.c | 8 ++-
src/include/nodes/pathnodes.h | 8 +++
src/include/rewrite/rewriteManip.h | 3 +-
13 files changed, 130 insertions(+), 34 deletions(-)
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index a4fcfcc86c8..d1dd6ba725f 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -2471,8 +2471,8 @@ reconsider_full_join_clause(PlannerInfo *root, OuterJoinClauseInfo *ojcinfo)
* representation for FULL JOIN USING output columns, this
* wouldn't be needed?)
*/
- cfirst = remove_nulling_relids(cfirst, fjrelids, NULL);
- csecond = remove_nulling_relids(csecond, fjrelids, NULL);
+ cfirst = remove_nulling_relids(cfirst, fjrelids, NULL, false);
+ csecond = remove_nulling_relids(csecond, fjrelids, NULL, false);
if (equal(leftvar, cfirst) && equal(rightvar, csecond))
{
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 5eb71635d15..0b610e46df8 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -1408,7 +1408,7 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
sortkey = (Expr *)
remove_nulling_relids((Node *) sortkey,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
pathkey = make_pathkey_from_sortop(root,
sortkey,
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 97ea95a4eb8..071b39df0d4 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2643,13 +2643,15 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
*
* We first strip out all the nullingrels bits corresponding to
* commuting joins below this one, and then successively put them back
- * as we crawl up the join stack.
+ * as we crawl up the join stack. Note that we do not allow stripping
+ * no-op PlaceHolderVars here; if a PHV were removed, we would be
+ * unable to restore its nullingrels bits later.
*/
quals = jtitem->oj_joinclauses;
if (!bms_is_empty(joins_below))
quals = (List *) remove_nulling_relids((Node *) quals,
joins_below,
- NULL);
+ NULL, false);
/*
* We'll need to mark the lower versions of the quals as not safe to
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7a6b8b749f2..9b897721de6 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5584,7 +5584,7 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target)
expr = (Expr *)
remove_nulling_relids((Node *) expr,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
add_column_to_pathtarget(input_target, expr, sgref);
}
@@ -5628,7 +5628,7 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target)
non_group_vars = (List *)
remove_nulling_relids((Node *) non_group_vars,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
add_new_columns_to_pathtarget(input_target, non_group_vars);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 16d200cfb46..e63d52f0d8c 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2523,11 +2523,11 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
plan->targetlist = (List *)
remove_nulling_relids((Node *) plan->targetlist,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
plan->qual = (List *)
remove_nulling_relids((Node *) plan->qual,
bms_make_singleton(root->group_rtindex),
- NULL);
+ NULL, true);
}
output_targetlist = NIL;
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index c80bfc88d82..9e93775d91c 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2725,8 +2725,9 @@ pullup_replace_vars_callback(Var *var,
* references to the same subquery output as being equal(). So it's worth
* a bit of extra effort to avoid it.
*
- * The cached items have phlevelsup = 0 and phnullingrels = NULL; we'll
- * copy them and adjust those values for this reference site below.
+ * The cached items have phlevelsup = 0, phnullingrels = NULL, and
+ * phpreserved = false. We'll copy them and adjust phpreserved here; the
+ * other values are adjusted for this reference site below.
*/
if (need_phv &&
varattno >= InvalidAttrNumber &&
@@ -2735,6 +2736,13 @@ pullup_replace_vars_callback(Var *var,
{
/* Just copy the entry and fall through to adjust phlevelsup etc */
newnode = copyObject(rcon->rv_cache[varattno]);
+
+ /*
+ * Mark the PlaceHolderVar as preserved if it's used for
+ * identification purposes.
+ */
+ if (rcon->wrap_option != REPLACE_WRAP_NONE)
+ ((PlaceHolderVar *) newnode)->phpreserved = true;
}
else
{
@@ -2920,6 +2928,13 @@ pullup_replace_vars_callback(Var *var,
if (varattno >= InvalidAttrNumber &&
varattno <= list_length(rcon->targetlist))
rcon->rv_cache[varattno] = copyObject(newnode);
+
+ /*
+ * Mark the PlaceHolderVar as preserved if it's used for
+ * identification purposes.
+ */
+ if (rcon->wrap_option != REPLACE_WRAP_NONE)
+ ((PlaceHolderVar *) newnode)->phpreserved = true;
}
}
}
@@ -3196,23 +3211,28 @@ reduce_outer_joins(PlannerInfo *root)
* remove references to those joins as nulling rels. This is handled as
* an additional pass, for simplicity and because we can handle all
* fully-reduced joins in a single pass over the parse tree.
+ *
+ * We allow stripping no-op PlaceHolderVars here, as this is a
+ * tree-simplification pass and we want to remove useless wrappers.
*/
if (!bms_is_empty(state2.inner_reduced))
{
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
state2.inner_reduced,
- NULL);
+ NULL, true);
/* There could be references in the append_rel_list, too */
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
state2.inner_reduced,
- NULL);
+ NULL, true);
}
/*
* Partially-reduced full joins have to be done one at a time, since
* they'll each need a different setting of except_relids.
+ *
+ * As above, we allow stripping no-op PlaceHolderVars.
*/
foreach(lc, state2.partial_reduced)
{
@@ -3222,11 +3242,13 @@ reduce_outer_joins(PlannerInfo *root)
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
full_join_relids,
- statep->unreduced_side);
+ statep->unreduced_side,
+ true);
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
full_join_relids,
- statep->unreduced_side);
+ statep->unreduced_side,
+ true);
}
}
@@ -3680,18 +3702,21 @@ remove_useless_result_rtes(PlannerInfo *root)
* know that such an outer join wouldn't really have nulled anything.) We
* don't do this during the main recursion, for simplicity and because we
* can handle all such joins in a single pass over the parse tree.
+ *
+ * We allow stripping no-op PlaceHolderVars here, as this is a
+ * tree-simplification pass and we want to remove useless wrappers.
*/
if (!bms_is_empty(dropped_outer_joins))
{
root->parse = (Query *)
remove_nulling_relids((Node *) root->parse,
dropped_outer_joins,
- NULL);
+ NULL, true);
/* There could be references in the append_rel_list, too */
root->append_rel_list = (List *)
remove_nulling_relids((Node *) root->append_rel_list,
dropped_outer_joins,
- NULL);
+ NULL, true);
}
/*
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index e1706363c88..4279fd85117 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -44,11 +44,11 @@ static bool contain_placeholder_references_walker(Node *node,
* phrels is the syntactic location (as a set of relids) to attribute
* to the expression.
*
- * The caller is responsible for adjusting phlevelsup and phnullingrels
- * as needed. Because we do not know here which query level the PHV
- * will be associated with, it's important that this function touches
- * only root->glob; messing with other parts of PlannerInfo would be
- * likely to do the wrong thing.
+ * The caller is responsible for adjusting phlevelsup, phnullingrels, and
+ * phpreserved as needed. Because we do not know here which query level
+ * the PHV will be associated with, it's important that this function
+ * touches only root->glob; messing with other parts of PlannerInfo
+ * would be likely to do the wrong thing.
*/
PlaceHolderVar *
make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
@@ -58,6 +58,7 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
phv->phexpr = expr;
phv->phrels = phrels;
phv->phnullingrels = NULL; /* caller may change this later */
+ phv->phpreserved = false; /* caller may change this later */
phv->phid = ++(root->glob->lastPHId);
phv->phlevelsup = 0; /* caller may change this later */
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index f57631e876f..2d4574ce916 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -2264,11 +2264,11 @@ have_partkey_equi_join(PlannerInfo *root, RelOptInfo *joinrel,
if (bms_overlap(rel1->relids, root->outer_join_rels))
expr1 = (Expr *) remove_nulling_relids((Node *) expr1,
root->outer_join_rels,
- NULL);
+ NULL, false);
if (bms_overlap(rel2->relids, root->outer_join_rels))
expr2 = (Expr *) remove_nulling_relids((Node *) expr2,
root->outer_join_rels,
- NULL);
+ NULL, false);
}
/*
diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 752ea9222f6..ca66b07009e 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -1161,7 +1161,7 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
sanitized_node =
remove_nulling_relids(node,
bms_make_singleton(context->root->group_rtindex),
- NULL);
+ NULL, true);
}
/*
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 6fa174412f2..e272531c522 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -53,6 +53,7 @@ typedef struct
const Bitmapset *removable_relids;
const Bitmapset *except_relids;
int sublevels_up;
+ bool remove_noop_phvs;
} remove_nulling_relids_context;
static bool contain_aggs_of_level_walker(Node *node,
@@ -68,6 +69,7 @@ static Node *add_nulling_relids_mutator(Node *node,
add_nulling_relids_context *context);
static Node *remove_nulling_relids_mutator(Node *node,
remove_nulling_relids_context *context);
+static bool is_safe_to_strip_payload(Node *node);
/*
@@ -1323,17 +1325,22 @@ add_nulling_relids_mutator(Node *node,
* in Var.varnullingrels and PlaceHolderVar.phnullingrels fields within
* the given expression, except in nodes belonging to rels listed in
* except_relids.
+ *
+ * If remove_noop_phvs is true, we also check if PlaceHolderVars become
+ * no-ops and strip them if so.
*/
Node *
remove_nulling_relids(Node *node,
const Bitmapset *removable_relids,
- const Bitmapset *except_relids)
+ const Bitmapset *except_relids,
+ bool remove_noop_phvs)
{
remove_nulling_relids_context context;
context.removable_relids = removable_relids;
context.except_relids = except_relids;
context.sublevels_up = 0;
+ context.remove_noop_phvs = remove_noop_phvs;
return query_or_expression_tree_mutator(node,
remove_nulling_relids_mutator,
&context,
@@ -1371,11 +1378,15 @@ remove_nulling_relids_mutator(Node *node,
!bms_overlap(phv->phrels, context->except_relids))
{
/*
- * Note: it might seem desirable to remove the PHV altogether if
- * phnullingrels goes to empty. Currently we dare not do that
- * because we use PHVs in some cases to enforce separate identity
- * of subexpressions; see wrap_option usages in prepjointree.c.
+ * If phnullingrels goes to empty, the PHV is no longer needed for
+ * outer-join nulling. We can remove it if we are allowed to
+ * remove no-op PHVs, provided that it is not marked as preserved
+ * (which indicates that it is needed to enforce separate identity
+ * of subexpressions) and that the contained expression is safe to
+ * pull up without breaking various invariants established by
+ * expression preprocessing.
*/
+
/* Copy the PlaceHolderVar and mutate what's below ... */
phv = (PlaceHolderVar *)
expression_tree_mutator(node,
@@ -1388,6 +1399,14 @@ remove_nulling_relids_mutator(Node *node,
phv->phrels = bms_difference(phv->phrels,
context->removable_relids);
Assert(!bms_is_empty(phv->phrels));
+
+ /* Strip the PHV if it's safe; see comment above */
+ if (context->remove_noop_phvs &&
+ !phv->phpreserved &&
+ bms_is_empty(phv->phnullingrels) &&
+ is_safe_to_strip_payload((Node *) phv->phexpr))
+ return (Node *) phv->phexpr;
+
return (Node *) phv;
}
/* Otherwise fall through to copy the PlaceHolderVar normally */
@@ -1408,6 +1427,44 @@ remove_nulling_relids_mutator(Node *node,
return expression_tree_mutator(node, remove_nulling_relids_mutator, context);
}
+/*
+ * is_safe_to_strip_payload
+ *
+ * Check whether the given node is safe to pull up into the surrounding
+ * expression structure.
+ *
+ * This is used to remove a PlaceHolderVar. Even if a PlaceHolderVar is no
+ * longer needed, we cannot simply remove it if doing so would expose the
+ * contained expression to a parent node in a way that breaks various
+ * invariants established by earlier expression preprocessing.
+ *
+ * Since we don't know what the parent node is, we need to be conservative.
+ */
+static bool
+is_safe_to_strip_payload(Node *node)
+{
+ if (node == NULL)
+ return false;
+
+ switch (nodeTag(node))
+ {
+ case T_Var:
+ case T_Const:
+ case T_PlaceHolderVar:
+ /* Atomic nodes should be safe to expose */
+ return true;
+
+ case T_OpExpr:
+ case T_CoalesceExpr:
+ return true;
+
+ default:
+ return false;
+ }
+
+ return false;
+}
+
/*
* replace_rte_variables() finds all Vars in an expression tree
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 29fec655593..fdf328c0fac 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -3655,7 +3655,7 @@ add_unique_group_var(PlannerInfo *root, List *varinfos,
* extended statistics (see estimate_multivariate_ndistinct). So strip
* them out first.
*/
- var = remove_nulling_relids(var, root->outer_join_rels, NULL);
+ var = remove_nulling_relids(var, root->outer_join_rels, NULL, false);
foreach(lc, varinfos)
{
@@ -4236,7 +4236,8 @@ estimate_multivariate_bucketsize(PlannerInfo *root, RelOptInfo *inner,
* Clear nullingrels to correctly match hash keys. See
* add_unique_group_var()'s comment for details.
*/
- expr = remove_nulling_relids(expr, root->outer_join_rels, NULL);
+ expr = remove_nulling_relids(expr, root->outer_join_rels,
+ NULL, false);
/*
* Detect and exclude exact duplicates from the list of hash
@@ -5762,7 +5763,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
* extended statistics. So strip them out first.
*/
if (bms_overlap(varnos, root->outer_join_rels))
- node = remove_nulling_relids(node, root->outer_join_rels, NULL);
+ node = remove_nulling_relids(node, root->outer_join_rels,
+ NULL, false);
foreach(ilist, onerel->indexlist)
{
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 449885b9319..c4671ef362a 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2985,6 +2985,11 @@ typedef struct MergeScanSelCache
* level of a PlaceHolderVar might be a join rather than a base relation.
* Likewise, phnullingrels corresponds to varnullingrels.
*
+ * phpreserved indicates whether the PlaceHolderVar needs to be preserved
+ * when its phnullingrels becomes empty. This is set true in cases where
+ * the PlaceHolderVar is used to enforce the separate identity of the
+ * contained expression.
+ *
* Although the planner treats this as an expression node type, it is not
* recognized by the parser or executor, so we declare it here rather than
* in primnodes.h.
@@ -3018,6 +3023,9 @@ typedef struct PlaceHolderVar
/* RT indexes of outer joins that can null PHV's value */
Relids phnullingrels;
+ /* true if PHV enforces separate identity */
+ bool phpreserved pg_node_attr(equal_ignore);
+
/* ID for PHV (unique within planner run) */
Index phid;
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index f8216c22fb7..05245faa49c 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -89,7 +89,8 @@ extern Node *add_nulling_relids(Node *node,
const Bitmapset *added_relids);
extern Node *remove_nulling_relids(Node *node,
const Bitmapset *removable_relids,
- const Bitmapset *except_relids);
+ const Bitmapset *except_relids,
+ bool remove_noop_phvs);
extern Node *replace_rte_variables(Node *node,
int target_varno, int sublevels_up,
--
2.39.5 (Apple Git-154)
[application/octet-stream] v3-0002-Fixing-test-cases.patch (50.4K, ../../CAMbWs4-qJGVHLonakB+5C_DvhA0i7Wua4StzG4n0F42i+dB+oA@mail.gmail.com/3-v3-0002-Fixing-test-cases.patch)
download | inline diff:
From 147b5cc81537fc646e6908268f0a5ba1b658fc7f Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Tue, 20 Jan 2026 19:32:08 +0900
Subject: [PATCH v3 2/2] Fixing test cases
This optimization improves plans for many queries in the regression
tests. However, it also reduces coverage in tests specifically
designed to verify the planner's handling of PlaceHolderVars. To
maintain this coverage, this patch modifies the affected queries to
force the preservation of PHVs by changing target expressions into
forms that the new logic considers unsafe to strip.
---
.../postgres_fdw/expected/postgres_fdw.out | 14 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 4 +-
src/test/regress/expected/groupingsets.out | 19 +-
src/test/regress/expected/join.out | 225 +++++++++---------
src/test/regress/expected/memoize.out | 16 +-
src/test/regress/expected/partition_join.out | 50 ++--
src/test/regress/sql/join.sql | 73 +++---
src/test/regress/sql/memoize.sql | 8 +-
src/test/regress/sql/partition_join.sql | 24 +-
9 files changed, 227 insertions(+), 206 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 6066510c7c0..017f20939f9 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3841,7 +3841,7 @@ FROM
"S 1"."T 1" AS ref_0,
LATERAL (
SELECT ref_0."C 1" c1, subq_0.*
- FROM (SELECT ref_0.c2, ref_1.c3
+ FROM (SELECT ref_0.c2::bigint, ref_1.c3
FROM ft1 AS ref_1) AS subq_0
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
) AS subq_1
@@ -3850,18 +3850,18 @@ ORDER BY ref_0."C 1";
QUERY PLAN
---------------------------------------------------------------------------------------------------------
Nested Loop
- Output: ref_0.c2, ref_0."C 1", (ref_0.c2), ref_1.c3, ref_0."C 1"
+ Output: ref_0.c2, ref_0."C 1", ((ref_0.c2)::bigint), ref_1.c3, ref_0."C 1"
-> Nested Loop
- Output: ref_0.c2, ref_0."C 1", ref_1.c3, (ref_0.c2)
+ Output: ref_0.c2, ref_0."C 1", ref_1.c3, ((ref_0.c2)::bigint)
-> Index Scan using t1_pkey on "S 1"."T 1" ref_0
Output: ref_0."C 1", ref_0.c2, ref_0.c3, ref_0.c4, ref_0.c5, ref_0.c6, ref_0.c7, ref_0.c8
Index Cond: (ref_0."C 1" < 10)
-> Memoize
- Output: ref_1.c3, (ref_0.c2)
- Cache Key: ref_0.c2
+ Output: ref_1.c3, ((ref_0.c2)::bigint)
+ Cache Key: (ref_0.c2)::bigint
Cache Mode: binary
-> Foreign Scan on public.ft1 ref_1
- Output: ref_1.c3, ref_0.c2
+ Output: ref_1.c3, (ref_0.c2)::bigint
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
-> Materialize
Output: ref_3.c3
@@ -3875,7 +3875,7 @@ FROM
"S 1"."T 1" AS ref_0,
LATERAL (
SELECT ref_0."C 1" c1, subq_0.*
- FROM (SELECT ref_0.c2, ref_1.c3
+ FROM (SELECT ref_0.c2::bigint, ref_1.c3
FROM ft1 AS ref_1) AS subq_0
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
) AS subq_1
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 4f7ab2ed0ac..929959e8b85 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1100,7 +1100,7 @@ FROM
"S 1"."T 1" AS ref_0,
LATERAL (
SELECT ref_0."C 1" c1, subq_0.*
- FROM (SELECT ref_0.c2, ref_1.c3
+ FROM (SELECT ref_0.c2::bigint, ref_1.c3
FROM ft1 AS ref_1) AS subq_0
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
) AS subq_1
@@ -1112,7 +1112,7 @@ FROM
"S 1"."T 1" AS ref_0,
LATERAL (
SELECT ref_0."C 1" c1, subq_0.*
- FROM (SELECT ref_0.c2, ref_1.c3
+ FROM (SELECT ref_0.c2::bigint, ref_1.c3
FROM ft1 AS ref_1) AS subq_0
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
) AS subq_1
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 921017489c0..68d44f12925 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -566,22 +566,19 @@ select * from (
group by grouping sets(1, 2)
) ss
where x = 1 and q1 = 123;
- QUERY PLAN
---------------------------------------------------
+ QUERY PLAN
+--------------------------------------------
Subquery Scan on ss
Output: ss.x, ss.q1, ss.sum
Filter: ((ss.x = 1) AND (ss.q1 = 123))
-> GroupAggregate
Output: (1), i1.q1, sum(i1.q2)
- Group Key: (1)
+ Group Key: 1
Sort Key: i1.q1
Group Key: i1.q1
- -> Sort
- Output: (1), i1.q1, i1.q2
- Sort Key: (1)
- -> Seq Scan on public.int8_tbl i1
- Output: 1, i1.q1, i1.q2
-(13 rows)
+ -> Seq Scan on public.int8_tbl i1
+ Output: 1, i1.q1, i1.q2
+(10 rows)
select * from (
select 1 as x, q1, sum(q2)
@@ -2617,8 +2614,8 @@ select 1 as one group by rollup(one) order by one nulls first;
-----------------------------
Sort
Sort Key: (1) NULLS FIRST
- -> MixedAggregate
- Hash Key: 1
+ -> GroupAggregate
+ Group Key: 1
Group Key: ()
-> Result
(6 rows)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index d05a0ca0373..22756ed0364 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -36,6 +36,16 @@ create temp table onerow();
insert into onerow default values;
analyze onerow;
--
+-- A STABLE, non-inlinable identity function used to
+-- prevent the planner from stripping no-op PHVs in some tests.
+--
+CREATE OR REPLACE FUNCTION phv(anyelement)
+RETURNS anyelement AS $$
+BEGIN
+ RETURN $1;
+END;
+$$ LANGUAGE plpgsql STABLE COST 0.00001;
+--
-- CORRELATION NAMES
-- Make sure that table/column aliases are supported
-- before diving into more complex join syntax.
@@ -3687,13 +3697,13 @@ from nt3 as nt3
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
from nt2 as nt2
left join
- (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
+ (select nt1.*, (nt1.id > 0 and nt1.id < 5) as a3 from nt1) as ss1
on ss1.id = nt2.nt1_id
) as ss2
on ss2.id = nt3.nt2_id
where nt3.id = 1 and ss2.b3;
- QUERY PLAN
-----------------------------------------------
+ QUERY PLAN
+------------------------------------------------------
Nested Loop
-> Nested Loop
-> Index Scan using nt3_pkey on nt3
@@ -3702,7 +3712,7 @@ where nt3.id = 1 and ss2.b3;
Index Cond: (id = nt3.nt2_id)
-> Index Only Scan using nt1_pkey on nt1
Index Cond: (id = nt2.nt1_id)
- Filter: (nt2.b1 AND true)
+ Filter: (nt2.b1 AND ((id > 0) AND (id < 5)))
(9 rows)
select nt3.id
@@ -3711,7 +3721,7 @@ from nt3 as nt3
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
from nt2 as nt2
left join
- (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
+ (select nt1.*, (nt1.id > 0 and nt1.id < 5) as a3 from nt1) as ss1
on ss1.id = nt2.nt1_id
) as ss2
on ss2.id = nt3.nt2_id
@@ -3776,20 +3786,20 @@ select * from
int4_tbl as i41,
lateral
(select 1 as x from
- (select i41.f1 as lat,
+ (select phv(i41.f1) as lat,
i42.f1 as loc from
int8_tbl as i81, int4_tbl as i42) as ss1
right join int4_tbl as i43 on (i43.f1 > 1)
where ss1.loc = ss1.lat) as ss2
where i41.f1 > 0;
- QUERY PLAN
---------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------
Nested Loop
-> Nested Loop
-> Seq Scan on int4_tbl i41
Filter: (f1 > 0)
-> Nested Loop
- Join Filter: (i42.f1 = i41.f1)
+ Join Filter: (i42.f1 = phv(i41.f1))
-> Seq Scan on int8_tbl i81
-> Materialize
-> Seq Scan on int4_tbl i42
@@ -3802,7 +3812,7 @@ select * from
int4_tbl as i41,
lateral
(select 1 as x from
- (select i41.f1 as lat,
+ (select phv(i41.f1) as lat,
i42.f1 as loc from
int8_tbl as i81, int4_tbl as i42) as ss1
right join int4_tbl as i43 on (i43.f1 > 1)
@@ -3955,28 +3965,28 @@ explain (costs off)
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (select 1,0 from onerow) v1(x1,x2)
- left join (select 3,1 from onerow) v2(y1,y2)
+ left join (select phv(3),1 from onerow) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
left join tenk1 t2
on (subq1.y1 = t2.unique1)
where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
- QUERY PLAN
------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------
Nested Loop
-> Nested Loop
Join Filter: (t1.stringu1 > t2.stringu2)
-> Nested Loop
-> Nested Loop
- -> Seq Scan on onerow
-> Seq Scan on onerow onerow_1
- -> Index Scan using tenk1_unique2 on tenk1 t1
- Index Cond: ((unique2 = (11)) AND (unique2 < 42))
- -> Index Scan using tenk1_unique1 on tenk1 t2
- Index Cond: (unique1 = (3))
+ -> Index Scan using tenk1_unique1 on tenk1 t2
+ Index Cond: (unique1 = (phv(3)))
+ -> Seq Scan on onerow
+ -> Index Scan using tenk1_unique2 on tenk1 t1
+ Index Cond: ((unique2 = (phv(11))) AND (unique2 < 42))
-> Seq Scan on int4_tbl i1
Filter: (f1 = 0)
(13 rows)
@@ -3984,9 +3994,9 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (select 1,0 from onerow) v1(x1,x2)
- left join (select 3,1 from onerow) v2(y1,y2)
+ left join (select phv(3),1 from onerow) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -4076,34 +4086,34 @@ explain (costs off)
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (values(1,0)) v1(x1,x2)
- left join (values(3,1)) v2(y1,y2)
+ left join (values(phv(3),1)) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
left join tenk1 t2
on (subq1.y1 = t2.unique1)
where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
- QUERY PLAN
------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------
Nested Loop
Join Filter: (t1.stringu1 > t2.stringu2)
-> Nested Loop
-> Seq Scan on int4_tbl i1
Filter: (f1 = 0)
-> Index Scan using tenk1_unique2 on tenk1 t1
- Index Cond: ((unique2 = (11)) AND (unique2 < 42))
+ Index Cond: ((unique2 = (phv(11))) AND (unique2 < 42))
-> Index Scan using tenk1_unique1 on tenk1 t2
- Index Cond: (unique1 = (3))
+ Index Cond: (unique1 = (phv(3)))
(9 rows)
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (values(1,0)) v1(x1,x2)
- left join (values(3,1)) v2(y1,y2)
+ left join (values(phv(3),1)) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -4119,22 +4129,22 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
-- or we end up with noplace to evaluate the lateral PHV
explain (verbose, costs off)
select * from
- (select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
+ (select 1 as x) ss1 left join (select phv(2) as y) ss2 on (true),
lateral (select ss2.y as z limit 1) ss3;
- QUERY PLAN
----------------------------
+ QUERY PLAN
+-----------------------------------
Nested Loop
- Output: 1, (2), ((2))
+ Output: 1, (phv(2)), ((phv(2)))
-> Result
- Output: 2
+ Output: phv(2)
-> Limit
- Output: ((2))
+ Output: ((phv(2)))
-> Result
- Output: (2)
+ Output: (phv(2))
(8 rows)
select * from
- (select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
+ (select 1 as x) ss1 left join (select phv(2) as y) ss2 on (true),
lateral (select ss2.y as z limit 1) ss3;
x | y | z
---+---+---
@@ -4213,25 +4223,25 @@ select * from t t1
explain (verbose, costs off)
select * from
(select k from
- (select i, coalesce(i, j) as k from
+ (select i, (i > 0 and j > 0) as k from
(select i from t union all select 0)
join (select 1 as j limit 1) on i = j)
right join (select 2 as x) on true
join (select 3 as y) on i is not null
),
lateral (select k as kl limit 1);
- QUERY PLAN
--------------------------------------------------------------------
+ QUERY PLAN
+--------------------------------------------------------------------
Nested Loop
- Output: COALESCE(t.i, (1)), ((COALESCE(t.i, (1))))
+ Output: ((t.i > 0) AND ((1) > 0)), ((((t.i > 0) AND ((1) > 0))))
-> Limit
Output: 1
-> Result
Output: 1
-> Nested Loop
- Output: t.i, ((COALESCE(t.i, (1))))
+ Output: t.i, ((((t.i > 0) AND ((1) > 0))))
-> Result
- Output: t.i, COALESCE(t.i, (1))
+ Output: t.i, ((t.i > 0) AND ((1) > 0))
-> Append
-> Index Only Scan using t_pkey on pg_temp.t
Output: t.i
@@ -4240,9 +4250,9 @@ select * from
Output: 0
One-Time Filter: ((1) = 0)
-> Limit
- Output: ((COALESCE(t.i, (1))))
+ Output: ((((t.i > 0) AND ((1) > 0))))
-> Result
- Output: (COALESCE(t.i, (1)))
+ Output: (((t.i > 0) AND ((1) > 0)))
(21 rows)
rollback;
@@ -4336,26 +4346,26 @@ explain (costs off)
select * from
(select 0 as z) as t1
left join
- (select true as a) as t2
+ (select phv(true) as a) as t2
on true,
lateral (select true as b
union all
select a as b) as t3
where b;
- QUERY PLAN
----------------------------------------
+ QUERY PLAN
+--------------------------------------------
Nested Loop
-> Result
-> Append
-> Result
-> Result
- One-Time Filter: (true)
+ One-Time Filter: (phv(true))
(6 rows)
select * from
(select 0 as z) as t1
left join
- (select true as a) as t2
+ (select phv(true) as a) as t2
on true,
lateral (select true as b
union all
@@ -5138,8 +5148,8 @@ select t1.* from
on (t1.f1 = b1.d1)
left join int4_tbl i4
on (i8.q2 = i4.f1);
- QUERY PLAN
-----------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------
Hash Left Join
Output: t1.f1
Hash Cond: (i8.q2 = i4.f1)
@@ -5150,22 +5160,22 @@ select t1.* from
Output: t1.f1
-> Materialize
Output: i8.q2
- -> Hash Right Join
+ -> Nested Loop Left Join
Output: i8.q2
- Hash Cond: ((NULL::integer) = i8b1.q2)
- -> Hash Join
- Output: i8.q2, (NULL::integer)
- Hash Cond: (i8.q1 = i8b2.q1)
- -> Seq Scan on public.int8_tbl i8
- Output: i8.q1, i8.q2
- -> Hash
- Output: i8b2.q1, (NULL::integer)
- -> Seq Scan on public.int8_tbl i8b2
- Output: i8b2.q1, NULL::integer
- -> Hash
- Output: i8b1.q2
- -> Seq Scan on public.int8_tbl i8b1
- Output: i8b1.q2
+ Join Filter: (NULL::integer = i8b1.q2)
+ -> Seq Scan on public.int8_tbl i8b1
+ Output: i8b1.q1, i8b1.q2
+ -> Materialize
+ Output: i8.q2
+ -> Hash Join
+ Output: i8.q2
+ Hash Cond: (i8.q1 = i8b2.q1)
+ -> Seq Scan on public.int8_tbl i8
+ Output: i8.q1, i8.q2
+ -> Hash
+ Output: i8b2.q1
+ -> Seq Scan on public.int8_tbl i8b2
+ Output: i8b2.q1
-> Hash
Output: i4.f1
-> Seq Scan on public.int4_tbl i4
@@ -5630,18 +5640,18 @@ explain (verbose, costs off)
select ss2.* from
int4_tbl i41
left join int8_tbl i8
- join (select i42.f1 as c1, i43.f1 as c2, 42 as c3
+ join (select i42.f1 as c1, i43.f1 as c2, phv(42) as c3
from int4_tbl i42, int4_tbl i43) ss1
on i8.q1 = ss1.c2
on i41.f1 = ss1.c1,
lateral (select i41.*, i8.*, ss1.* from text_tbl limit 1) ss2
where ss1.c2 = 0;
- QUERY PLAN
-------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Nested Loop
- Output: (i41.f1), (i8.q1), (i8.q2), (i42.f1), (i43.f1), ((42))
+ Output: (i41.f1), (i8.q1), (i8.q2), (i42.f1), (i43.f1), ((phv(42)))
-> Hash Join
- Output: i41.f1, i42.f1, i8.q1, i8.q2, i43.f1, 42
+ Output: i41.f1, i42.f1, i8.q1, i8.q2, i43.f1, phv(42)
Hash Cond: (i41.f1 = i42.f1)
-> Nested Loop
Output: i8.q1, i8.q2, i43.f1, i41.f1
@@ -5660,15 +5670,15 @@ where ss1.c2 = 0;
-> Seq Scan on public.int4_tbl i42
Output: i42.f1
-> Limit
- Output: (i41.f1), (i8.q1), (i8.q2), (i42.f1), (i43.f1), ((42))
+ Output: (i41.f1), (i8.q1), (i8.q2), (i42.f1), (i43.f1), ((phv(42)))
-> Seq Scan on public.text_tbl
- Output: i41.f1, i8.q1, i8.q2, i42.f1, i43.f1, (42)
+ Output: i41.f1, i8.q1, i8.q2, i42.f1, i43.f1, (phv(42))
(25 rows)
select ss2.* from
int4_tbl i41
left join int8_tbl i8
- join (select i42.f1 as c1, i43.f1 as c2, 42 as c3
+ join (select i42.f1 as c1, i43.f1 as c2, phv(42) as c3
from int4_tbl i42, int4_tbl i43) ss1
on i8.q1 = ss1.c2
on i41.f1 = ss1.c1,
@@ -6237,8 +6247,8 @@ select d.* from d left join (select 1 as x from b group by rollup(x)) s
Hash Cond: (d.a = (1))
-> Seq Scan on d
-> Hash
- -> MixedAggregate
- Hash Key: 1
+ -> GroupAggregate
+ Group Key: 1
Group Key: ()
-> Seq Scan on b
(8 rows)
@@ -6781,19 +6791,19 @@ insert into t values (1,1), (2,2);
explain (costs off)
select 1
from t t1
- left join (select t2.a, 1 as c
+ left join (select t2.a, phv(1) as c
from t t2 left join t t3 on t2.a = t3.a) s
on true
left join t t4 on true
where s.a < s.c;
- QUERY PLAN
--------------------------------------
+ QUERY PLAN
+------------------------------------------
Nested Loop Left Join
-> Nested Loop
-> Seq Scan on t t1
-> Materialize
-> Seq Scan on t t2
- Filter: (a < 1)
+ Filter: (a < phv(1))
-> Materialize
-> Seq Scan on t t4
(8 rows)
@@ -6806,13 +6816,13 @@ from t t1
on true
left join t t4 on true
where s.a < s.c;
- QUERY PLAN
------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------
Nested Loop Left Join
-> Nested Loop
-> Seq Scan on t t1
- -> Seq Scan on t t2
- Filter: (a < COALESCE(t1.a, 1))
+ -> Index Only Scan using t_a_key on t t2
+ Index Cond: (a < COALESCE(t1.a, 1))
-> Materialize
-> Seq Scan on t t4
(7 rows)
@@ -6836,15 +6846,15 @@ explain (verbose, costs off)
select i8.*, ss.v, t.unique2
from int8_tbl i8
left join int4_tbl i4 on i4.f1 = 1
- left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join lateral (select phv(i4.f1 + 1) as v) as ss on true
left join tenk1 t on t.unique2 = ss.v
where q2 = 456;
QUERY PLAN
-------------------------------------------------------------
Nested Loop Left Join
- Output: i8.q1, i8.q2, ((i4.f1 + 1)), t.unique2
+ Output: i8.q1, i8.q2, (phv((i4.f1 + 1))), t.unique2
-> Nested Loop Left Join
- Output: i8.q1, i8.q2, (i4.f1 + 1)
+ Output: i8.q1, i8.q2, phv((i4.f1 + 1))
-> Seq Scan on public.int8_tbl i8
Output: i8.q1, i8.q2
Filter: (i8.q2 = 456)
@@ -6853,13 +6863,13 @@ where q2 = 456;
Filter: (i4.f1 = 1)
-> Index Only Scan using tenk1_unique2 on public.tenk1 t
Output: t.unique2
- Index Cond: (t.unique2 = ((i4.f1 + 1)))
+ Index Cond: (t.unique2 = (phv((i4.f1 + 1))))
(13 rows)
select i8.*, ss.v, t.unique2
from int8_tbl i8
left join int4_tbl i4 on i4.f1 = 1
- left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join lateral (select phv(i4.f1 + 1) as v) as ss on true
left join tenk1 t on t.unique2 = ss.v
where q2 = 456;
q1 | q2 | v | unique2
@@ -6874,20 +6884,20 @@ create temp table parttbl1 partition of parttbl for values from (1) to (100);
insert into parttbl values (11), (12);
explain (costs off)
select * from
- (select *, 12 as phv from parttbl) as ss
+ (select *, phv(12) as phv from parttbl) as ss
right join int4_tbl on true
where ss.a = ss.phv and f1 = 0;
QUERY PLAN
------------------------------------
Nested Loop
-> Seq Scan on parttbl1 parttbl
- Filter: (a = 12)
+ Filter: (a = phv(12))
-> Seq Scan on int4_tbl
Filter: (f1 = 0)
(5 rows)
select * from
- (select *, 12 as phv from parttbl) as ss
+ (select *, phv(12) as phv from parttbl) as ss
right join int4_tbl on true
where ss.a = ss.phv and f1 = 0;
a | phv | f1
@@ -7670,7 +7680,7 @@ on true;
-- not implemented yet.
explain (verbose, costs off)
select 1 from emp1 t1 left join
- ((select 1 as x, * from emp1 t2) s1 inner join
+ ((select phv(1) as x, * from emp1 t2) s1 inner join
(select * from emp1 t3) s2 on s1.id = s2.id)
on true
where s1.x = 1;
@@ -7684,7 +7694,7 @@ where s1.x = 1;
Output: t3.id
-> Seq Scan on public.emp1 t3
Output: t3.id
- Filter: (1 = 1)
+ Filter: (phv(1) = 1)
(9 rows)
-- Check that PHVs do not impose any constraints on removing self joins
@@ -7731,7 +7741,7 @@ INSERT INTO tbl_phv (x, y)
VACUUM ANALYZE tbl_phv;
EXPLAIN (COSTS OFF, VERBOSE)
SELECT 1 FROM tbl_phv t1 LEFT JOIN
- (SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
+ (SELECT phv(1) extra, x, y FROM tbl_phv tl) t3 JOIN
(SELECT y FROM tbl_phv tr) t4
ON t4.y = t3.y
ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
@@ -7744,7 +7754,7 @@ ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
-> Index Scan using tbl_phv_idx on public.tbl_phv tr
Output: tr.x, tr.y
Index Cond: (tr.x = (t1.x % 2))
- Filter: (1 IS NOT NULL)
+ Filter: (phv(1) IS NOT NULL)
(8 rows)
DROP TABLE IF EXISTS tbl_phv;
@@ -8881,23 +8891,23 @@ select * from
explain (verbose, costs off)
select * from
(select 0 as val0) as ss0
- left join (select 1 as val) as ss1 on true
+ left join (select phv(1) as val) as ss1 on true
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
- QUERY PLAN
---------------------------------
+ QUERY PLAN
+-----------------------------------
Nested Loop Left Join
- Output: 0, (1), ((1))
+ Output: 0, (phv(1)), ((phv(1)))
Join Filter: false
-> Result
- Output: 1
+ Output: phv(1)
-> Result
- Output: (1)
+ Output: (phv(1))
One-Time Filter: false
(8 rows)
select * from
(select 0 as val0) as ss0
- left join (select 1 as val) as ss1 on true
+ left join (select phv(1) as val) as ss1 on true
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
val0 | val | val_filtered
------+-----+--------------
@@ -9885,20 +9895,20 @@ DROP TABLE group_tbl;
-- Test that we ignore PlaceHolderVars when looking up statistics
EXPLAIN (COSTS OFF)
SELECT t1.unique1 FROM tenk1 t1 LEFT JOIN
- (SELECT *, 42 AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
+ (SELECT *, phv(42) AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
WHERE ss.unique1 = ss.phv AND t1.unique1 < 100;
QUERY PLAN
--------------------------------------------------
Nested Loop
-> Seq Scan on tenk1 t2
- Filter: (unique1 = 42)
+ Filter: (unique1 = phv(42))
-> Index Scan using tenk1_unique2 on tenk1 t1
Index Cond: (unique2 = t2.unique2)
Filter: (unique1 < 100)
(6 rows)
SELECT t1.unique1 FROM tenk1 t1 LEFT JOIN
- (SELECT *, 42 AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
+ (SELECT *, phv(42) AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
WHERE ss.unique1 = ss.phv AND t1.unique1 < 100;
unique1
---------
@@ -9947,3 +9957,4 @@ SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
19000
(1 row)
+DROP FUNCTION phv(anyelement);
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out
index 00c30b91459..708cea644d9 100644
--- a/src/test/regress/expected/memoize.out
+++ b/src/test/regress/expected/memoize.out
@@ -138,7 +138,7 @@ WHERE t1.unique1 < 10;
-- Try with LATERAL references within PlaceHolderVars
SELECT explain_memoize('
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.two+1 AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
+LATERAL (SELECT (t1.two+1)::bigint AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
explain_memoize
----------------------------------------------------------------------------------------------
@@ -148,11 +148,11 @@ WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
Filter: (unique1 < 1000)
Rows Removed by Filter: 9000
-> Memoize (actual rows=1.00 loops=N)
- Cache Key: (t1.two + 1)
+ Cache Key: ((t1.two + 1))::bigint
Cache Mode: binary
Hits: 998 Misses: 2 Evictions: Zero Overflows: 0 Memory Usage: NkB
-> Index Only Scan using tenk1_unique1 on tenk1 t2 (actual rows=1.00 loops=N)
- Filter: ((t1.two + 1) = unique1)
+ Filter: (((t1.two + 1))::bigint = unique1)
Rows Removed by Filter: 9999
Heap Fetches: N
Index Searches: N
@@ -160,7 +160,7 @@ WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
-- And check we get the expected results.
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.two+1 AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
+LATERAL (SELECT (t1.two+1)::bigint AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;
count | avg
-------+--------------------
@@ -170,7 +170,7 @@ WHERE s.c1 = s.c2 AND t1.unique1 < 1000;
-- Ensure we do not omit the cache keys from PlaceHolderVars
SELECT explain_memoize('
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.twenty AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
+LATERAL (SELECT t1.twenty::bigint AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
ON t1.two = s.two
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
explain_memoize
@@ -181,17 +181,17 @@ WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
Filter: (unique1 < 1000)
Rows Removed by Filter: 9000
-> Memoize (actual rows=1.00 loops=N)
- Cache Key: t1.two, t1.twenty
+ Cache Key: t1.two, (t1.twenty)::bigint
Cache Mode: binary
Hits: 980 Misses: 20 Evictions: Zero Overflows: 0 Memory Usage: NkB
-> Seq Scan on tenk1 t2 (actual rows=1.00 loops=N)
- Filter: ((t1.twenty = unique1) AND (t1.two = two))
+ Filter: (((t1.twenty)::bigint = unique1) AND (t1.two = two))
Rows Removed by Filter: 9999
(12 rows)
-- And check we get the expected results.
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.twenty AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
+LATERAL (SELECT t1.twenty::bigint AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
ON t1.two = s.two
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;
count | avg
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index f6d3ade368a..e9f9515ea3d 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -576,8 +576,8 @@ SELECT * FROM prt1 t1 JOIN LATERAL
-- lateral reference in scan's restriction clauses
EXPLAIN (COSTS OFF)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.a;
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
QUERY PLAN
---------------------------------------------------------------
Aggregate
@@ -586,22 +586,22 @@ SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
-> Seq Scan on prt1_p1 t1_1
-> Index Scan using iprt2_p1_b on prt2_p1 t2_1
Index Cond: (b = t1_1.a)
- Filter: (t1_1.b = a)
+ Filter: ((t1_1.b >= 0) AND (t1_1.b = a))
-> Nested Loop
-> Seq Scan on prt1_p2 t1_2
-> Index Scan using iprt2_p2_b on prt2_p2 t2_2
Index Cond: (b = t1_2.a)
- Filter: (t1_2.b = a)
+ Filter: ((t1_2.b >= 0) AND (t1_2.b = a))
-> Nested Loop
-> Seq Scan on prt1_p3 t1_3
-> Index Scan using iprt2_p3_b on prt2_p3 t2_3
Index Cond: (b = t1_3.a)
- Filter: (t1_3.b = a)
+ Filter: ((t1_3.b >= 0) AND (t1_3.b = a))
(17 rows)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.a;
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
count
-------
100
@@ -609,8 +609,8 @@ SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
EXPLAIN (COSTS OFF)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.b;
+ (SELECT (t1.b >= 0 AND t1.b = b) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
QUERY PLAN
--------------------------------------------------------------------
Aggregate
@@ -619,22 +619,22 @@ SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
-> Seq Scan on prt1_p1 t1_1
-> Index Only Scan using iprt2_p1_b on prt2_p1 t2_1
Index Cond: (b = t1_1.a)
- Filter: (b = t1_1.b)
+ Filter: ((t1_1.b >= 0) AND (t1_1.b = b))
-> Nested Loop
-> Seq Scan on prt1_p2 t1_2
-> Index Only Scan using iprt2_p2_b on prt2_p2 t2_2
Index Cond: (b = t1_2.a)
- Filter: (b = t1_2.b)
+ Filter: ((t1_2.b >= 0) AND (t1_2.b = b))
-> Nested Loop
-> Seq Scan on prt1_p3 t1_3
-> Index Only Scan using iprt2_p3_b on prt2_p3 t2_3
Index Cond: (b = t1_3.a)
- Filter: (b = t1_3.b)
+ Filter: ((t1_3.b >= 0) AND (t1_3.b = b))
(17 rows)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.b;
+ (SELECT (t1.b >= 0 AND t1.b = b) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
count
-------
5
@@ -2200,39 +2200,39 @@ SELECT * FROM prt1_l t1 JOIN LATERAL
-- partitionwise join with lateral reference in scan's restriction clauses
EXPLAIN (COSTS OFF)
SELECT COUNT(*) FROM prt1_l t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2_l t2) s
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2_l t2) s
ON t1.a = s.b AND t1.b = s.a AND t1.c = s.c
- WHERE s.t1b = s.a;
- QUERY PLAN
----------------------------------------------------------------------------------------------------------------
+ WHERE s.t1b;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------
Aggregate
-> Append
-> Nested Loop
-> Seq Scan on prt1_l_p1 t1_1
-> Seq Scan on prt2_l_p1 t2_1
- Filter: ((a = t1_1.b) AND (t1_1.a = b) AND (t1_1.b = a) AND ((t1_1.c)::text = (c)::text))
+ Filter: (((t1_1.b >= 0) AND (t1_1.b = a)) AND (t1_1.a = b) AND (t1_1.b = a) AND ((t1_1.c)::text = (c)::text))
-> Nested Loop
-> Seq Scan on prt1_l_p2_p1 t1_2
-> Seq Scan on prt2_l_p2_p1 t2_2
- Filter: ((a = t1_2.b) AND (t1_2.a = b) AND (t1_2.b = a) AND ((t1_2.c)::text = (c)::text))
+ Filter: (((t1_2.b >= 0) AND (t1_2.b = a)) AND (t1_2.a = b) AND (t1_2.b = a) AND ((t1_2.c)::text = (c)::text))
-> Nested Loop
-> Seq Scan on prt1_l_p2_p2 t1_3
-> Seq Scan on prt2_l_p2_p2 t2_3
- Filter: ((a = t1_3.b) AND (t1_3.a = b) AND (t1_3.b = a) AND ((t1_3.c)::text = (c)::text))
+ Filter: (((t1_3.b >= 0) AND (t1_3.b = a)) AND (t1_3.a = b) AND (t1_3.b = a) AND ((t1_3.c)::text = (c)::text))
-> Nested Loop
-> Seq Scan on prt1_l_p3_p1 t1_4
-> Seq Scan on prt2_l_p3_p1 t2_4
- Filter: ((a = t1_4.b) AND (t1_4.a = b) AND (t1_4.b = a) AND ((t1_4.c)::text = (c)::text))
+ Filter: (((t1_4.b >= 0) AND (t1_4.b = a)) AND (t1_4.a = b) AND (t1_4.b = a) AND ((t1_4.c)::text = (c)::text))
-> Nested Loop
-> Seq Scan on prt1_l_p3_p2 t1_5
-> Seq Scan on prt2_l_p3_p2 t2_5
- Filter: ((a = t1_5.b) AND (t1_5.a = b) AND (t1_5.b = a) AND ((t1_5.c)::text = (c)::text))
+ Filter: (((t1_5.b >= 0) AND (t1_5.b = a)) AND (t1_5.a = b) AND (t1_5.b = a) AND ((t1_5.c)::text = (c)::text))
(22 rows)
SELECT COUNT(*) FROM prt1_l t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2_l t2) s
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2_l t2) s
ON t1.a = s.b AND t1.b = s.a AND t1.c = s.c
- WHERE s.t1b = s.a;
+ WHERE s.t1b;
count
-------
100
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index b91fb7574df..8adaedb63ad 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -42,6 +42,17 @@ create temp table onerow();
insert into onerow default values;
analyze onerow;
+--
+-- A STABLE, non-inlinable identity function used to
+-- prevent the planner from stripping no-op PHVs in some tests.
+--
+CREATE OR REPLACE FUNCTION phv(anyelement)
+RETURNS anyelement AS $$
+BEGIN
+ RETURN $1;
+END;
+$$ LANGUAGE plpgsql STABLE COST 0.00001;
+
--
-- CORRELATION NAMES
@@ -1169,7 +1180,7 @@ from nt3 as nt3
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
from nt2 as nt2
left join
- (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
+ (select nt1.*, (nt1.id > 0 and nt1.id < 5) as a3 from nt1) as ss1
on ss1.id = nt2.nt1_id
) as ss2
on ss2.id = nt3.nt2_id
@@ -1181,7 +1192,7 @@ from nt3 as nt3
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
from nt2 as nt2
left join
- (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
+ (select nt1.*, (nt1.id > 0 and nt1.id < 5) as a3 from nt1) as ss1
on ss1.id = nt2.nt1_id
) as ss2
on ss2.id = nt3.nt2_id
@@ -1217,7 +1228,7 @@ select * from
int4_tbl as i41,
lateral
(select 1 as x from
- (select i41.f1 as lat,
+ (select phv(i41.f1) as lat,
i42.f1 as loc from
int8_tbl as i81, int4_tbl as i42) as ss1
right join int4_tbl as i43 on (i43.f1 > 1)
@@ -1228,7 +1239,7 @@ select * from
int4_tbl as i41,
lateral
(select 1 as x from
- (select i41.f1 as lat,
+ (select phv(i41.f1) as lat,
i42.f1 as loc from
int8_tbl as i81, int4_tbl as i42) as ss1
right join int4_tbl as i43 on (i43.f1 > 1)
@@ -1279,9 +1290,9 @@ explain (costs off)
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (select 1,0 from onerow) v1(x1,x2)
- left join (select 3,1 from onerow) v2(y1,y2)
+ left join (select phv(3),1 from onerow) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -1292,9 +1303,9 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (select 1,0 from onerow) v1(x1,x2)
- left join (select 3,1 from onerow) v2(y1,y2)
+ left join (select phv(3),1 from onerow) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -1343,9 +1354,9 @@ explain (costs off)
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (values(1,0)) v1(x1,x2)
- left join (values(3,1)) v2(y1,y2)
+ left join (values(phv(3),1)) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -1356,9 +1367,9 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
tenk1 t1
inner join int4_tbl i1
- left join (select v1.x2, v2.y1, 11 AS d1
+ left join (select v1.x2, v2.y1, phv(11) AS d1
from (values(1,0)) v1(x1,x2)
- left join (values(3,1)) v2(y1,y2)
+ left join (values(phv(3),1)) v2(y1,y2)
on v1.x1 = v2.y2) subq1
on (i1.f1 = subq1.x2)
on (t1.unique2 = subq1.d1)
@@ -1370,10 +1381,10 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
-- or we end up with noplace to evaluate the lateral PHV
explain (verbose, costs off)
select * from
- (select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
+ (select 1 as x) ss1 left join (select phv(2) as y) ss2 on (true),
lateral (select ss2.y as z limit 1) ss3;
select * from
- (select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
+ (select 1 as x) ss1 left join (select phv(2) as y) ss2 on (true),
lateral (select ss2.y as z limit 1) ss3;
-- This example demonstrates the folly of our old "have_dangerous_phv" logic
@@ -1401,7 +1412,7 @@ select * from t t1
explain (verbose, costs off)
select * from
(select k from
- (select i, coalesce(i, j) as k from
+ (select i, (i > 0 and j > 0) as k from
(select i from t union all select 0)
join (select 1 as j limit 1) on i = j)
right join (select 2 as x) on true
@@ -1439,7 +1450,7 @@ explain (costs off)
select * from
(select 0 as z) as t1
left join
- (select true as a) as t2
+ (select phv(true) as a) as t2
on true,
lateral (select true as b
union all
@@ -1449,7 +1460,7 @@ where b;
select * from
(select 0 as z) as t1
left join
- (select true as a) as t2
+ (select phv(true) as a) as t2
on true,
lateral (select true as b
union all
@@ -1978,7 +1989,7 @@ explain (verbose, costs off)
select ss2.* from
int4_tbl i41
left join int8_tbl i8
- join (select i42.f1 as c1, i43.f1 as c2, 42 as c3
+ join (select i42.f1 as c1, i43.f1 as c2, phv(42) as c3
from int4_tbl i42, int4_tbl i43) ss1
on i8.q1 = ss1.c2
on i41.f1 = ss1.c1,
@@ -1988,7 +1999,7 @@ where ss1.c2 = 0;
select ss2.* from
int4_tbl i41
left join int8_tbl i8
- join (select i42.f1 as c1, i43.f1 as c2, 42 as c3
+ join (select i42.f1 as c1, i43.f1 as c2, phv(42) as c3
from int4_tbl i42, int4_tbl i43) ss1
on i8.q1 = ss1.c2
on i41.f1 = ss1.c1,
@@ -2551,7 +2562,7 @@ insert into t values (1,1), (2,2);
explain (costs off)
select 1
from t t1
- left join (select t2.a, 1 as c
+ left join (select t2.a, phv(1) as c
from t t2 left join t t3 on t2.a = t3.a) s
on true
left join t t4 on true
@@ -2581,14 +2592,14 @@ explain (verbose, costs off)
select i8.*, ss.v, t.unique2
from int8_tbl i8
left join int4_tbl i4 on i4.f1 = 1
- left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join lateral (select phv(i4.f1 + 1) as v) as ss on true
left join tenk1 t on t.unique2 = ss.v
where q2 = 456;
select i8.*, ss.v, t.unique2
from int8_tbl i8
left join int4_tbl i4 on i4.f1 = 1
- left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join lateral (select phv(i4.f1 + 1) as v) as ss on true
left join tenk1 t on t.unique2 = ss.v
where q2 = 456;
@@ -2599,12 +2610,12 @@ create temp table parttbl1 partition of parttbl for values from (1) to (100);
insert into parttbl values (11), (12);
explain (costs off)
select * from
- (select *, 12 as phv from parttbl) as ss
+ (select *, phv(12) as phv from parttbl) as ss
right join int4_tbl on true
where ss.a = ss.phv and f1 = 0;
select * from
- (select *, 12 as phv from parttbl) as ss
+ (select *, phv(12) as phv from parttbl) as ss
right join int4_tbl on true
where ss.a = ss.phv and f1 = 0;
@@ -2957,7 +2968,7 @@ on true;
-- not implemented yet.
explain (verbose, costs off)
select 1 from emp1 t1 left join
- ((select 1 as x, * from emp1 t2) s1 inner join
+ ((select phv(1) as x, * from emp1 t2) s1 inner join
(select * from emp1 t3) s2 on s1.id = s2.id)
on true
where s1.x = 1;
@@ -2986,7 +2997,7 @@ INSERT INTO tbl_phv (x, y)
VACUUM ANALYZE tbl_phv;
EXPLAIN (COSTS OFF, VERBOSE)
SELECT 1 FROM tbl_phv t1 LEFT JOIN
- (SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
+ (SELECT phv(1) extra, x, y FROM tbl_phv tl) t3 JOIN
(SELECT y FROM tbl_phv tr) t4
ON t4.y = t3.y
ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
@@ -3332,12 +3343,12 @@ select * from
explain (verbose, costs off)
select * from
(select 0 as val0) as ss0
- left join (select 1 as val) as ss1 on true
+ left join (select phv(1) as val) as ss1 on true
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
select * from
(select 0 as val0) as ss0
- left join (select 1 as val) as ss1 on true
+ left join (select phv(1) as val) as ss1 on true
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
-- case that breaks the old ph_may_need optimization
@@ -3767,11 +3778,11 @@ DROP TABLE group_tbl;
-- Test that we ignore PlaceHolderVars when looking up statistics
EXPLAIN (COSTS OFF)
SELECT t1.unique1 FROM tenk1 t1 LEFT JOIN
- (SELECT *, 42 AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
+ (SELECT *, phv(42) AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
WHERE ss.unique1 = ss.phv AND t1.unique1 < 100;
SELECT t1.unique1 FROM tenk1 t1 LEFT JOIN
- (SELECT *, 42 AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
+ (SELECT *, phv(42) AS phv FROM tenk1 t2) ss ON t1.unique2 = ss.unique2
WHERE ss.unique1 = ss.phv AND t1.unique1 < 100;
--
@@ -3790,3 +3801,5 @@ SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+
+DROP FUNCTION phv(anyelement);
diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql
index 8d1cdd6990c..014ce8274c1 100644
--- a/src/test/regress/sql/memoize.sql
+++ b/src/test/regress/sql/memoize.sql
@@ -79,24 +79,24 @@ WHERE t1.unique1 < 10;
-- Try with LATERAL references within PlaceHolderVars
SELECT explain_memoize('
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.two+1 AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
+LATERAL (SELECT (t1.two+1)::bigint AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
-- And check we get the expected results.
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.two+1 AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
+LATERAL (SELECT (t1.two+1)::bigint AS c1, t2.unique1 AS c2 FROM tenk1 t2) s ON TRUE
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;
-- Ensure we do not omit the cache keys from PlaceHolderVars
SELECT explain_memoize('
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.twenty AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
+LATERAL (SELECT t1.twenty::bigint AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
ON t1.two = s.two
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;', false);
-- And check we get the expected results.
SELECT COUNT(*), AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
-LATERAL (SELECT t1.twenty AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
+LATERAL (SELECT t1.twenty::bigint AS c1, t2.unique1 AS c2, t2.two FROM tenk1 t2) s
ON t1.two = s.two
WHERE s.c1 = s.c2 AND t1.unique1 < 1000;
diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql
index c4549fc1ad8..ad1735e5c5e 100644
--- a/src/test/regress/sql/partition_join.sql
+++ b/src/test/regress/sql/partition_join.sql
@@ -119,19 +119,19 @@ SELECT * FROM prt1 t1 JOIN LATERAL
-- lateral reference in scan's restriction clauses
EXPLAIN (COSTS OFF)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.a;
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.a;
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
EXPLAIN (COSTS OFF)
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.b;
+ (SELECT (t1.b >= 0 AND t1.b = b) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
SELECT count(*) FROM prt1 t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2 t2) s
- ON t1.a = s.b WHERE s.t1b = s.b;
+ (SELECT (t1.b >= 0 AND t1.b = b) AS t1b, t2.* FROM prt2 t2) s
+ ON t1.a = s.b WHERE s.t1b;
-- bug with inadequate sort key representation
SET enable_partitionwise_aggregate TO true;
@@ -439,13 +439,13 @@ SELECT * FROM prt1_l t1 JOIN LATERAL
-- partitionwise join with lateral reference in scan's restriction clauses
EXPLAIN (COSTS OFF)
SELECT COUNT(*) FROM prt1_l t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2_l t2) s
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2_l t2) s
ON t1.a = s.b AND t1.b = s.a AND t1.c = s.c
- WHERE s.t1b = s.a;
+ WHERE s.t1b;
SELECT COUNT(*) FROM prt1_l t1 LEFT JOIN LATERAL
- (SELECT t1.b AS t1b, t2.* FROM prt2_l t2) s
+ (SELECT (t1.b >= 0 AND t1.b = a) AS t1b, t2.* FROM prt2_l t2) s
ON t1.a = s.b AND t1.b = s.a AND t1.c = s.c
- WHERE s.t1b = s.a;
+ WHERE s.t1b;
-- join with one side empty
EXPLAIN (COSTS OFF)
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-01-21 08:18 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 03:11 [PATCH v8 4/8] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]>
2024-09-03 02:53 Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2024-09-03 03:31 ` Re: Remove no-op PlaceHolderVars Tom Lane <[email protected]>
2024-09-03 08:50 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2026-01-16 02:49 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
2026-01-21 08:18 ` Re: Remove no-op PlaceHolderVars Richard Guo <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox