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[email protected]

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_until_then expr_until_loop opt_expr_until_when
 %type    opt_exitcond
 
-%type <ival>   assign_var foreach_slice
+%type <datum>  assign_var
 %type         cursor_variable
 %type   decl_cursor_arg
 %type     for_variable
+%type    foreach_slice
 %type    for_control
 
 %type         any_identifier opt_block_label opt_loop_label opt_label
@@ -209,7 +210,8 @@ static  void            check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %type     getdiag_area_opt
 %type    getdiag_list
 %type  getdiag_list_item
-%type    getdiag_item getdiag_target
+%type   getdiag_target
+%type    getdiag_item
 
 %type    opt_scrollable
 %type   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;