]> granicus.if.org Git - postgresql/commitdiff
Fix plpgsql to reinitialize record variables at block re-entry.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 9 Dec 2017 17:03:00 +0000 (12:03 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 9 Dec 2017 17:03:04 +0000 (12:03 -0500)
If one exits and re-enters a DECLARE ... BEGIN ... END block within a
single execution of a plpgsql function, perhaps due to a surrounding loop,
the declared variables are supposed to get re-initialized to null (or
whatever their initializer is).  But this failed to happen for variables
of type "record", because while exec_stmt_block() expected such variables
to be included in the block's initvarnos list, plpgsql_add_initdatums()
only adds DTYPE_VAR variables to that list.  This bug appears to have
been there since the aboriginal addition of plpgsql to our tree.

Fix by teaching plpgsql_add_initdatums() to include DTYPE_REC variables
as well.  (We don't need to consider other DTYPEs because they don't
represent separately-stored values.)  I failed to resist the temptation
to make some nearby cosmetic adjustments, too.

No back-patch, because there have not been field complaints, and it
seems possible that somewhere out there someone has code depending
on the incorrect behavior.  In any case this change would have no
impact on correctly-written code.

Discussion: https://postgr.es/m/22994.1512800671@sss.pgh.pa.us

src/pl/plpgsql/src/pl_comp.c
src/pl/plpgsql/src/pl_exec.c
src/pl/plpgsql/src/plpgsql.h
src/test/regress/expected/plpgsql.out
src/test/regress/sql/plpgsql.sql

index 1300ea6a525105b81b95d59b0932426ca8d0f056..2d7844bd9daeccfb9e84a6f31a96557be38a6668 100644 (file)
@@ -2384,14 +2384,14 @@ plpgsql_finish_datums(PLpgSQL_function *function)
 
 /* ----------
  * plpgsql_add_initdatums              Make an array of the datum numbers of
- *                                     all the simple VAR datums created since the last call
+ *                                     all the initializable datums created since the last call
  *                                     to this function.
  *
  * If varnos is NULL, we just forget any datum entries created since the
  * last call.
  *
- * This is used around a DECLARE section to create a list of the VARs
- * that have to be initialized at block entry.  Note that VARs can also
+ * This is used around a DECLARE section to create a list of the datums
+ * that have to be initialized at block entry.  Note that datums can also
  * be created elsewhere than DECLARE, eg by a FOR-loop, but it is then
  * the responsibility of special-purpose code to initialize them.
  * ----------
@@ -2402,11 +2402,16 @@ plpgsql_add_initdatums(int **varnos)
        int                     i;
        int                     n = 0;
 
+       /*
+        * The set of dtypes recognized here must match what exec_stmt_block()
+        * cares about (re)initializing at block entry.
+        */
        for (i = datums_last; i < plpgsql_nDatums; i++)
        {
                switch (plpgsql_Datums[i]->dtype)
                {
                        case PLPGSQL_DTYPE_VAR:
+                       case PLPGSQL_DTYPE_REC:
                                n++;
                                break;
 
@@ -2427,6 +2432,7 @@ plpgsql_add_initdatums(int **varnos)
                                switch (plpgsql_Datums[i]->dtype)
                                {
                                        case PLPGSQL_DTYPE_VAR:
+                                       case PLPGSQL_DTYPE_REC:
                                                (*varnos)[n++] = plpgsql_Datums[i]->dno;
 
                                        default:
index 1959d6dc424bf183fad9c35394bc78cc460cdbf3..fa4d573e50c4ccf14217f76302bfb42627d18ebe 100644 (file)
@@ -1184,7 +1184,6 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
 {
        volatile int rc = -1;
        int                     i;
-       int                     n;
 
        /*
         * First initialize all variables declared in this block
@@ -1193,13 +1192,17 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
 
        for (i = 0; i < block->n_initvars; i++)
        {
-               n = block->initvarnos[i];
+               int                     n = block->initvarnos[i];
+               PLpgSQL_datum *datum = estate->datums[n];
 
-               switch (estate->datums[n]->dtype)
+               /*
+                * The set of dtypes handled here must match plpgsql_add_initdatums().
+                */
+               switch (datum->dtype)
                {
                        case PLPGSQL_DTYPE_VAR:
                                {
-                                       PLpgSQL_var *var = (PLpgSQL_var *) (estate->datums[n]);
+                                       PLpgSQL_var *var = (PLpgSQL_var *) datum;
 
                                        /*
                                         * Free any old value, in case re-entering block, and
@@ -1241,7 +1244,7 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
 
                        case PLPGSQL_DTYPE_REC:
                                {
-                                       PLpgSQL_rec *rec = (PLpgSQL_rec *) (estate->datums[n]);
+                                       PLpgSQL_rec *rec = (PLpgSQL_rec *) datum;
 
                                        if (rec->freetup)
                                        {
@@ -1258,13 +1261,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
                                }
                                break;
 
-                       case PLPGSQL_DTYPE_RECFIELD:
-                       case PLPGSQL_DTYPE_ARRAYELEM:
-                               break;
-
                        default:
-                               elog(ERROR, "unrecognized dtype: %d",
-                                        estate->datums[n]->dtype);
+                               elog(ERROR, "unrecognized dtype: %d", datum->dtype);
                }
        }
 
index 8448578537008e33cd6bbff1eec6466cb1ff4bea..39bd82acd19f4facf5b356c3bb5705587b3d8d9d 100644 (file)
@@ -407,8 +407,8 @@ typedef struct PLpgSQL_stmt_block
        int                     lineno;
        char       *label;
        List       *body;                       /* List of statements */
-       int                     n_initvars;
-       int                *initvarnos;
+       int                     n_initvars;             /* Length of initvarnos[] */
+       int                *initvarnos;         /* dnos of variables declared in this block */
        PLpgSQL_exception_block *exceptions;
 } PLpgSQL_stmt_block;
 
index d6e5bc335369ca12c121a7fafc700188922b388d..29f9e86d5608fe92792e93fa689c9fe7b0bb52da 100644 (file)
@@ -5025,6 +5025,33 @@ select scope_test();
 (1 row)
 
 drop function scope_test();
+-- Check that variables are reinitialized on block re-entry.
+do $$
+begin
+  for i in 1..3 loop
+    declare
+      x int;
+      y int := i;
+      r record;
+    begin
+      if i = 1 then
+        x := 42;
+        r := row(i, i+1);
+      end if;
+      raise notice 'x = %', x;
+      raise notice 'y = %', y;
+      raise notice 'r = %', r;
+    end;
+  end loop;
+end$$;
+NOTICE:  x = 42
+NOTICE:  y = 1
+NOTICE:  r = (1,2)
+NOTICE:  x = <NULL>
+NOTICE:  y = 2
+ERROR:  record "r" is not assigned yet
+DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.
+CONTEXT:  PL/pgSQL function inline_code_block line 15 at RAISE
 -- Check handling of conflicts between plpgsql vars and table columns.
 set plpgsql.variable_conflict = error;
 create function conflict_test() returns setof int8_tbl as $$
index 1c355132b77fcae683e620ba1a7c61b5e354a770..07b6fc89716d111f41f6b97a751f388fd54ee478 100644 (file)
@@ -4014,6 +4014,27 @@ select scope_test();
 
 drop function scope_test();
 
+-- Check that variables are reinitialized on block re-entry.
+
+do $$
+begin
+  for i in 1..3 loop
+    declare
+      x int;
+      y int := i;
+      r record;
+    begin
+      if i = 1 then
+        x := 42;
+        r := row(i, i+1);
+      end if;
+      raise notice 'x = %', x;
+      raise notice 'y = %', y;
+      raise notice 'r = %', r;
+    end;
+  end loop;
+end$$;
+
 -- Check handling of conflicts between plpgsql vars and table columns.
 
 set plpgsql.variable_conflict = error;