1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-05 07:21:24 +03:00

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  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
src
pl
plpgsql
test
regress

@ -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"),

@ -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();

@ -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;