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
This commit is contained in:
Tom Lane 2016-12-13 16:33:03 -05:00
parent 1f542a2eac
commit 55caaaeba8
3 changed files with 33 additions and 22 deletions

View File

@ -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> expr_until_then expr_until_loop opt_expr_until_when
%type <expr> opt_exitcond %type <expr> opt_exitcond
%type <ival> assign_var foreach_slice %type <datum> assign_var
%type <var> cursor_variable %type <var> cursor_variable
%type <datum> decl_cursor_arg %type <datum> decl_cursor_arg
%type <forvariable> for_variable %type <forvariable> for_variable
%type <ival> foreach_slice
%type <stmt> for_control %type <stmt> for_control
%type <str> any_identifier opt_block_label opt_loop_label opt_label %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 <boolean> getdiag_area_opt
%type <list> getdiag_list %type <list> getdiag_list
%type <diagitem> getdiag_list_item %type <diagitem> getdiag_list_item
%type <ival> getdiag_item getdiag_target %type <datum> getdiag_target
%type <ival> getdiag_item
%type <ival> opt_scrollable %type <ival> opt_scrollable
%type <fetch> opt_fetch_direction %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 = palloc0(sizeof(PLpgSQL_stmt_assign));
new->cmd_type = PLPGSQL_STMT_ASSIGN; new->cmd_type = PLPGSQL_STMT_ASSIGN;
new->lineno = plpgsql_location_to_lineno(@1); new->lineno = plpgsql_location_to_lineno(@1);
new->varno = $1; new->varno = $1->dno;
new->expr = $3; new->expr = $3;
$$ = (PLpgSQL_stmt *)new; $$ = (PLpgSQL_stmt *)new;
@ -1014,7 +1016,7 @@ getdiag_list_item : getdiag_target assign_operator getdiag_item
PLpgSQL_diag_item *new; PLpgSQL_diag_item *new;
new = palloc(sizeof(PLpgSQL_diag_item)); new = palloc(sizeof(PLpgSQL_diag_item));
new->target = $1; new->target = $1->dno;
new->kind = $3; new->kind = $3;
$$ = new; $$ = new;
@ -1069,17 +1071,16 @@ getdiag_item :
} }
; ;
getdiag_target : T_DATUM getdiag_target : assign_var
{ {
check_assignable($1.datum, @1); if ($1->dtype == PLPGSQL_DTYPE_ROW ||
if ($1.datum->dtype == PLPGSQL_DTYPE_ROW || $1->dtype == PLPGSQL_DTYPE_REC)
$1.datum->dtype == PLPGSQL_DTYPE_REC)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR), (errcode(ERRCODE_SYNTAX_ERROR),
errmsg("\"%s\" is not a scalar variable", errmsg("\"%s\" is not a scalar variable",
NameOfDatum(&($1))), ((PLpgSQL_variable *) $1)->refname),
parser_errposition(@1))); parser_errposition(@1)));
$$ = $1.datum->dno; $$ = $1;
} }
| T_WORD | T_WORD
{ {
@ -1097,7 +1098,7 @@ getdiag_target : T_DATUM
assign_var : T_DATUM assign_var : T_DATUM
{ {
check_assignable($1.datum, @1); check_assignable($1.datum, @1);
$$ = $1.datum->dno; $$ = $1.datum;
} }
| assign_var '[' expr_until_rightbracket | assign_var '[' expr_until_rightbracket
{ {
@ -1106,13 +1107,13 @@ assign_var : T_DATUM
new = palloc0(sizeof(PLpgSQL_arrayelem)); new = palloc0(sizeof(PLpgSQL_arrayelem));
new->dtype = PLPGSQL_DTYPE_ARRAYELEM; new->dtype = PLPGSQL_DTYPE_ARRAYELEM;
new->subscript = $3; new->subscript = $3;
new->arrayparentno = $1; new->arrayparentno = $1->dno;
/* initialize cached type data to "not valid" */ /* initialize cached type data to "not valid" */
new->parenttypoid = InvalidOid; new->parenttypoid = InvalidOid;
plpgsql_adddatum((PLpgSQL_datum *) new); plpgsql_adddatum((PLpgSQL_datum *) new);
$$ = new->dno; $$ = (PLpgSQL_datum *) new;
} }
; ;
@ -2173,7 +2174,13 @@ stmt_null : K_NULL ';'
cursor_variable : T_DATUM 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, ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH), (errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cursor variable must be a simple variable"), errmsg("cursor variable must be a simple variable"),

View File

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

View File

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