IMO, the routine eval_const_expressions_mutator contains some stale code:
case T_SubPlan:
case T_AlternativeSubPlan:
/*
* Return a SubPlan unchanged --- too late to do anything with it.
*
* XXX should we ereport() here instead? Probably this routine
* should never be invoked after SubPlan creation.
*/
return node;
At least, this code could be achieved with estimate_expression_value().
So, we should fix the comment. But maybe we need to do a bit more.
According to the mutator idea, only the Query node is returned
unchanged. If the Subplan node is on top of the expression, the call
above returns the same node, which may be unconventional.
I'm not totally sure about the impossibility of constantifying SubPlan:
we already have InitPlans for uncorrelated subplans. Maybe something
about parameters that can be estimated as constants at this level and,
as a result, allow to return a Const instead of SubPlan?
But at least we can return a flat copy of the SubPplan node just for the
convention — the same thing for the AlternativeSubPlan. See the patch in
the attachment.
--
regards,
Andrei Lepikhov
Postgres Professional
From a227e7e72d917726085001027c350d2fda2ad3c4 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepik...@postgrespro.ru>
Date: Tue, 27 Feb 2024 11:00:23 +0700
Subject: [PATCH] Force eval_const_expressions_mutator to return a copy of the
node.
In some situations, when SubPlan or AlternativeSubPlan nodes are on the top of
the expression tree, this routine returns the same node, which could baffle
users who, remembering the mutator convention, wanted to free the evaluation
result immediately.
Author: a.rybakina.
---
src/backend/optimizer/util/clauses.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/backend/optimizer/util/clauses.c
b/src/backend/optimizer/util/clauses.c
index edc25d712e..f09c6c6420 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2907,15 +2907,28 @@ eval_const_expressions_mutator(Node *node,
}
case T_SubPlan:
+ {
+ SubPlan *subplan = makeNode(SubPlan);
+
+ memcpy(subplan, node, sizeof(SubPlan));
+
+ /*
+ * Return a SubPlan unchanged. If the subplan had been
uncorrelated
+ * it already have been converted to an InitPlan.
+ */
+ return (Node *) subplan;
+ }
case T_AlternativeSubPlan:
+ {
+ AlternativeSubPlan *subplan =
makeNode(AlternativeSubPlan);
+
+ memcpy(subplan, node, sizeof(AlternativeSubPlan));
/*
* Return a SubPlan unchanged --- too late to do
anything with it.
- *
- * XXX should we ereport() here instead? Probably this
routine
- * should never be invoked after SubPlan creation.
*/
- return node;
+ return (Node *) subplan;
+ }
case T_RelabelType:
{
RelabelType *relabel = (RelabelType *) node;
--
2.43.2
