In plpgsql, don't preassign portal names to bound cursor variables.

A refcursor variable that is bound to a specific query (by declaring
it with "CURSOR FOR") now chooses a portal name in the same way as an
unbound, plain refcursor variable.  Its string value starts out as
NULL, and unless that's overridden by manual assignment, it will be
replaced by a unique-within-session portal name during OPEN.

The previous behavior was to initialize such variables to contain
their own name, resulting in that also being the portal name unless
the user overwrote it before OPEN.  The trouble with this is that
it causes failures due to conflicting portal names if the same
cursor variable name is used in different functions.  It is pretty
non-orthogonal to have bound and unbound refcursor variables behave
differently on this point, too, so let's change it.

This change can cause compatibility problems for applications that
open a bound cursor in a plpgsql function and then use it in the
calling code without explicitly passing back the refcursor value
(portal name).  If the calling code simply assumes that the portal
name matches the called function's variable name, it will now fail.
That can be fixed by explicitly assigning a string value to the
refcursor variable before OPEN, e.g.

    DECLARE myc CURSOR FOR SELECT ...;
    BEGIN
      myc := 'myc';  -- add this
      OPEN myc;

We have no documentation examples showing the troublesome usage
pattern, so we can hope it's rare in practice.

Patch by me; thanks to Pavel Stehule and Jan Wieck for review.

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

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 2b2a1a82159292bea04a9bf0cf6082bc23fe8156..7fc8d1467f927ab16e5d3efa5db70a40385adda7 100644 (file)
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3177,7 +3177,9 @@ DECLARE
     <para>
      Before a cursor can be used to retrieve rows, it must be
      <firstterm>opened</firstterm>. (This is the equivalent action to the SQL
-     command <command>DECLARE CURSOR</command>.) <application>PL/pgSQL</application> has
+     command <link linkend="sql-declare"><command>DECLARE
+     CURSOR</command></link>.)
+     <application>PL/pgSQL</application> has
      three forms of the <command>OPEN</command> statement, two of which use unbound
      cursor variables while the third uses a bound cursor variable.
     </para>
@@ -3187,9 +3189,28 @@ DECLARE
       Bound cursor variables can also be used without explicitly opening the cursor,
       via the <command>FOR</command> statement described in
       <xref linkend="plpgsql-cursor-for-loop"/>.
+      A <command>FOR</command> loop will open the cursor and then
+      close it again when the loop completes.
      </para>
     </note>
 
+    <indexterm>
+     <primary>portal</primary>
+     <secondary>in PL/pgSQL</secondary>
+    </indexterm>
+
+    <para>
+     Opening a cursor involves creating a server-internal data structure
+     called a <firstterm>portal</firstterm>, which holds the execution
+     state for the cursor's query.  A portal has a name, which must be
+     unique within the session for the duration of the portal's existence.
+     By default, <application>PL/pgSQL</application> will assign a unique
+     name to each portal it creates.  However, if you assign a non-null
+     string value to a cursor variable, that string will be used as its
+     portal name.  This feature can be used as described in
+     <xref linkend="plpgsql-cursor-returning"/>.
+    </para>
+
     <sect3>
      <title><command>OPEN FOR</command> <replaceable>query</replaceable></title>
 
@@ -3338,7 +3359,7 @@ BEGIN
      opened the cursor to begin with.  You can return a <type>refcursor</type>
      value out of a function and let the caller operate on the cursor.
      (Internally, a <type>refcursor</type> value is simply the string name
-     of a so-called portal containing the active query for the cursor.  This name
+     of the portal containing the active query for the cursor.  This name
      can be passed around, assigned to other <type>refcursor</type> variables,
      and so on, without disturbing the portal.)
     </para>
@@ -3480,7 +3501,7 @@ CLOSE curs1;
        </para>
      </sect3>
 
-    <sect3>
+    <sect3 id="plpgsql-cursor-returning">
      <title>Returning Cursors</title>
 
        <para>
@@ -3500,7 +3521,8 @@ CLOSE curs1;
         simply assign a string to the <type>refcursor</type> variable before
         opening it.  The string value of the <type>refcursor</type> variable
         will be used by <command>OPEN</command> as the name of the underlying portal.
-        However, if the <type>refcursor</type> variable is null,
+        However, if the <type>refcursor</type> variable's value is null
+        (as it will be by default), then
         <command>OPEN</command> automatically generates a name that does not
         conflict with any existing portal, and assigns it to the
         <type>refcursor</type> variable.
@@ -3508,12 +3530,12 @@ CLOSE curs1;
 
        <note>
         <para>
-         A bound cursor variable is initialized to the string value
-         representing its name, so that the portal name is the same as
-         the cursor variable name, unless the programmer overrides it
-         by assignment before opening the cursor.  But an unbound cursor
-         variable defaults to the null value initially, so it will receive
-         an automatically-generated unique name, unless overridden.
+         Prior to <productname>PostgreSQL</productname> 16, bound cursor
+         variables were initialized to contain their own names, rather
+         than being left as null, so that the underlying portal name would
+         be the same as the cursor variable's name by default.  This was
+         changed because it created too much risk of conflicts between
+         similarly-named cursors in different functions.
         </para>
        </note>
 

diff --git a/doc/src/sgml/ref/declare.sgml b/doc/src/sgml/ref/declare.sgml
index bbbd335bd0bf4fde7caed06f2ea03fd9e233da11..5712825314e5e5ec53fb0bb834b49165ef4c16fe 100644 (file)
--- a/doc/src/sgml/ref/declare.sgml
+++ b/doc/src/sgml/ref/declare.sgml
@@ -13,6 +13,11 @@ PostgreSQL documentation
   <secondary>DECLARE</secondary>
  </indexterm>
 
+ <indexterm>
+  <primary>portal</primary>
+  <secondary>DECLARE</secondary>
+ </indexterm>
+
  <refmeta>
   <refentrytitle>DECLARE</refentrytitle>
   <manvolnum>7</manvolnum>
@@ -61,6 +66,8 @@ DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ ASENSITIV
     <listitem>
      <para>
       The name of the cursor to be created.
+      This must be different from any other active cursor name in the
+      session.
      </para>
     </listitem>
    </varlistentry>
@@ -305,6 +312,15 @@ DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ ASENSITIV
     <command>DECLARE</command> and <command>OPEN</command> statements.
    </para>
 
+   <para>
+    The server data structure underlying an open cursor is called a
+    <firstterm>portal</firstterm>.  Portal names are exposed in the
+    client protocol: a client can fetch rows directly from an open
+    portal, if it knows the portal name.  When creating a cursor with
+    <command>DECLARE</command>, the portal name is the same as the
+    cursor name.
+   </para>
+
    <para>
     You can see all available cursors by querying the <link
     linkend="view-pg-cursors"><structname>pg_cursors</structname></link>

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index fe63766e5d55575f09237a7da783541789ffdc2c..a9de7936ce1f1edd1f25b2c5387f2adf646071c8 100644 (file)
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -534,10 +534,6 @@ decl_statement     : decl_varname decl_const decl_datatype decl_collate decl_notnull
                                  decl_cursor_args decl_is_for decl_cursor_query
                                        {
                                                PLpgSQL_var *new;
-                                               PLpgSQL_expr *curname_def;
-                                               char            buf[NAMEDATALEN * 2 + 64];
-                                               char       *cp1;
-                                               char       *cp2;
 
                                                /* pop local namespace for cursor args */
                                                plpgsql_ns_pop();
@@ -550,29 +546,6 @@ decl_statement     : decl_varname decl_const decl_datatype decl_collate decl_notnull
                                                                                                                                                  NULL),
                                                                                                   true);
 
-                                               curname_def = palloc0(sizeof(PLpgSQL_expr));
-
-                                               /* Note: refname has been truncated to NAMEDATALEN */
-                                               cp1 = new->refname;
-                                               cp2 = buf;
-                                               /*
-                                                * Don't trust standard_conforming_strings here;
-                                                * it might change before we use the string.
-                                                */
-                                               if (strchr(cp1, '\\') != NULL)
-                                                       *cp2++ = ESCAPE_STRING_SYNTAX;
-                                               *cp2++ = '\'';
-                                               while (*cp1)
-                                               {
-                                                       if (SQL_STR_DOUBLE(*cp1, true))
-                                                               *cp2++ = *cp1;
-                                                       *cp2++ = *cp1++;
-                                               }
-                                               strcpy(cp2, "'::pg_catalog.refcursor");
-                                               curname_def->query = pstrdup(buf);
-                                               curname_def->parseMode = RAW_PARSE_PLPGSQL_EXPR;
-                                               new->default_val = curname_def;
-
                                                new->cursor_explicit_expr = $7;
                                                if ($5 == NULL)
                                                        new->cursor_explicit_argrow = -1;

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 08e42f17dc23ba8b98de906f11bed3eb874cc0ad..cdc519256a741edc95fa0a69bc9ad6709aab5cac 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3482,6 +3482,9 @@ declare
   c2 cursor
        for select * from generate_series(41,43) i;
 begin
+  -- assign portal names to cursors to get stable output
+  c := 'c';
+  c2 := 'c2';
   for r in c(5,7) loop
     raise notice '% from %', r.i, c;
   end loop;
@@ -3624,6 +3627,22 @@ select * from forc_test;
 (10 rows)
 
 drop function forc01();
+-- it's okay to re-use a cursor variable name, even when bound
+do $$
+declare cnt int := 0;
+  c1 cursor for select * from forc_test;
+begin
+  for r1 in c1 loop
+    declare c1 cursor for select * from forc_test;
+    begin
+      for r2 in c1 loop
+        cnt := cnt + 1;
+      end loop;
+    end;
+  end loop;
+  raise notice 'cnt = %', cnt;
+end $$;
+NOTICE:  cnt = 100
 -- fail because cursor has no query bound to it
 create or replace function forc_bad() returns void as $$
 declare

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 588c331033739bbb5cdff2c1a44b3408bc8bc276..9a53b150814baac3ff7c04f9e7d9e0ab0f929680 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2929,6 +2929,9 @@ declare
   c2 cursor
        for select * from generate_series(41,43) i;
 begin
+  -- assign portal names to cursors to get stable output
+  c := 'c';
+  c2 := 'c2';
   for r in c(5,7) loop
     raise notice '% from %', r.i, c;
   end loop;
@@ -3002,6 +3005,23 @@ select * from forc_test;
 
 drop function forc01();
 
+-- it's okay to re-use a cursor variable name, even when bound
+
+do $$
+declare cnt int := 0;
+  c1 cursor for select * from forc_test;
+begin
+  for r1 in c1 loop
+    declare c1 cursor for select * from forc_test;
+    begin
+      for r2 in c1 loop
+        cnt := cnt + 1;
+      end loop;
+    end;
+  end loop;
+  raise notice 'cnt = %', cnt;
+end $$;
+
 -- fail because cursor has no query bound to it
 
 create or replace function forc_bad() returns void as $$