Hi

út 17. 10. 2023 v 3:20 odesílatel Quan Zongliang <quanzongli...@yeah.net>
napsal:

>
> Attached new patch
>    More explicit error messages based on type.
>
>
> On 2023/10/16 18:15, Quan Zongliang wrote:
> >
> >
> > Implement TODO item:
> > PL/pgSQL
> > Incomplete item Allow handling of %TYPE arrays, e.g. tab.col%TYPE[]
> >
> > As a first step, deal only with [], such as
> > xxx.yyy%TYPE[]
> > xxx%TYPE[]
> >
> > It can be extended to support multi-dimensional and complex syntax in
> > the future.
> >
>

I did some deeper check:

- I don't like too much parser's modification (I am sending alternative own
implementation) - the SQL parser allows richer syntax, and for full
functionality is only few lines more

- original patch doesn't solve %ROWTYPE

(2023-11-20 10:04:36) postgres=# select * from foo;
┌────┬────┐
│ a  │ b  │
╞════╪════╡
│ 10 │ 20 │
│ 30 │ 40 │
└────┴────┘
(2 rows)

(2023-11-20 10:08:29) postgres=# do $$
declare v foo%rowtype[];
begin
  v := array(select row(a,b) from foo);
  raise notice '%', v;
end;
$$;
NOTICE:  {"(10,20)","(30,40)"}
DO

- original patch doesn't solve type RECORD
the error message should be more intuitive, although the arrays of record
type can be supported, but it probably needs bigger research.

(2023-11-20 10:10:34) postgres=# do $$
declare r record; v r%type[];
begin
  v := array(select row(a,b) from foo);
  raise notice '%', v;
end;
$$;
ERROR:  syntax error at or near "%"
LINE 2: declare r record; v r%type[];
                             ^
CONTEXT:  invalid type name "r%type[]"

- missing documentation

- I don't like using the word "partitioned" in the regress test name
"partitioned_table". It is confusing

Regards

Pavel



> >
> > --
> > Quan Zongliang
commit 3418f61191fe4b377717451b929cefb8028d3718
Author: ok...@github.com <pavel.steh...@gmail.com>
Date:   Mon Nov 20 08:49:41 2023 +0100

    poc

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index a341cde2c1..83e91d82d5 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2095,6 +2095,29 @@ plpgsql_build_datatype(Oid typeOid, int32 typmod,
 	return typ;
 }
 
+/*
+ * Returns an array for type specified as argument.
+ */
+PLpgSQL_type *
+plpgsql_datatype_arrayof(PLpgSQL_type *dtype)
+{
+	Oid			array_typeid;
+
+	if (dtype->typisarray)
+		return dtype;
+
+	array_typeid = get_array_type(dtype->typoid);
+
+	if (!OidIsValid(dtype->typoid))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("could not find array type for data type %s",
+						format_type_be(dtype->typoid))));
+
+	return plpgsql_build_datatype(array_typeid, dtype->atttypmod,
+								  dtype->collation, NULL);
+}
+
 /*
  * Utility subroutine to make a PLpgSQL_type struct given a pg_type entry
  * and additional details (see comments for plpgsql_build_datatype).
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 6a09bfdd67..4784e8fd5c 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -2789,7 +2789,7 @@ read_datatype(int tok)
 	StringInfoData ds;
 	char	   *type_name;
 	int			startlocation;
-	PLpgSQL_type *result;
+	PLpgSQL_type *result = NULL;
 	int			parenlevel = 0;
 
 	/* Should only be called while parsing DECLARE sections */
@@ -2817,15 +2817,11 @@ read_datatype(int tok)
 							   K_TYPE, "type"))
 			{
 				result = plpgsql_parse_wordtype(dtname);
-				if (result)
-					return result;
 			}
 			else if (tok_is_keyword(tok, &yylval,
 									K_ROWTYPE, "rowtype"))
 			{
 				result = plpgsql_parse_wordrowtype(dtname);
-				if (result)
-					return result;
 			}
 		}
 	}
@@ -2841,15 +2837,11 @@ read_datatype(int tok)
 							   K_TYPE, "type"))
 			{
 				result = plpgsql_parse_wordtype(dtname);
-				if (result)
-					return result;
 			}
 			else if (tok_is_keyword(tok, &yylval,
 									K_ROWTYPE, "rowtype"))
 			{
 				result = plpgsql_parse_wordrowtype(dtname);
-				if (result)
-					return result;
 			}
 		}
 	}
@@ -2865,19 +2857,58 @@ read_datatype(int tok)
 							   K_TYPE, "type"))
 			{
 				result = plpgsql_parse_cwordtype(dtnames);
-				if (result)
-					return result;
 			}
 			else if (tok_is_keyword(tok, &yylval,
 									K_ROWTYPE, "rowtype"))
 			{
 				result = plpgsql_parse_cwordrowtype(dtnames);
-				if (result)
-					return result;
 			}
 		}
 	}
 
+	/* Array declaration can follow, but we check it only for known type */
+	if (result)
+	{
+		bool		be_array = false;
+
+		tok = yylex();
+
+		/*
+		 * SQL syntax allows multiple [] [ iconst ], ARRAY, ARRAY [ ]
+		 * or ARRAY [ icons ]. Should we support all? It is not too hard.
+		 */
+		if (tok_is_keyword(tok, &yylval,
+						   K_ARRAY, "array"))
+		{
+			be_array = true;
+			tok = yylex();
+		}
+
+		if (tok == '[')
+		{
+			be_array = true;
+
+			while (tok == '[')
+			{
+				tok = yylex();
+				if (tok == ICONST)
+					tok = yylex();
+
+				if (tok != ']')
+					yyerror("syntax error, expected \"]\"");
+
+				tok = yylex();
+			}
+		}
+
+		plpgsql_push_back_token(tok);
+
+		if (be_array)
+			result = plpgsql_datatype_arrayof(result);
+
+		return result;
+	}
+
 	while (tok != ';')
 	{
 		if (tok == 0)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 9f0a912115..9da5e5b225 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1249,6 +1249,7 @@ extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
 extern PGDLLEXPORT PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
 														Oid collation,
 														TypeName *origtypname);
+extern PLpgSQL_type *plpgsql_datatype_arrayof(PLpgSQL_type *dtype);
 extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
 												PLpgSQL_type *dtype,
 												bool add2namespace);

Reply via email to