Hi.
I have tried to do some review of this patch. Below are my comments.
Introduction:
This patch fixes and adds the following functionality:
- %TYPE - now can be used for composite types.
- %ARRAYTYPE - new functionality, provides the array type from a
variable or table column.
- %ELEMENTTYPE - new funcitonality, provides the element type of a given
array.
New regression tests are included in the patch. Changes to the
documentation are not provided.
Initial Run:
The patch applies correctly to HEAD. Regression tests pass successfully,
without errors. It seems that the patch work as you expected.
Performance:
It seems patch have not possible performance issues for the existing
functionality.
Coding:
The style looks fine. I attached the patch that does some corrections in
code and documentation. I have corrected indentation in pl_comp.c and
"read_datatype" function in pl_gram.y. I think changes in
"read_datatype" function would be better to avoid a code duplication.
But I could be wrong of course.
Conclusion:
The patch could be applied on master with documentation corrections. But
I'm not sure that your task could be resloved only by adding %ARRAYTYPE
and %ELEMENTTYPE. Maybe you will give some examples?
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 710,715 **** SELECT merge_fields(t.*) FROM table1 t WHERE ... ;
--- 710,756 ----
</para>
</sect2>
+ <sect2 id="plpgsql-declaration-arraytype">
+ <title>Array Types</title>
+
+ <synopsis>
+ <replaceable>variable</replaceable>%ARRAYTYPE
+ </synopsis>
+
+ <para>
+ <literal>%ARRAYTYPE</literal> provides the array type from a variable or
+ table column. You can use this to declare array variables that will
+ hold database values. To declare an array variable that will hold
+ values from <literal>users.user_id</> you can write:
+ <programlisting>
+ user_id users.user_id%ARRAYTYPE;
+ </programlisting>
+ </para>
+
+ <para>
+ By using <literal>%ARRAYTYPE</literal> you don't need to know the data
+ type of elements you are referencing.
+ </para>
+ </sect2>
+
+ <sect2 id="plpgsql-declaration-elementtype">
+ <title>Array Element Types</title>
+
+ <synopsis>
+ <replaceable>variable</replaceable>%ELEMENTTYPE
+ </synopsis>
+
+ <para>
+ <literal>%ELEMENTTYPE</literal> provides the element type of a given
+ array. To declare a variable with the same data type as
+ <literal>users</> array element you can write:
+ <programlisting>
+ user_id users%ELEMENTTYPE;
+ </programlisting>
+ </para>
+
+ </sect2>
+
<sect2 id="plpgsql-declaration-collation">
<title>Collation of <application>PL/pgSQL</application> Variables</title>
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***************
*** 1619,1625 **** plpgsql_parse_tripword(char *word1, char *word2, char *word3,
/*
* Derive type from ny base type controlled by reftype_mode
- *
*/
static PLpgSQL_type *
derive_type(PLpgSQL_type *base_type, int reftype_mode)
--- 1619,1624 ----
***************
*** 1661,1667 **** derive_type(PLpgSQL_type *base_type, int reftype_mode)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("there are not array type for type %s",
! format_type_be(base_type->typoid))));
return plpgsql_build_datatype(typoid, -1,
plpgsql_curr_compile->fn_input_collation);
--- 1660,1666 ----
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("there are not array type for type %s",
! format_type_be(base_type->typoid))));
return plpgsql_build_datatype(typoid, -1,
plpgsql_curr_compile->fn_input_collation);
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 2694,2700 **** read_datatype(int tok)
StringInfoData ds;
char *type_name;
int startlocation;
! PLpgSQL_type *result;
int parenlevel = 0;
/* Should only be called while parsing DECLARE sections */
--- 2694,2700 ----
StringInfoData ds;
char *type_name;
int startlocation;
! PLpgSQL_type *result = 0;
int parenlevel = 0;
/* Should only be called while parsing DECLARE sections */
***************
*** 2720,2751 **** read_datatype(int tok)
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
- {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
! {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ELEMENT);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
! {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ARRAY);
- if (result)
- return result;
- }
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
- {
result = plpgsql_parse_wordrowtype(dtname);
! if (result)
! return result;
! }
}
}
else if (plpgsql_token_is_unreserved_keyword(tok))
--- 2720,2737 ----
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! else if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ELEMENT);
! else if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ARRAY);
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
result = plpgsql_parse_wordrowtype(dtname);
! if (result)
! return result;
}
}
else if (plpgsql_token_is_unreserved_keyword(tok))
***************
*** 2758,2789 **** read_datatype(int tok)
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
- {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
! {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ELEMENT);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
! {
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ARRAY);
- if (result)
- return result;
- }
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
- {
result = plpgsql_parse_wordrowtype(dtname);
! if (result)
! return result;
! }
}
}
else if (tok == T_CWORD)
--- 2744,2761 ----
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! else if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ELEMENT);
! else if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ARRAY);
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
result = plpgsql_parse_wordrowtype(dtname);
! if (result)
! return result;
}
}
else if (tok == T_CWORD)
***************
*** 2796,2827 **** read_datatype(int tok)
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
- {
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_TYPE);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
! {
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_ELEMENT);
! if (result)
! return result;
! }
! if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
! {
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_ARRAY);
- if (result)
- return result;
- }
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
- {
result = plpgsql_parse_cwordrowtype(dtnames);
! if (result)
! return result;
! }
}
}
--- 2768,2785 ----
tok = yylex();
if (tok_is_keyword(tok, &yylval,
K_TYPE, "type"))
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_TYPE);
! else if (tok_is_keyword(tok, &yylval,
! K_ELEMENTTYPE, "elementtype"))
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_ELEMENT);
! else if (tok_is_keyword(tok, &yylval,
! K_ARRAYTYPE, "arraytype"))
result = plpgsql_parse_cwordtype(dtnames, PLPGSQL_REFTYPE_ARRAY);
else if (tok_is_keyword(tok, &yylval,
K_ROWTYPE, "rowtype"))
result = plpgsql_parse_cwordrowtype(dtnames);
! if (result)
! return result;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
