Revert inappropriate weakening of an Assert in plpgsql.
authorTom Lane <[email protected]>
Fri, 21 Mar 2025 19:55:06 +0000 (15:55 -0400)
committerTom Lane <[email protected]>
Fri, 21 Mar 2025 19:55:06 +0000 (15:55 -0400)
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

index e82052750362846224d8f135eebd1a0503773e0e..9c41ca082532d578f68c7e3aec04a5e114bbac0c 100644 (file)
@@ -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;
        }