From: Tom Lane Date: Tue, 13 Dec 2016 21:33:03 +0000 (-0500) Subject: Improve handling of array elements as getdiag_targets and cursor_variables. X-Git-Tag: REL_10_BETA1~1254 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=55caaaeba877eac1feb6481fb413fa04ae9046ac;p=postgresql 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 0b41e3acb6..3802a5bea2 100644 --- 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 assign_var foreach_slice +%type 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 147fb9f2bb..ac8dd91d03 100644 --- 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 49223ff2b9..e7445be189 100644 --- 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;