Improve handling of array elements as getdiag_targets and cursor_variables.

There's no good reason why plpgsql's GET DIAGNOSTICS statement can't
support an array element as target variable, since the execution code
already uses the generic exec_assign_value() function to assign to it.
Hence, refactor the grammar to allow that, by making getdiag_target
depend on the assign_var production.

Ideally we'd also let a cursor_variable expand to an element of a
refcursor[] array, but that's substantially harder since those statements
also have to handle bound-cursor-variable cases.  For now, just make sure
the reported error is sensible, ie "cursor variable must be a simple
variable" not "variable must be of type cursor or refcursor".  The latter
was quite confusing from the user's viewpoint, since what he wrote
satisfies the claimed restriction.

Per bug #14463 from Zhou Digoal.  Given the lack of previous complaints,
I see no need for a back-patch.

Discussion: https://postgr.es/m/20161213152548.14897.81245@wrigleys.postgresql.org

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 0b41e3acb6c14cc2a2148b916b5cde8d412ea0bf..3802a5bea263fd54a2fdc5cc5379c79b91d644c1 100644 (file)
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -180,10 +180,11 @@ static    void                    check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %type <expr>   expr_until_then expr_until_loop opt_expr_until_when
 %type <expr>   opt_exitcond
 
-%type <ival>   assign_var foreach_slice
+%type <datum>  assign_var
 %type <var>            cursor_variable
 %type <datum>  decl_cursor_arg
 %type <forvariable>    for_variable
+%type <ival>   foreach_slice
 %type <stmt>   for_control
 
 %type <str>            any_identifier opt_block_label opt_loop_label opt_label
@@ -209,7 +210,8 @@ static      void                    check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %type <boolean>        getdiag_area_opt
 %type <list>   getdiag_list
 %type <diagitem> getdiag_list_item
-%type <ival>   getdiag_item getdiag_target
+%type <datum>  getdiag_target
+%type <ival>   getdiag_item
 
 %type <ival>   opt_scrollable
 %type <fetch>  opt_fetch_direction
@@ -916,7 +918,7 @@ stmt_assign         : assign_var assign_operator expr_until_semi
                                                new = palloc0(sizeof(PLpgSQL_stmt_assign));
                                                new->cmd_type = PLPGSQL_STMT_ASSIGN;
                                                new->lineno   = plpgsql_location_to_lineno(@1);
-                                               new->varno = $1;
+                                               new->varno = $1->dno;
                                                new->expr  = $3;
 
                                                $$ = (PLpgSQL_stmt *)new;
@@ -1014,7 +1016,7 @@ getdiag_list_item : getdiag_target assign_operator getdiag_item
                                                PLpgSQL_diag_item *new;
 
                                                new = palloc(sizeof(PLpgSQL_diag_item));
-                                               new->target = $1;
+                                               new->target = $1->dno;
                                                new->kind = $3;
 
                                                $$ = new;
@@ -1069,17 +1071,16 @@ getdiag_item :
                                        }
                                ;
 
-getdiag_target : T_DATUM
+getdiag_target : assign_var
                                        {
-                                               check_assignable($1.datum, @1);
-                                               if ($1.datum->dtype == PLPGSQL_DTYPE_ROW ||
-                                                       $1.datum->dtype == PLPGSQL_DTYPE_REC)
+                                               if ($1->dtype == PLPGSQL_DTYPE_ROW ||
+                                                       $1->dtype == PLPGSQL_DTYPE_REC)
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("\"%s\" is not a scalar variable",
-                                                                                       NameOfDatum(&($1))),
+                                                                                       ((PLpgSQL_variable *) $1)->refname),
                                                                         parser_errposition(@1)));
-                                               $$ = $1.datum->dno;
+                                               $$ = $1;
                                        }
                                | T_WORD
                                        {
@@ -1097,7 +1098,7 @@ getdiag_target    : T_DATUM
 assign_var             : T_DATUM
                                        {
                                                check_assignable($1.datum, @1);
-                                               $$ = $1.datum->dno;
+                                               $$ = $1.datum;
                                        }
                                | assign_var '[' expr_until_rightbracket
                                        {
@@ -1106,13 +1107,13 @@ assign_var              : T_DATUM
                                                new = palloc0(sizeof(PLpgSQL_arrayelem));
                                                new->dtype              = PLPGSQL_DTYPE_ARRAYELEM;
                                                new->subscript  = $3;
-                                               new->arrayparentno = $1;
+                                               new->arrayparentno = $1->dno;
                                                /* initialize cached type data to "not valid" */
                                                new->parenttypoid = InvalidOid;
 
                                                plpgsql_adddatum((PLpgSQL_datum *) new);
 
-                                               $$ = new->dno;
+                                               $$ = (PLpgSQL_datum *) new;
                                        }
                                ;
 
@@ -2173,7 +2174,13 @@ stmt_null                : K_NULL ';'
 
 cursor_variable        : T_DATUM
                                        {
-                                               if ($1.datum->dtype != PLPGSQL_DTYPE_VAR)
+                                               /*
+                                                * In principle we should support a cursor_variable
+                                                * that is an array element, but for now we don't, so
+                                                * just throw an error if next token is '['.
+                                                */
+                                               if ($1.datum->dtype != PLPGSQL_DTYPE_VAR ||
+                                                       plpgsql_peek() == '[')
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_DATATYPE_MISMATCH),
                                                                         errmsg("cursor variable must be a simple variable"),

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 147fb9f2bb82b66bd3d24eb9b0c2a87ef58804e9..ac8dd91d03ac516be457f06d0e312255cd8d22ac 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -4623,6 +4623,7 @@ drop function tftest(int);
 create or replace function rttest()
 returns setof int as $$
 declare rc int;
+  rca int[];
 begin
   return query values(10),(20);
   get diagnostics rc = row_count;
@@ -4631,11 +4632,12 @@ begin
   get diagnostics rc = row_count;
   raise notice '% %', found, rc;
   return query execute 'values(10),(20)';
-  get diagnostics rc = row_count;
-  raise notice '% %', found, rc;
+  -- just for fun, let's use array elements as targets
+  get diagnostics rca[1] = row_count;
+  raise notice '% %', found, rca[1];
   return query execute 'select * from (values(10),(20)) f(a) where false';
-  get diagnostics rc = row_count;
-  raise notice '% %', found, rc;
+  get diagnostics rca[2] = row_count;
+  raise notice '% %', found, rca[2];
 end;
 $$ language plpgsql;
 select * from rttest();

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 49223ff2b92b74c403a9aa39b5b75f11d9545759..e7445be189667ab2b9dd22e7474b75d1da797223 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3719,6 +3719,7 @@ drop function tftest(int);
 create or replace function rttest()
 returns setof int as $$
 declare rc int;
+  rca int[];
 begin
   return query values(10),(20);
   get diagnostics rc = row_count;
@@ -3727,11 +3728,12 @@ begin
   get diagnostics rc = row_count;
   raise notice '% %', found, rc;
   return query execute 'values(10),(20)';
-  get diagnostics rc = row_count;
-  raise notice '% %', found, rc;
+  -- just for fun, let's use array elements as targets
+  get diagnostics rca[1] = row_count;
+  raise notice '% %', found, rca[1];
   return query execute 'select * from (values(10),(20)) f(a) where false';
-  get diagnostics rc = row_count;
-  raise notice '% %', found, rc;
+  get diagnostics rca[2] = row_count;
+  raise notice '% %', found, rca[2];
 end;
 $$ language plpgsql;