From 43b46aae12b220b7eca8250c6c361083f35985a0 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 5 Jan 2024 14:32:34 -0500 Subject: [PATCH] Clean up some edge cases in plpgsql's %TYPE parsing. Support referencing a composite-type variable in %TYPE. Remove the undocumented, untested, and pretty useless ability to have the subject of %TYPE be an (unqualified) type name. You get the same result by just not writing %TYPE. Add or adjust some test cases to improve code coverage here. Discussion: https://postgr.es/m/716852.1704402127@sss.pgh.pa.us --- .../plpgsql/src/expected/plpgsql_record.out | 46 ++++++++++++++ src/pl/plpgsql/src/pl_comp.c | 61 ++++++++----------- src/pl/plpgsql/src/sql/plpgsql_record.sql | 40 ++++++++++++ src/test/regress/expected/plpgsql.out | 10 +-- src/test/regress/sql/plpgsql.sql | 8 +-- 5 files changed, 119 insertions(+), 46 deletions(-) diff --git a/src/pl/plpgsql/src/expected/plpgsql_record.out b/src/pl/plpgsql/src/expected/plpgsql_record.out index afb922df29..a9b5b778ef 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_record.out +++ b/src/pl/plpgsql/src/expected/plpgsql_record.out @@ -306,6 +306,52 @@ NOTICE: r1 = (1,2) ERROR: record "r1" has no field "nosuchfield" CONTEXT: SQL expression "r1.nosuchfield" PL/pgSQL function inline_code_block line 9 at RAISE +-- check %type with block-qualified variable names +do $$ +<> +declare + v int; + r two_int8s; + v1 v%type; + v2 blk.v%type; + r1 r%type; + r2 blk.r%type; +begin + raise notice '%', pg_typeof(v1); + raise notice '%', pg_typeof(v2); + raise notice '%', pg_typeof(r1); + raise notice '%', pg_typeof(r2); +end$$; +NOTICE: integer +NOTICE: integer +NOTICE: two_int8s +NOTICE: two_int8s +-- check that type record can be passed through %type +do $$ +declare r1 record; + r2 r1%type; +begin + r2 := row(1,2); + raise notice 'r2 = %', r2; + r2 := row(3,4,5); + raise notice 'r2 = %', r2; +end$$; +NOTICE: r2 = (1,2) +NOTICE: r2 = (3,4,5) +-- arrays of record are not supported at the moment +do $$ +declare r1 record[]; +begin +end$$; +ERROR: variable "r1" has pseudo-type record[] +CONTEXT: compilation of PL/pgSQL function "inline_code_block" near line 2 +do $$ +declare r1 record; + r2 r1%type[]; +begin +end$$; +ERROR: variable "r2" has pseudo-type record[] +CONTEXT: compilation of PL/pgSQL function "inline_code_block" near line 3 -- check repeated assignments to composite fields create table some_table (id int, data text); do $$ diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index b745eaa3f8..f18e8e3c64 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -1596,8 +1596,8 @@ plpgsql_parse_tripword(char *word1, char *word2, char *word3, /* ---------- - * plpgsql_parse_wordtype The scanner found word%TYPE. word can be - * a variable name or a basetype. + * plpgsql_parse_wordtype The scanner found word%TYPE. word should be + * a pre-existing variable name. * * Returns datatype struct, or NULL if no match found for word. * ---------- @@ -1605,10 +1605,7 @@ plpgsql_parse_tripword(char *word1, char *word2, char *word3, PLpgSQL_type * plpgsql_parse_wordtype(char *ident) { - PLpgSQL_type *dtype; PLpgSQL_nsitem *nse; - TypeName *typeName; - HeapTuple typeTup; /* * Do a lookup in the current namespace stack @@ -1623,39 +1620,13 @@ plpgsql_parse_wordtype(char *ident) { case PLPGSQL_NSTYPE_VAR: return ((PLpgSQL_var *) (plpgsql_Datums[nse->itemno]))->datatype; - - /* XXX perhaps allow REC/ROW here? */ - + case PLPGSQL_NSTYPE_REC: + return ((PLpgSQL_rec *) (plpgsql_Datums[nse->itemno]))->datatype; default: return NULL; } } - /* - * Word wasn't found in the namespace stack. Try to find a data type with - * that name, but ignore shell types and complex types. - */ - typeName = makeTypeName(ident); - typeTup = LookupTypeName(NULL, typeName, NULL, false); - if (typeTup) - { - Form_pg_type typeStruct = (Form_pg_type) GETSTRUCT(typeTup); - - if (!typeStruct->typisdefined || - typeStruct->typrelid != InvalidOid) - { - ReleaseSysCache(typeTup); - return NULL; - } - - dtype = build_datatype(typeTup, -1, - plpgsql_curr_compile->fn_input_collation, - typeName); - - ReleaseSysCache(typeTup); - return dtype; - } - /* * Nothing found - up to now it's a word without any special meaning for * us. @@ -1666,6 +1637,9 @@ plpgsql_parse_wordtype(char *ident) /* ---------- * plpgsql_parse_cwordtype Same lookup for compositeword%TYPE + * + * Here, we allow either a block-qualified variable name, or a reference + * to a column of some table. * ---------- */ PLpgSQL_type * @@ -1673,6 +1647,7 @@ plpgsql_parse_cwordtype(List *idents) { PLpgSQL_type *dtype = NULL; PLpgSQL_nsitem *nse; + int nnames; const char *fldname; Oid classOid; HeapTuple classtup = NULL; @@ -1688,21 +1663,27 @@ plpgsql_parse_cwordtype(List *idents) if (list_length(idents) == 2) { /* - * Do a lookup in the current namespace stack. We don't need to check - * number of names matched, because we will only consider scalar - * variables. + * Do a lookup in the current namespace stack */ nse = plpgsql_ns_lookup(plpgsql_ns_top(), false, strVal(linitial(idents)), strVal(lsecond(idents)), NULL, - NULL); + &nnames); if (nse != NULL && nse->itemtype == PLPGSQL_NSTYPE_VAR) { + /* Block-qualified reference to scalar variable. */ dtype = ((PLpgSQL_var *) (plpgsql_Datums[nse->itemno]))->datatype; goto done; } + else if (nse != NULL && nse->itemtype == PLPGSQL_NSTYPE_REC && + nnames == 2) + { + /* Block-qualified reference to record variable. */ + dtype = ((PLpgSQL_rec *) (plpgsql_Datums[nse->itemno]))->datatype; + goto done; + } /* * First word could also be a table name @@ -1716,6 +1697,12 @@ plpgsql_parse_cwordtype(List *idents) { RangeVar *relvar; + /* + * We could check for a block-qualified reference to a field of a + * record variable, but %TYPE is documented as applying to variables, + * not fields of variables. Things would get rather ambiguous if we + * allowed either interpretation. + */ relvar = makeRangeVar(strVal(linitial(idents)), strVal(lsecond(idents)), -1); diff --git a/src/pl/plpgsql/src/sql/plpgsql_record.sql b/src/pl/plpgsql/src/sql/plpgsql_record.sql index db655335b1..96dcc414e9 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_record.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_record.sql @@ -199,6 +199,46 @@ begin raise notice 'r1.nosuchfield = %', r1.nosuchfield; end$$; +-- check %type with block-qualified variable names +do $$ +<> +declare + v int; + r two_int8s; + v1 v%type; + v2 blk.v%type; + r1 r%type; + r2 blk.r%type; +begin + raise notice '%', pg_typeof(v1); + raise notice '%', pg_typeof(v2); + raise notice '%', pg_typeof(r1); + raise notice '%', pg_typeof(r2); +end$$; + +-- check that type record can be passed through %type +do $$ +declare r1 record; + r2 r1%type; +begin + r2 := row(1,2); + raise notice 'r2 = %', r2; + r2 := row(3,4,5); + raise notice 'r2 = %', r2; +end$$; + +-- arrays of record are not supported at the moment +do $$ +declare r1 record[]; +begin +end$$; + +do $$ +declare r1 record; + r2 r1%type[]; +begin +end$$; + -- check repeated assignments to composite fields create table some_table (id int, data text); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 272f5d2111..57c1ae092d 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -5795,18 +5795,18 @@ SELECT * FROM get_from_partitioned_table(1) AS t; (1 row) CREATE OR REPLACE FUNCTION list_partitioned_table() -RETURNS SETOF partitioned_table.a%TYPE AS $$ +RETURNS SETOF public.partitioned_table.a%TYPE AS $$ DECLARE - row partitioned_table%ROWTYPE; - a_val partitioned_table.a%TYPE; + row public.partitioned_table%ROWTYPE; + a_val public.partitioned_table.a%TYPE; BEGIN - FOR row IN SELECT * FROM partitioned_table ORDER BY a LOOP + FOR row IN SELECT * FROM public.partitioned_table ORDER BY a LOOP a_val := row.a; RETURN NEXT a_val; END LOOP; RETURN; END; $$ LANGUAGE plpgsql; -NOTICE: type reference partitioned_table.a%TYPE converted to integer +NOTICE: type reference public.partitioned_table.a%TYPE converted to integer SELECT * FROM list_partitioned_table() AS t; t --- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 924d524094..5a9b2f0040 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -4734,12 +4734,12 @@ END; $$ LANGUAGE plpgsql; SELECT * FROM get_from_partitioned_table(1) AS t; CREATE OR REPLACE FUNCTION list_partitioned_table() -RETURNS SETOF partitioned_table.a%TYPE AS $$ +RETURNS SETOF public.partitioned_table.a%TYPE AS $$ DECLARE - row partitioned_table%ROWTYPE; - a_val partitioned_table.a%TYPE; + row public.partitioned_table%ROWTYPE; + a_val public.partitioned_table.a%TYPE; BEGIN - FOR row IN SELECT * FROM partitioned_table ORDER BY a LOOP + FOR row IN SELECT * FROM public.partitioned_table ORDER BY a LOOP a_val := row.a; RETURN NEXT a_val; END LOOP;