From 16a3ae504e2030583851c8f45cbed14b0c546d6e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 21 Mar 2025 15:55:06 -0400 Subject: [PATCH] Revert inappropriate weakening of an Assert in plpgsql. Commit 682ce911f modified exec_save_simple_expr to accept a Param in the tlist of a Gather node, rather than the normal case of a Var referencing the Gather's input. It turns out that this was a kluge to work around the bug later fixed in 0f7ec8d9c, namely that setrefs.c was failing to replace Params in upper plan nodes with Var references to the same Params appearing in the child tlists. With that fixed, there seems no reason to continue to allow a Param here. (Moreover, even if we did expect a Param here, the semantically correct thing to do would be to take the Param as the expression being sought. Whatever it may represent, it is *not* a reference to the child.) Hence, revert that part of 682ce911f. That all happened a long time ago. However, since the net effect here is just to tighten an Assert condition, I'm content to change it only in master. Discussion: https://postgr.es/m/1565347.1742572349@sss.pgh.pa.us --- src/pl/plpgsql/src/pl_exec.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index e8205275036..9c41ca08253 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -8306,8 +8306,7 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan) * might've stuck a Material node atop it. The simplest way to deal with * this is to look through the Gather and/or Material nodes. The upper * node's tlist would normally contain a Var referencing the child node's - * output, but it could also be a Param, or it could be a Const that - * setrefs.c copied as-is. + * output ... but setrefs.c might also have copied a Const as-is. */ plan = stmt->planTree; for (;;) @@ -8334,9 +8333,9 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan) /* If setrefs.c copied up a Const, no need to look further */ if (IsA(tle_expr, Const)) break; - /* Otherwise, it had better be a Param or an outer Var */ - Assert(IsA(tle_expr, Param) || (IsA(tle_expr, Var) && - ((Var *) tle_expr)->varno == OUTER_VAR)); + /* Otherwise, it better be an outer Var */ + Assert(IsA(tle_expr, Var)); + Assert(((Var *) tle_expr)->varno == OUTER_VAR); /* Descend to the child node */ plan = plan->lefttree; }