Обсуждение: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Hi,
We cannot to declare variable with referenced type on other composite variable. This limit is probably artificial, because any composite type is any type too in PostgreSQL.
The issue:
referencing on composite variables doesn't work
do $$ declare x int; y x%type; begin end; $$; -- ok
do $$ declare x pg_class; y x%type; begin end; $$; -- invalid type name "x%type"
do $$ declare x pg_class; y x%rowtype; begin end; $$; -- relation "x" does not exist
do $$ declare x int; y x%type; begin end; $$; -- ok
do $$ declare x pg_class; y x%type; begin end; $$; -- invalid type name "x%type"
do $$ declare x pg_class; y x%rowtype; begin end; $$; -- relation "x" does not exist
The %ROWTYPE needs a record in pg_class. Probably we should not to change it. The change can bring a compatibility issues. So there are two possibilities:
1. %TYPE can be used for any kind of variables. This behave will be consistent with polymorphic parameters - we have "anyelement", and we have not "anyrow".
2. introduce new keyword - %RECTYPE .. it can work, but there will be gap between polymorphic parameters.
Comments, notices?
Regards
Pavel
2015-10-19 9:52 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi,We cannot to declare variable with referenced type on other composite variable. This limit is probably artificial, because any composite type is any type too in PostgreSQL.The issue:referencing on composite variables doesn't work
do $$ declare x int; y x%type; begin end; $$; -- ok
do $$ declare x pg_class; y x%type; begin end; $$; -- invalid type name "x%type"
do $$ declare x pg_class; y x%rowtype; begin end; $$; -- relation "x" does not existThe %ROWTYPE needs a record in pg_class. Probably we should not to change it. The change can bring a compatibility issues. So there are two possibilities:1. %TYPE can be used for any kind of variables. This behave will be consistent with polymorphic parameters - we have "anyelement", and we have not "anyrow".2. introduce new keyword - %RECTYPE .. it can work, but there will be gap between polymorphic parameters.Comments, notices?
Hi
I am sending patch that enables to use references to polymorphic parameters of row types. Another functionality is possibility to get array or element type of referenced variable. It removes some gaps when polymorphic parameters are used.
create type test_composite_type as (x int, y int);
create or replace function test_simple(src anyelement)
returns anyelement as $$
declare dest src%type;
begin
dest := src;
return dest;
end;
$$ language plpgsql;
select test_simple(10);
test_simple
-------------
10
(1 row)
select test_simple('hoj'::text);
test_simple
-------------
hoj
(1 row)
select test_simple((10,20)::test_composite_type);
test_simple
-------------
(10,20)
(1 row)
create or replace function test_poly_element(x anyelement)
returns anyarray as $$
declare result x%arraytype;
begin
result := ARRAY[x];
raise notice '% %', pg_typeof(result), result;
return result;
end;
$$ language plpgsql;
select test_poly_element(1);
NOTICE: integer[] {1}
test_poly_element
-------------------
{1}
(1 row)
select test_poly_element('hoj'::text);
NOTICE: text[] {hoj}
test_poly_element
-------------------
{hoj}
(1 row)
select test_poly_element((10,20)::test_composite_type);
NOTICE: test_composite_type[] {"(10,20)"}
test_poly_element
-------------------
{"(10,20)"}
(1 row)
create or replace function test_poly_array(x anyarray)
returns anyelement as $$
declare result x%elementtype;
begin
result := x[1];
raise notice '% %', pg_typeof(result), result;
return result;
end;
$$ language plpgsql;
select test_poly_array(ARRAY[1]);
NOTICE: integer 1
test_poly_array
-----------------
1
(1 row)
select test_poly_array(ARRAY['hoj'::text]);
NOTICE: text hoj
test_poly_array
-----------------
hoj
(1 row)
select test_poly_array(ARRAY[(10,20)::test_composite_type]);
NOTICE: test_composite_type (10,20)
test_poly_array
-----------------
(10,20)
(1 row)
create type test_composite_type as (x int, y int);
create or replace function test_simple(src anyelement)
returns anyelement as $$
declare dest src%type;
begin
dest := src;
return dest;
end;
$$ language plpgsql;
select test_simple(10);
test_simple
-------------
10
(1 row)
select test_simple('hoj'::text);
test_simple
-------------
hoj
(1 row)
select test_simple((10,20)::test_composite_type);
test_simple
-------------
(10,20)
(1 row)
create or replace function test_poly_element(x anyelement)
returns anyarray as $$
declare result x%arraytype;
begin
result := ARRAY[x];
raise notice '% %', pg_typeof(result), result;
return result;
end;
$$ language plpgsql;
select test_poly_element(1);
NOTICE: integer[] {1}
test_poly_element
-------------------
{1}
(1 row)
select test_poly_element('hoj'::text);
NOTICE: text[] {hoj}
test_poly_element
-------------------
{hoj}
(1 row)
select test_poly_element((10,20)::test_composite_type);
NOTICE: test_composite_type[] {"(10,20)"}
test_poly_element
-------------------
{"(10,20)"}
(1 row)
create or replace function test_poly_array(x anyarray)
returns anyelement as $$
declare result x%elementtype;
begin
result := x[1];
raise notice '% %', pg_typeof(result), result;
return result;
end;
$$ language plpgsql;
select test_poly_array(ARRAY[1]);
NOTICE: integer 1
test_poly_array
-----------------
1
(1 row)
select test_poly_array(ARRAY['hoj'::text]);
NOTICE: text hoj
test_poly_array
-----------------
hoj
(1 row)
select test_poly_array(ARRAY[(10,20)::test_composite_type]);
NOTICE: test_composite_type (10,20)
test_poly_array
-----------------
(10,20)
(1 row)
Regards
Pavel
RegardsPavel
Вложения
On 10/30/15 6:01 AM, Pavel Stehule wrote: > I am sending patch that enables to use references to polymorphic > parameters of row types. Another functionality is possibility to get > array or element type of referenced variable. It removes some gaps when > polymorphic parameters are used. Did this make it into a commitfest? -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
2015-12-21 1:06 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 10/30/15 6:01 AM, Pavel Stehule wrote:I am sending patch that enables to use references to polymorphic
parameters of row types. Another functionality is possibility to get
array or element type of referenced variable. It removes some gaps when
polymorphic parameters are used.
Did this make it into a commitfest?
yes, it is relative trivial small patch without any side effects or possible performance issues.
The important (and possible disputable) part of this patch is new syntax
DECLARE
var othervar%arraytype,
var othervar%arraytype,
var othervar%elementtype;
It is consistent with current state, and doesn't increase a complexity of DECLARE part in plpgsql parser - what was reason for reject this idea 5 years ago (no necessary reserved keywords, ...) .
Regards
Pavel
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
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
Вложения
Hi
2015-12-21 16:21 GMT+01:00 Artur Zakirov <a.zakirov@postgrespro.ru>:
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?
Thank you for review. The changes in code are good idea.
I waited with documentation if there will be some objections to syntax. The month later, there are not any known objection.
The target of this feature isn't using for storing of database objects only, but for storing the values of polymorphic parameters.
CREATE OR REPLACE FUNCTION buble_sort(a anyarray)
RETURNS anyarray AS $$
DECLARE
aux a%ELEMENTTYPE;
repeat_again boolean DEFAULT true;
BEGIN
-- Don't use this code for large arrays!
-- use builtin sort
WHILE repeat_again
repeat_again := false;
FOR i IN array_lower(a, 1) .. array_upper(a, 2)
LOOP
IF a[i] > a[i+1] THEN
aux := a[i+1];
a[i+1] := a[i]; a[i] := aux;
repeat_again := true;
END IF;
END LOOP;
END WHILE;
RETURN a;
END;
$$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION array_init(v anyelement, size integer)
RETURNS anyarray AS $$
DECLARE result v%ARRAYTYPE DEFAULT '{}';
BEGIN
-- prefer builtin function array_fill
FOR i IN 1 .. size
LOOP
result := result || v;
END LOOP;
RETURN result;
END;
$$ LANGUAGE plpgsql;
Regards
Pavel
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Hi
2015-12-21 16:21 GMT+01:00 Artur Zakirov <a.zakirov@postgrespro.ru>:
I merged Artur's patch and appended examples to doc.
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.
I merged Artur's patch and appended examples to doc.
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?
It fixes the most missed/known features related to this part of plpgsql, what I found. But any ideas for this or follofup patches are welcome.
Regards
Pavel
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Вложения
Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
От
Michael Paquier
Дата:
On Tue, Dec 22, 2015 at 5:59 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > Hi > > 2015-12-21 16:21 GMT+01:00 Artur Zakirov <a.zakirov@postgrespro.ru>: >> >> 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. > > > I merged Artur's patch and appended examples to doc. > > >> >> >> 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? > > > It fixes the most missed/known features related to this part of plpgsql, > what I found. But any ideas for this or follofup patches are welcome. Patch moved to next CF, this entry is still very active. -- Michael
> diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
> new file mode 100644
> index 1ae4bb7..c819517
> *** a/src/pl/plpgsql/src/pl_comp.c
> --- b/src/pl/plpgsql/src/pl_comp.c
> *************** plpgsql_parse_tripword(char *word1, char
> *** 1617,1622 ****
> --- 1617,1677 ----
> return false;
> }
>
> + /*
> + * Derive type from ny base type controlled by reftype_mode
> + */
> + static PLpgSQL_type *
> + derive_type(PLpgSQL_type *base_type, int reftype_mode)
> + {
> + Oid typoid;
I think you should add a typedef to the REFTYPE enum, and have this
function take that type rather than int.
> + case PLPGSQL_REFTYPE_ARRAY:
> + {
> + /*
> + * Question: can we allow anyelement (array or nonarray) -> array direction.
> + * if yes, then probably we have to modify enforce_generic_type_consistency,
> + * parse_coerce.c where still is check on scalar type -> raise error
> + * ERROR: 42704: could not find array type for data type integer[]
> + *
> + if (OidIsValid(get_element_type(base_type->typoid)))
> + return base_type;
> + */
I think it would be better to resolve this question outside a code
comment.
> + typoid = get_array_type(base_type->typoid);
> + if (!OidIsValid(typoid))
> + ereport(ERROR,
> + (errcode(ERRCODE_DATATYPE_MISMATCH),
> + errmsg("there are not array type for type %s",
> + format_type_be(base_type->typoid))));
nodeFuncs.c uses this wording:errmsg("could not find array type for data type %s",
which I think you should adopt.
> --- 1681,1687 ----
> * ----------
> */
> PLpgSQL_type *
> ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
> {
> PLpgSQL_type *dtype;
> PLpgSQL_nsitem *nse;
Use the typedef'ed enum, as above.
> --- 1699,1721 ----
> switch (nse->itemtype)
> {
> case PLPGSQL_NSTYPE_VAR:
> ! {
> ! dtype = ((PLpgSQL_var *) (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>
> ! case PLPGSQL_NSTYPE_ROW:
> ! {
> ! dtype = ((PLpgSQL_row *) (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>
> + /*
> + * XXX perhaps allow REC here? Probably it has not any sense, because
> + * in this moment, because PLpgSQL doesn't support rec parameters, so
> + * there should not be any rec polymorphic parameter, and any work can
> + * be done inside function.
> + */
I think you should remove from the "?" onwards in that comment, i.e.
just keep what was already in the original comment (minus the ROW)
> --- 1757,1763 ----
> * ----------
> */
> PLpgSQL_type *
> ! plpgsql_parse_cwordtype(List *idents, int reftype_mode)
> {
> PLpgSQL_type *dtype = NULL;
> PLpgSQL_nsitem *nse;
Typedef.
> --- 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;
> }
This plpgsql parser stuff is pretty tiresome. (Not this patch's fault
-- just saying.)
> *************** extern bool plpgsql_parse_dblword(char *
> *** 961,968 ****
> PLwdatum *wdatum, PLcword *cword);
> extern bool plpgsql_parse_tripword(char *word1, char *word2, char *word3,
> PLwdatum *wdatum, PLcword *cword);
> ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident);
> ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents);
> extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
> --- 973,980 ----
> PLwdatum *wdatum, PLcword *cword);
> extern bool plpgsql_parse_tripword(char *word1, char *word2, char *word3,
> PLwdatum *wdatum, PLcword *cword);
> ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident, int reftype_mode);
> ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents, int reftype_mode);
> extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
By the way, these functions are misnamed after this patch. They are
called "wordtype" and "cwordtype" originally because they accept
"word%TYPE" and "compositeword%TYPE", but after the patch they not only
accept TYPE at the right of the percent sign but also ELEMENTTYPE and
ARRAYTYPE. Not sure that this is something we want to be too strict
about.
> *** a/src/test/regress/expected/plpgsql.out
> --- b/src/test/regress/expected/plpgsql.out
> *************** end;
> *** 5573,5575 ****
> --- 5573,5667 ----
I think you should also add your array_init() example to the test set.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
FWIW the reason I read through this patch is that I wondered if there was anything in common with this other patch https://commitfest.postgresql.org/8/459/ -- and the answer seems to be "no". What that patch does is add a new construct TYPE(1+1) which in this case returns "int4"; I guess if we wanted to augment that functionality to cover Pavel's use case we would additionally need ELEMENTTYPE(somearray) and ARRAYTYPE(some-non-array) in the core grammar ... sounds like a hard sell. BTW are we all agreed that enabling foo%ARRAYTYPE and foo%ELEMENTYPE in plpgsql's DECLARE section is what we want for this? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > BTW are we all agreed that enabling > foo%ARRAYTYPE > and > foo%ELEMENTYPE > in plpgsql's DECLARE section is what we want for this? I know that Oracle uses syntax of this general type, but I've always found it ugly. It's also pretty non-extensible. You could want similar things for range types and any other container types we might get in the future, but clearly adding new reserved words for each one is no good. One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but then you want to make BAR an array of that type rather than a scalar, why not write that as DECLARE BAR FOO%TYPE[]? That seems quite natural to me. I think the part of this patch that makes %TYPE work for more kinds of types is probably a good idea, although I haven't carefully studied exactly what it does. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2016-01-18 22:21 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> BTW are we all agreed that enabling
> foo%ARRAYTYPE
> and
> foo%ELEMENTYPE
> in plpgsql's DECLARE section is what we want for this?
I know that Oracle uses syntax of this general type, but I've always
found it ugly. It's also pretty non-extensible. You could want
similar things for range types and any other container types we might
get in the future, but clearly adding new reserved words for each one
is no good.
It doesn't use reserved worlds.
One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
then you want to make BAR an array of that type rather than a scalar,
why not write that as DECLARE BAR FOO%TYPE[]? That seems quite
natural to me.
what you propose for syntax for taking a element of array?
I think the part of this patch that makes %TYPE work for more kinds of
types is probably a good idea, although I haven't carefully studied
exactly what it does.
I invite any ideas, but currently used notation is only in direction type->array. The working with symbols looks more difficult, than using words (in design area).
More - the textual form is more near to our system of polymorphics types: anyelement, anyarray, ... We have not anyelement[]
Regards
Pavel
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> I know that Oracle uses syntax of this general type, but I've always >> found it ugly. It's also pretty non-extensible. You could want >> similar things for range types and any other container types we might >> get in the future, but clearly adding new reserved words for each one >> is no good. > > It doesn't use reserved worlds. OK - keywords, then. >> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but >> then you want to make BAR an array of that type rather than a scalar, >> why not write that as DECLARE BAR FOO%TYPE[]? That seems quite >> natural to me. > > what you propose for syntax for taking a element of array? No idea. >> I think the part of this patch that makes %TYPE work for more kinds of >> types is probably a good idea, although I haven't carefully studied >> exactly what it does. > > > I invite any ideas, but currently used notation is only in direction > type->array. The working with symbols looks more difficult, than using words > (in design area). > > More - the textual form is more near to our system of polymorphics types: > anyelement, anyarray, ... We have not anyelement[] True, but this is hardly a straightforward extension of what we have today either. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2016-01-18 22:48 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> I know that Oracle uses syntax of this general type, but I've always
>> found it ugly. It's also pretty non-extensible. You could want
>> similar things for range types and any other container types we might
>> get in the future, but clearly adding new reserved words for each one
>> is no good.
>
> It doesn't use reserved worlds.
OK - keywords, then.
>> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
>> then you want to make BAR an array of that type rather than a scalar,
>> why not write that as DECLARE BAR FOO%TYPE[]? That seems quite
>> natural to me.
>
> what you propose for syntax for taking a element of array?
No idea.
the syntax for "array from" is natural, but for any other is hard. So it is reason, why I used text form. Using Oracle's pattern source%operation allows to use nonreserved keywords. Probably any text can be there. The keywords isn't necessary (not tested).
>> I think the part of this patch that makes %TYPE work for more kinds of
>> types is probably a good idea, although I haven't carefully studied
>> exactly what it does.
>
>
> I invite any ideas, but currently used notation is only in direction
> type->array. The working with symbols looks more difficult, than using words
> (in design area).
>
> More - the textual form is more near to our system of polymorphics types:
> anyelement, anyarray, ... We have not anyelement[]
True, but this is hardly a straightforward extension of what we have
today either.
It is, but sometime the polymorphic types can help.
The proposed feature/syntax has sense primary for polymorphic types. It should to follow our polymorphic types. The primary pair is "anyarray","anyelement" -> "arraytype","elemementtype".
If you don't use polymorphic parameters in plpgsql, then proposed feature can look like useless.
Regards
Pavel
On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > It is, but sometime the polymorphic types can help. > > The proposed feature/syntax has sense primary for polymorphic types. It > should to follow our polymorphic types. The primary pair is > "anyarray","anyelement" -> "arraytype","elemementtype". > > If you don't use polymorphic parameters in plpgsql, then proposed feature > can look like useless. I don't think it's useless, but I do think the syntax is ugly. Maybe it's the best we can do and we should just live with it, but Alvaro asked for opinions, so there's mine. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2016-01-20 0:34 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> It is, but sometime the polymorphic types can help.
>
> The proposed feature/syntax has sense primary for polymorphic types. It
> should to follow our polymorphic types. The primary pair is
> "anyarray","anyelement" -> "arraytype","elemementtype".
>
> If you don't use polymorphic parameters in plpgsql, then proposed feature
> can look like useless.
I don't think it's useless, but I do think the syntax is ugly. Maybe
it's the best we can do and we should just live with it, but Alvaro
asked for opinions, so there's mine.
ok
5 years ago, maybe more - I proposed more nice syntax - and it was rejected as too complex (reserved worlds was required). So this solution try to attack it from different side. It is simple and effective.
Regards
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2016-01-18 21:37 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
> diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
> new file mode 100644
> index 1ae4bb7..c819517
> *** a/src/pl/plpgsql/src/pl_comp.c
> --- b/src/pl/plpgsql/src/pl_comp.c
> *************** plpgsql_parse_tripword(char *word1, char
> *** 1617,1622 ****
> --- 1617,1677 ----
> return false;
> }
>
> + /*
> + * Derive type from ny base type controlled by reftype_mode
> + */
> + static PLpgSQL_type *
> + derive_type(PLpgSQL_type *base_type, int reftype_mode)
> + {
> + Oid typoid;
I think you should add a typedef to the REFTYPE enum, and have this
function take that type rather than int.
done
> + case PLPGSQL_REFTYPE_ARRAY:
> + {
> + /*
> + * Question: can we allow anyelement (array or nonarray) -> array direction.
> + * if yes, then probably we have to modify enforce_generic_type_consistency,
> + * parse_coerce.c where still is check on scalar type -> raise error
> + * ERROR: 42704: could not find array type for data type integer[]
> + *
> + if (OidIsValid(get_element_type(base_type->typoid)))
> + return base_type;
> + */
I think it would be better to resolve this question outside a code
comment.
done
> + typoid = get_array_type(base_type->typoid);
> + if (!OidIsValid(typoid))
> + ereport(ERROR,
> + (errcode(ERRCODE_DATATYPE_MISMATCH),
> + errmsg("there are not array type for type %s",
> + format_type_be(base_type->typoid))));
nodeFuncs.c uses this wording:
errmsg("could not find array type for data type %s",
which I think you should adopt.
sure, fixed
> --- 1681,1687 ----
> * ----------
> */
> PLpgSQL_type *
> ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
> {
> PLpgSQL_type *dtype;
> PLpgSQL_nsitem *nse;
Use the typedef'ed enum, as above.
> --- 1699,1721 ----
> switch (nse->itemtype)
> {
> case PLPGSQL_NSTYPE_VAR:
> ! {
> ! dtype = ((PLpgSQL_var *) (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>
> ! case PLPGSQL_NSTYPE_ROW:
> ! {
> ! dtype = ((PLpgSQL_row *) (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>
> + /*
> + * XXX perhaps allow REC here? Probably it has not any sense, because
> + * in this moment, because PLpgSQL doesn't support rec parameters, so
> + * there should not be any rec polymorphic parameter, and any work can
> + * be done inside function.
> + */
I think you should remove from the "?" onwards in that comment, i.e.
just keep what was already in the original comment (minus the ROW)
I tried to fix it, not sure if understood well.
> --- 1757,1763 ----
> * ----------
> */
> PLpgSQL_type *
> ! plpgsql_parse_cwordtype(List *idents, int reftype_mode)
> {
> PLpgSQL_type *dtype = NULL;
> PLpgSQL_nsitem *nse;
Typedef.
> --- 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;
> }
This plpgsql parser stuff is pretty tiresome. (Not this patch's fault
-- just saying.)
> *************** extern bool plpgsql_parse_dblword(char *
> *** 961,968 ****
> PLwdatum *wdatum, PLcword *cword);
> extern bool plpgsql_parse_tripword(char *word1, char *word2, char *word3,
> PLwdatum *wdatum, PLcword *cword);
> ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident);
> ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents);
> extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
> --- 973,980 ----
> PLwdatum *wdatum, PLcword *cword);
> extern bool plpgsql_parse_tripword(char *word1, char *word2, char *word3,
> PLwdatum *wdatum, PLcword *cword);
> ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident, int reftype_mode);
> ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents, int reftype_mode);
> extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
By the way, these functions are misnamed after this patch. They are
called "wordtype" and "cwordtype" originally because they accept
"word%TYPE" and "compositeword%TYPE", but after the patch they not only
accept TYPE at the right of the percent sign but also ELEMENTTYPE and
ARRAYTYPE. Not sure that this is something we want to be too strict
about.
Understand - used name ***reftype instead ****type
> *** a/src/test/regress/expected/plpgsql.out
> --- b/src/test/regress/expected/plpgsql.out
> *************** end;
> *** 5573,5575 ****
> --- 5573,5667 ----
I think you should also add your array_init() example to the test set.
done
Thank you for your comment
Attached updated patch
Regards
Pavel
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
It seems all fixes are done. I tested the patch and regression tests passed.
On 27.01.2016 20:58, Pavel Stehule wrote:
>
>
> > --- 1681,1687 ----
> > * ----------
> > */
> > PLpgSQL_type *
> > ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
> > {
> > PLpgSQL_type *dtype;
> > PLpgSQL_nsitem *nse;
>
> Use the typedef'ed enum, as above.
>
> > --- 1699,1721 ----
> > switch (nse->itemtype)
> > {
> > case PLPGSQL_NSTYPE_VAR:
> > ! {
> > ! dtype = ((PLpgSQL_var *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype,
> reftype_mode);
> > ! }
> >
> > ! case PLPGSQL_NSTYPE_ROW:
> > ! {
> > ! dtype = ((PLpgSQL_row *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype,
> reftype_mode);
> > ! }
> >
> > + /*
> > + * XXX perhaps allow REC here? Probably it
> has not any sense, because
> > + * in this moment, because PLpgSQL doesn't
> support rec parameters, so
> > + * there should not be any rec polymorphic
> parameter, and any work can
> > + * be done inside function.
> > + */
>
> I think you should remove from the "?" onwards in that comment, i.e.
> just keep what was already in the original comment (minus the ROW)
>
>
> I tried to fix it, not sure if understood well.
I think here Alvaro means that you should keep original comment without
the ROW. Like this:
/* XXX perhaps allow REC here? */
>
> > *************** extern bool plpgsql_parse_dblword(char *
> > *** 961,968 ****
> > PLwdatum *wdatum, PLcword
> *cword);
> > extern bool plpgsql_parse_tripword(char *word1, char *word2,
> char *word3,
> > PLwdatum *wdatum,
> PLcword *cword);
> > ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident);
> > ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents);
> > extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> > extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> > extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32
> typmod,
> > --- 973,980 ----
> > PLwdatum *wdatum, PLcword
> *cword);
> > extern bool plpgsql_parse_tripword(char *word1, char *word2,
> char *word3,
> > PLwdatum *wdatum,
> PLcword *cword);
> > ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident, int
> reftype_mode);
> > ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents, int
> reftype_mode);
> > extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
> > extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
> > extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32
> typmod,
>
> By the way, these functions are misnamed after this patch. They are
> called "wordtype" and "cwordtype" originally because they accept
> "word%TYPE" and "compositeword%TYPE", but after the patch they not only
> accept TYPE at the right of the percent sign but also ELEMENTTYPE and
> ARRAYTYPE. Not sure that this is something we want to be too strict
> about.
>
>
> Understand - used name ***reftype instead ****type
I am not sure, but it seems that new names is a little worse. I think
original names are good too. They accept a word and return the
PLpgSQL_type structure.
>
> Thank you for your comment
>
> Attached updated patch
>
> Regards
>
> Pavel
>
>
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
I noticed a little typo in the comment in the derive_type():
/* Return base_type, when it is a array already */
should be:
/* Return base_type, when it is an array already */
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Hi
I am sending updated version - the changes are related to fix comments.2016-02-19 10:41 GMT+01:00 Artur Zakirov <a.zakirov@postgrespro.ru>:
It seems all fixes are done. I tested the patch and regression tests passed.I think here Alvaro means that you should keep original comment without the ROW. Like this:
/* XXX perhaps allow REC here? */
I tried rewording this comment
I am not sure, but it seems that new names is a little worse. I think original names are good too. They accept a word and return the PLpgSQL_type structure.
By the way, these functions are misnamed after this patch. They are
called "wordtype" and "cwordtype" originally because they accept
"word%TYPE" and "compositeword%TYPE", but after the patch they not only
accept TYPE at the right of the percent sign but also ELEMENTTYPE and
ARRAYTYPE. Not sure that this is something we want to be too strict
about.
Understand - used name ***reftype instead ****type
The "TYPE" word in this name was related to syntax %TYPE. And because new syntax allows more constructs, then the change name is correct. I am think. But choosing names is hard work. The new name little bit more strongly show relation to work with referenced types.
I noticed a little typo in the comment in the derive_type():
/* Return base_type, when it is a array already */
should be:
/* Return base_type, when it is an array already */
fixed
Regards
Pavel
Вложения
On 21.02.2016 11:31, Pavel Stehule wrote: > Hi > > I am sending updated version - the changes are related to fix comments. > Great. I am new in reviewing, I think Pavel took into account all comments. This patch is compiled and regression tests are passed. So I change its status to "Ready for Committer". > > > By the way, these functions are misnamed after this patch. > They are > called "wordtype" and "cwordtype" originally because they > accept > "word%TYPE" and "compositeword%TYPE", but after the patch > they not only > accept TYPE at the right of the percent sign but also > ELEMENTTYPE and > ARRAYTYPE. Not sure that this is something we want to be > too strict > about. > > > Understand - used name ***reftype instead ****type > > > I am not sure, but it seems that new names is a little worse. I > think original names are good too. They accept a word and return the > PLpgSQL_type structure. > > > The "TYPE" word in this name was related to syntax %TYPE. And because > new syntax allows more constructs, then the change name is correct. I am > think. But choosing names is hard work. The new name little bit more > strongly show relation to work with referenced types. > Agree. -- Artur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company
Hi
2016-02-24 10:48 GMT+01:00 Artur Zakirov <a.zakirov@postgrespro.ru>:
On 21.02.2016 11:31, Pavel Stehule wrote:Hi
I am sending updated version - the changes are related to fix comments.
Great.
I am new in reviewing, I think Pavel took into account all comments. This patch is compiled and regression tests are passed. So I change its status to "Ready for Committer".
Thank you very much
Regards
Pavel
Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
От
Peter Eisentraut
Дата:
On 1/18/16 4:21 PM, Robert Haas wrote: > One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but > then you want to make BAR an array of that type rather than a scalar, > why not write that as DECLARE BAR FOO%TYPE[]? That seems quite > natural to me. Right, and it's arguably dubious that that doesn't already work. Unfortunately, these % things are just random plpgsql parser hacks, not real types. Maybe this should be done in the main PostgreSQL parser with parameter hooks, if we wanted this feature to be available outside plpgsql as well. > I think the part of this patch that makes %TYPE work for more kinds of > types is probably a good idea, although I haven't carefully studied > exactly what it does. I agree that this should be more general. For instance, this patch would allow you to get the element type of an array-typed variable, but there is no way to get the element type of just another type. If we could do something like DECLARE var ELEMENT OF point; (not necessary that syntax) then DECLARE var ELEMENT OF othervar%TYPE; should just fall into place.
Hi
2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
On 1/18/16 4:21 PM, Robert Haas wrote:
> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> then you want to make BAR an array of that type rather than a scalar,
> why not write that as DECLARE BAR FOO%TYPE[]? That seems quite
> natural to me.
Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types. Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.
I am not fan to propagate this feature outside PLpgSQL - it is possible new dependency between database object, and the cost is higher than benefits.
> I think the part of this patch that makes %TYPE work for more kinds of
> types is probably a good idea, although I haven't carefully studied
> exactly what it does.
I agree that this should be more general. For instance, this patch
would allow you to get the element type of an array-typed variable, but
there is no way to get the element type of just another type. If we
could do something like
DECLARE
var ELEMENT OF point;
isn't it bug? What is sense of this construct? Our other manipulation with a arrays we raise a error, when we try to to take a element from non array value.
Today I did work on this patch and I am able to implement the syntax proposed by you. It is proprietary, but similar to ADA anonymous types.
DECLARE x array() of type
Regards
Pavel
(not necessary that syntax)
then
DECLARE
var ELEMENT OF othervar%TYPE;
should just fall into place.
On 3/2/16 3:52 PM, Pavel Stehule wrote: > Right, and it's arguably dubious that that doesn't already work. > Unfortunately, these % things are just random plpgsql parser hacks, not > real types. Maybe this should be done in the main PostgreSQL parser > with parameter hooks, if we wanted this feature to be available outside > plpgsql as well. > > > I am not fan to propagate this feature outside PLpgSQL - it is possible > new dependency between database object, and the cost is higher than > benefits. I fail to see how it'd be a dependency. I'd expect it to look up the type when you run the command, just like plpgsql does. I think it'd be useful to have. That said, I think that should be a completely separate patch and discussion. Lets at least get it into plpgsql first. As for the array of element/element of array feature; I agree it would be nice, but we're pretty late in the game for that, and I don't see why that couldn't be added later. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
Hi
2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
On 1/18/16 4:21 PM, Robert Haas wrote:
> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> then you want to make BAR an array of that type rather than a scalar,
> why not write that as DECLARE BAR FOO%TYPE[]? That seems quite
> natural to me.
Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types. Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.
> I think the part of this patch that makes %TYPE work for more kinds of
> types is probably a good idea, although I haven't carefully studied
> exactly what it does.
I agree that this should be more general. For instance, this patch
would allow you to get the element type of an array-typed variable, but
there is no way to get the element type of just another type. If we
could do something like
DECLARE
var ELEMENT OF point;
(not necessary that syntax)
then
DECLARE
var ELEMENT OF othervar%TYPE;
should just fall into place.
I am sending update of this patch. The basic concept is same, syntax was changed per your and Robert requirement.
Regards
Pavel
Вложения
Hi
2016-03-03 0:27 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 3/2/16 3:52 PM, Pavel Stehule wrote:Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types. Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.
I am not fan to propagate this feature outside PLpgSQL - it is possible
new dependency between database object, and the cost is higher than
benefits.
I fail to see how it'd be a dependency. I'd expect it to look up the type when you run the command, just like plpgsql does. I think it'd be useful to have.
if we publish this feature to SQL, then somebody can use it in table definition
CREATE TABLE a(a int);
CREATE TABLE b(a a.a%TYPE)
And the people expecting the living relation between table a and table b. So when I do ALTER a.a, then b.a should be changed. What if I drop a.a or drop a?
So this is reason, why I don't would this feature in SQL side.
Regards
Pavel
That said, I think that should be a completely separate patch and discussion. Lets at least get it into plpgsql first.
As for the array of element/element of array feature; I agree it would be nice, but we're pretty late in the game for that, and I don't see why that couldn't be added later.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
On 03/03/2016 05:45 AM, Pavel Stehule wrote: > 2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e@gmx.net > <mailto:peter_e@gmx.net>>: > > On 1/18/16 4:21 PM, Robert Haas wrote: > > One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but > > then you want to make BAR an array of that type rather than a scalar, > > why not write that as DECLARE BAR FOO%TYPE[]? That seems quite > > natural to me. > > Right, and it's arguably dubious that that doesn't already work. > Unfortunately, these % things are just random plpgsql parser hacks, not > real types. Maybe this should be done in the main PostgreSQL parser > with parameter hooks, if we wanted this feature to be available outside > plpgsql as well. > > > I think the part of this patch that makes %TYPE work for more kinds of > > types is probably a good idea, although I haven't carefully studied > > exactly what it does. > > I agree that this should be more general. For instance, this patch > would allow you to get the element type of an array-typed variable, but > there is no way to get the element type of just another type. If we > could do something like > > DECLARE > var ELEMENT OF point; > > (not necessary that syntax) > > then > > DECLARE > var ELEMENT OF othervar%TYPE; > > should just fall into place. > > I am sending update of this patch. The basic concept is same, syntax was > changed per your and Robert requirement. This new version of the patch was posted after the commitfest item was marked ready for committer. Does anyone have further comments or objections to the concept or syntax before I try to take this forward? Thanks, Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
Joe Conway <mail@joeconway.com> writes:
> This new version of the patch was posted after the commitfest item was
> marked ready for committer. Does anyone have further comments or
> objections to the concept or syntax before I try to take this forward?
The quoted excerpt fails to say what solution was adopted to the array
syntax issues, so it's impossible to have an opinion without actually
reading the patch.
However ... one thing I was intending to mention on this thread is that
"get the array type over this type" isn't the only extension one might
wish for. Another likely desire is "get the type of field 'foo' of this
composite type". I don't suggest that this patch needs to implement
that right now; but it would be a good thing if we could see how the
chosen syntax could be extended in such a direction. Otherwise we might
be painting ourselves into a corner.
regards, tom lane
On 14.03.2016 17:54, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> This new version of the patch was posted after the commitfest item was
>> marked ready for committer. Does anyone have further comments or
>> objections to the concept or syntax before I try to take this forward?
>
> The quoted excerpt fails to say what solution was adopted to the array
> syntax issues, so it's impossible to have an opinion without actually
> reading the patch.
>
> However ... one thing I was intending to mention on this thread is that
> "get the array type over this type" isn't the only extension one might
> wish for. Another likely desire is "get the type of field 'foo' of this
> composite type". I don't suggest that this patch needs to implement
> that right now; but it would be a good thing if we could see how the
> chosen syntax could be extended in such a direction. Otherwise we might
> be painting ourselves into a corner.
>
> regards, tom lane
>
I looked this patch and the previous. The patch applies correctly to
HEAD. Regression tests pass successfully, without errors.
In comparison with the previous patch it adds the following functionality:
- %TYPE - now can be used for composite types (same syntax).
- %TYPE[] - new functionality, provides the array type from a
variable or table column (syntax was changed).
- var ELEMENT OF othervar%TYPE - new funcitonality, provides the element
type of a given array (syntax was changed).
Was changed plpgsql_derive_type(). Now it has the following definition:
> PLpgSQL_type *
> plpgsql_derive_type(PLpgSQL_type *base_type, bool to_element_type, bool to_array_type)
Previously it had the following definition:
> static PLpgSQL_type *
> derive_type(PLpgSQL_type *base_type, PLpgSQL_reftype reftype)
where PLpgSQL_reftype was the enum:
> + typedef enum
> + {
> + PLPGSQL_REFTYPE_TYPE, /* use type of some variable */
> + PLPGSQL_REFTYPE_ELEMENT, /* use a element type of referenced variable */
> + PLPGSQL_REFTYPE_ARRAY /* use a array type of referenced variable */
> + } PLpgSQL_reftype;
I think the previous version was better, because enum is better than
additional function parameters. But it is only for me.
Also there is a little typo here:
> + * This routine is used for generating element or array type from base type.
> + * The options to_element_type and to_array_type can be used together, when
> + * we would to ensure valid result. The array array type is original type, so
> + * this direction is safe. The element of scalar type is not allowed, but if
> + * we do "to array" transformation first, then this direction should be safe
> + * too. This design is tolerant, because we should to support a design of
> + * polymorphic parameters, where a array value can be passed as anyelement
> + * or anyarray parameter.
> + */
> + PLpgSQL_type *
> + plpgsql_derive_type(PLpgSQL_type *base_type,
Here the word "array" occurs two times in the third line.
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
I wrote:
> However ... one thing I was intending to mention on this thread is that
> "get the array type over this type" isn't the only extension one might
> wish for. Another likely desire is "get the type of field 'foo' of this
> composite type". I don't suggest that this patch needs to implement
> that right now; but it would be a good thing if we could see how the
> chosen syntax could be extended in such a direction. Otherwise we might
> be painting ourselves into a corner.
To enlarge a little bit: it seems to me that what we're really wishing for
here is a type name syntax that goes beyond simple names. If we were
starting in a green field, we might choose a recursively-composable syntax
like the following.
type_name can be:
* A simple type name, such as int8 or varchar[42].
* TYPE_OF(expression), meaning that the SQL expression is parsed but never
executed, we just take this construct as naming its result type.
* ARRAY_OF(type_name), meaning the array type having type_name as its
element type.
* ELEMENT_OF(type_name), meaning the element type of the array type
named by type_name.
* ROW_OF(type_name [, type_name ...]), meaning the composite type with
those types as columns.
* FIELD_OF(type_name, foo), meaning the type of column "foo" of the
composite type named by type_name. I'm not sure if there would be
use-cases for selecting a column other than by a simple literal name,
but there could be variants of this function if so.
It's possible to think of other cases, for example what about range
types? You could allow ELEMENT_OF() to apply to range types, certainly.
I'm not sure about the other direction, because multiple range types
could have the same element type; but it's possible to hope that some
type-naming function along the lines of RANGE_OF(type_name, other args)
could disambiguate. The main reason I'm thinking of a function-like
syntax here is that it can easily handle additional arguments when
needed.
Comparing this flight of fancy to where we are today, we have
%TYPE as a remarkably ugly and limited implementation of TYPE_OF(),
and we have the precedent that foo[] means ARRAY_OF(foo). I'm not
sure how we get any extensibility out of either of those things.
Or in short: maybe it's time to blow up %TYPE and start fresh.
regards, tom lane
On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: >> However ... one thing I was intending to mention on this thread is that >> "get the array type over this type" isn't the only extension one might >> wish for. Another likely desire is "get the type of field 'foo' of this >> composite type". I don't suggest that this patch needs to implement >> that right now; but it would be a good thing if we could see how the >> chosen syntax could be extended in such a direction. Otherwise we might >> be painting ourselves into a corner. > > To enlarge a little bit: it seems to me that what we're really wishing for > here is a type name syntax that goes beyond simple names. If we were > starting in a green field, we might choose a recursively-composable syntax > like the following. > > type_name can be: > > * A simple type name, such as int8 or varchar[42]. > > * TYPE_OF(expression), meaning that the SQL expression is parsed but never > executed, we just take this construct as naming its result type. > > * ARRAY_OF(type_name), meaning the array type having type_name as its > element type. > > * ELEMENT_OF(type_name), meaning the element type of the array type > named by type_name. > > * ROW_OF(type_name [, type_name ...]), meaning the composite type with > those types as columns. > > * FIELD_OF(type_name, foo), meaning the type of column "foo" of the > composite type named by type_name. I'm not sure if there would be > use-cases for selecting a column other than by a simple literal name, > but there could be variants of this function if so. > > It's possible to think of other cases, for example what about range > types? You could allow ELEMENT_OF() to apply to range types, certainly. > I'm not sure about the other direction, because multiple range types > could have the same element type; but it's possible to hope that some > type-naming function along the lines of RANGE_OF(type_name, other args) > could disambiguate. The main reason I'm thinking of a function-like > syntax here is that it can easily handle additional arguments when > needed. > > Comparing this flight of fancy to where we are today, we have > %TYPE as a remarkably ugly and limited implementation of TYPE_OF(), > and we have the precedent that foo[] means ARRAY_OF(foo). I'm not > sure how we get any extensibility out of either of those things. > > Or in short: maybe it's time to blow up %TYPE and start fresh. That's not a dumb idea. I think %TYPE is an Oracle-ism, and it doesn't seem to have been their best-ever design decision. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Or in short: maybe it's time to blow up %TYPE and start fresh.
> That's not a dumb idea. I think %TYPE is an Oracle-ism, and it
> doesn't seem to have been their best-ever design decision.
It is, and it wasn't. What concerns me about the present patch is
that it's trying to shoehorn more functionality into something that
was badly designed to start with. I think we'd be better off leaving
%TYPE as a deprecated backwards-compatibility feature and inventing
something new and more extensible.
I'm not wedded to any part of the syntax I just wrote, but I do say
that we need something that allows composability of type selectors.
regards, tom lane
2016-03-14 20:38 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Or in short: maybe it's time to blow up %TYPE and start fresh.
> That's not a dumb idea. I think %TYPE is an Oracle-ism, and it
> doesn't seem to have been their best-ever design decision.
Using %TYPE has sense in PostgreSQL too. It is protection against a domain's explosion - I don't need to declare the domains like varchar_10, varchar_100, etc. It does more work in Oracle due living relation between table columns and PL/SQL variables - but this differences are same for domain types in PL/pgSQL. What is redundant in Postgres, is using %TYPE and %ROWTYPE. Because native PL languages has strong relation to system catalog I see this feature like well designed and helpful. Some different can be our implementation.
It is, and it wasn't. What concerns me about the present patch is
that it's trying to shoehorn more functionality into something that
was badly designed to start with. I think we'd be better off leaving
%TYPE as a deprecated backwards-compatibility feature and inventing
something new and more extensible.
I'm not wedded to any part of the syntax I just wrote, but I do say
that we need something that allows composability of type selectors.
Last version of this patch doesn't modify %TYPE part - and the implemented syntax is near to your proposed (not all cases), and near to ADA syntax. But, probably, you are unhappy with it.
Can you describe your expectations from this feature little bit more, please?
We can leave the original discussion about %TYPE. It was my mistake. I push two different ingredients to one soup.
We can leave the original discussion about %TYPE. It was my mistake. I push two different ingredients to one soup.
There are two independent features - referenced types - the original %TYPE and %ROWTYPE features, the possibility to take type from system catalog information.
Last patch implements linear ELEMENT OF ... , ARRAY OF ... . Can be solution recursive enhancing of last patch? We can implement other type modificator like RANGE OF (any other?)Then we can write some like
ARRAY OF RANGE OF int; or ELEMENT OF ELEMENT OF array_of_ranges
or if we use functional syntax
ARRAY_OF(RANGE_OF(int))
I prefer non functional syntax - it looks more natural in DECLARE part, but it is detail in this moment. I can live with functional syntax too. The functional syntax is easy parserable, but the SQL is not functional - so I see it foreign there.
Where you are expecting the implementation? In PLpgSQL only, or generally in DDL, or in both levels?
Regards
Pavel
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
> Where you are expecting the implementation? In PLpgSQL only, or generally
> in DDL, or in both levels?
I'd envision this as something the main parser does and plpgsql piggybacks
on. One of the many half-baked things about %TYPE is that the main parser
has an implementation, and plpgsql has its own not-entirely-compatible
one. (And one thing I especially don't like about the submitted patch is
that it makes those two diverge even further.)
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> That's not a dumb idea. I think %TYPE is an Oracle-ism, and it
>>> doesn't seem to have been their best-ever design decision.
> Using %TYPE has sense in PostgreSQL too.
It's certainly useful functionality; the question is whether this
particular syntax is an appropriate base for extended features.
As I see it, what we're talking about here could be called type operators:
given a type name or some other kind of SQL expression, produce the name
of a related type. The existing things of that sort are %TYPE and []
(we don't really implement [] as a type operator, but a user could
reasonably think of it as one). This patch proposes to make %TYPE and []
composable into a single operator, and then it proposes to add ELEMENT OF
as a different operator; and these things are only implemented in plpgsql.
My concern is basically that I don't want to stop there. I think we want
more type operators in future, such as the rowtype-related operators
I sketched upthread; and I think we will want these operators anywhere
that you can write a type name.
Now, in the core grammar we have [] which can be attached to any type
name, and we have %TYPE but it only works in very limited contexts.
There's a fundamental problem with extending %TYPE to be used anywhere
a type name can: consider
select 'foo'::x%type from t;
It's ambiguous whether this is an invocation of %TYPE syntax or whether %
is meant to be a regular operator and TYPE the name of a variable. Now,
we could remove that ambiguity by promoting TYPE to be a fully reserved
word (it is unreserved today). But that's not very palatable, and even
if we did reserve TYPE, I think we'd still need a lexer kluge to convert
%TYPE into a single token, else bison will have lookahead problems.
That sort of kluge is ugly, costs performance, and tends to have
unforeseen side-effects.
So my opinion is that rather than extending %TYPE, we need a new syntax
that is capable of being used in more general contexts.
There's another problem with the proposal as given: it adds a prefix
type operator (ELEMENT OF) where before we only had postfix ones.
That means there's an ambiguity about which one binds tighter. This is
not a big deal right now, since there'd be little point in combining
ELEMENT OF and [] in the same operation, but it's going to create a mess
when we try to add additional type operators. You're going to need to
allow parentheses to control binding order. I also find it unsightly
that the prefix operator looks so little like the postfix operators
syntactically, even though they do very similar sorts of things.
In short there basically isn't much to like about these syntax details.
I also do not like adding the feature to plpgsql first. At best, that's
going to be code we throw away when we implement the same functionality
in the core's typename parser. At worst, we'll have a permanent
incompatibility because we find we can't make the core parser use exactly
the same syntax. (For example, it's possible we'd find out we have to
make ELEMENT a fully-reserved word in order to use this ELEMENT OF syntax.
Or maybe it's fine; but until we've tried to cram it into the Typename
production, we won't know. I'm a bit suspicious of expecting it to be
fine, though, since AFAICS this patch breaks the ability to use "element"
as a plain type name in a plpgsql variable declaration. Handwritten
parsing code like this tends to be full of such gotchas.)
In short, I think we should reject this implementation and instead try
to implement the type operators we want in the core grammar's Typename
production, from which plpgsql will pick it up automatically. That is
going to require some other syntax than this. As I said, I'm not
particularly pushing the function-like syntax I wrote upthread; but
I want to see something that is capable of supporting all those features
and can be extended later if we think of other type operators we want.
regards, tom lane
On 03/15/2016 05:17 PM, Tom Lane wrote: > In short, I think we should reject this implementation and instead try > to implement the type operators we want in the core grammar's Typename > production, from which plpgsql will pick it up automatically. That is > going to require some other syntax than this. As I said, I'm not > particularly pushing the function-like syntax I wrote upthread; but > I want to see something that is capable of supporting all those features > and can be extended later if we think of other type operators we want. +1 Anyone want to argue against changing the status of this to Rejected or at least Returned with feedback? Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
2016-03-16 16:46 GMT+01:00 Joe Conway <mail@joeconway.com>:
On 03/15/2016 05:17 PM, Tom Lane wrote:
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically. That is
> going to require some other syntax than this. As I said, I'm not
> particularly pushing the function-like syntax I wrote upthread; but
> I want to see something that is capable of supporting all those features
> and can be extended later if we think of other type operators we want.
+1
Anyone want to argue against changing the status of this to Rejected or
at least Returned with feedback?
I would to reduce this patch to fix row type issue. There is not any disagreement. I'll send reduced patch today.
Any other functionality is not 9.6 topic.
Regards
Pavel
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
2016-03-16 16:50 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
2016-03-16 16:46 GMT+01:00 Joe Conway <mail@joeconway.com>:On 03/15/2016 05:17 PM, Tom Lane wrote:
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically. That is
> going to require some other syntax than this. As I said, I'm not
> particularly pushing the function-like syntax I wrote upthread; but
> I want to see something that is capable of supporting all those features
> and can be extended later if we think of other type operators we want.
+1
Anyone want to argue against changing the status of this to Rejected or
at least Returned with feedback?I would to reduce this patch to fix row type issue. There is not any disagreement. I'll send reduced patch today.Any other functionality is not 9.6 topic.
I played with the reduced patch, and the benefit without all other things is negligible. It should be rejected.
Regards
Pavel
RegardsPavel
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
On 03/16/2016 09:38 AM, Pavel Stehule wrote: > 2016-03-16 16:50 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com > <mailto:pavel.stehule@gmail.com>>: > 2016-03-16 16:46 GMT+01:00 Joe Conway <mail@joeconway.com > <mailto:mail@joeconway.com>>: > > On 03/15/2016 05:17 PM, Tom Lane wrote: > > In short, I think we should reject this implementation and instead try > > to implement the type operators we want in the core grammar's Typename > > production, from which plpgsql will pick it up automatically. That is > > going to require some other syntax than this. As I said, I'm not > > particularly pushing the function-like syntax I wrote upthread; but > > I want to see something that is capable of supporting all those features > > and can be extended later if we think of other type operators we want. > > +1 > > Anyone want to argue against changing the status of this to > Rejected or > at least Returned with feedback? > > > I would to reduce this patch to fix row type issue. There is not any > disagreement. I'll send reduced patch today. > > Any other functionality is not 9.6 topic. > > I played with the reduced patch, and the benefit without all other > things is negligible. It should be rejected. Ok, thanks -- done. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
On 3/3/16 4:51 AM, Pavel Stehule wrote: > CREATE TABLE a(a int); > CREATE TABLE b(a a.a%TYPE) > > And the people expecting the living relation between table a and table > b. So when I do ALTER a.a, then b.a should be changed. What if I drop > a.a or drop a? > > So this is reason, why I don't would this feature in SQL side. I don't buy that. plpgsql doesn't work that way, so why would this? *especially* with the %TYPE decorator. Now, if the syntax was CREATE TABLE b(a a.a) then I would expect b.a to be a foreign key reference to a. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
On 3/15/16 7:17 PM, Tom Lane wrote:
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically.
+1.
Something else that's been discussed is allowing [] referencing to be
more modular. Offhand I don't see how that would impact this new type
referencing stuff, but maybe someone else sees an issue.
BTW, it might also be useful to allow {} to work as a reference method.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
2016-03-16 20:53 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 3/3/16 4:51 AM, Pavel Stehule wrote:CREATE TABLE a(a int);
CREATE TABLE b(a a.a%TYPE)
And the people expecting the living relation between table a and table
b. So when I do ALTER a.a, then b.a should be changed. What if I drop
a.a or drop a?
So this is reason, why I don't would this feature in SQL side.
I don't buy that. plpgsql doesn't work that way, so why would this? *especially* with the %TYPE decorator.
Now, if the syntax was
CREATE TABLE b(a a.a)
then I would expect b.a to be a foreign key reference to a.
probably we are talking about different situations. It is not important, because this patch was rejected.
Regards
Pavel
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 3/3/16 4:51 AM, Pavel Stehule wrote:
>> CREATE TABLE a(a int);
>> CREATE TABLE b(a a.a%TYPE)
>>
>> And the people expecting the living relation between table a and table
>> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> a.a or drop a?
>>
>> So this is reason, why I don't would this feature in SQL side.
> I don't buy that. plpgsql doesn't work that way, so why would this?
> *especially* with the %TYPE decorator.
Yeah. The %TYPE decorator doesn't work like that in the core parser
either: when you use it, the referenced type is determined immediately
and then it's just as if you'd written that type name to begin with.
I do not see a reason for any of these "type operators" to work
differently.
Another analogy that might help make the point is
set search_path = a;create table myschema.tab(f1 mytype);set search_path = b;
If there are types "mytype" in both schemas a and b, is myschema.tab.f1
now of type b.mytype? No. The meaning of the type reference is
determined when the command executes, and then you're done.
regards, tom lane
Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
От
"David G. Johnston"
Дата:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 3/3/16 4:51 AM, Pavel Stehule wrote:
>> CREATE TABLE a(a int);
>> CREATE TABLE b(a a.a%TYPE)
>>
>> And the people expecting the living relation between table a and table
>> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> a.a or drop a?
>>
>> So this is reason, why I don't would this feature in SQL side.
> I don't buy that. plpgsql doesn't work that way, so why would this?
> *especially* with the %TYPE decorator.
Yeah. The %TYPE decorator doesn't work like that in the core parser
either: when you use it, the referenced type is determined immediately
and then it's just as if you'd written that type name to begin with.
I'm missing something here...%TYPE ends up getting parsed repeatedly and so appears to be change if the variable upon which it is based changes - even if once parsed it remains constant for the lifetime of the function's evaluation.
I guess what is being said is that the "constant" behavior in SQL ends up being permanent because a given statement is only ever conceptually parsed and executed a single time - unlike a function body. The nature of any solution would still have the same characteristics within a function because the inherent re-parsing nature and not because of any direct capability of %TYPE itself.
I do not see a reason for any of these "type operators" to work
differently.
Another analogy that might help make the point is
set search_path = a;
create table myschema.tab(f1 mytype);
set search_path = b;
If there are types "mytype" in both schemas a and b, is myschema.tab.f1
now of type b.mytype? No. The meaning of the type reference is
determined when the command executes, and then you're done.
And its no different than our treatment of "*"
CREATE VIEW test_view
SELECT *
FROM temp_table;
Adding columns to temp_table doesn't impact which columns the view returns.
David J.
<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">2016-03-17 0:39 GMT+01:00 Tom Lane <span dir="ltr"><<ahref="mailto:tgl@sss.pgh.pa.us" target="_blank">tgl@sss.pgh.pa.us</a>></span>:<br /><blockquote class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Jim Nasby <Jim.Nasby@BlueTreble.com>writes:<br /> > On 3/3/16 4:51 AM, Pavel Stehule wrote:<br /> >> CREATE TABLE a(aint);<br /> >> CREATE TABLE b(a a.a%TYPE)<br /> >><br /> >> And the people expecting the living relationbetween table a and table<br /> >> b. So when I do ALTER a.a, then b.a should be changed. What if I drop<br/> >> a.a or drop a?<br /> >><br /> >> So this is reason, why I don't would this feature in SQLside.<br /><br /> > I don't buy that. plpgsql doesn't work that way, so why would this?<br /> > *especially* withthe %TYPE decorator.<br /><br /></span>Yeah. The %TYPE decorator doesn't work like that in the core parser<br /> either:when you use it, the referenced type is determined immediately<br /> and then it's just as if you'd written that typename to begin with.<br /> I do not see a reason for any of these "type operators" to work<br /> differently.<br /><br/> Another analogy that might help make the point is<br /><br /> set search_path = a;<br /> create tablemyschema.tab(f1 mytype);<br /> set search_path = b;<br /><br /> If there are types "mytype" in both schemasa and b, is myschema.tab.f1<br /> now of type b.mytype? No. The meaning of the type reference is<br /> determinedwhen the command executes, and then you're done.<br /></blockquote><br />This is valid for PostgreSQL. I am notsure if it is true in Oracle, if %TYPE means only reference to type, or %TYPE holds reference to original object - andwhen you change the original object, then the function is invalidated.<br /><br /></div><div class="gmail_quote">Using%TYPE with create time only semantic has not big practical benefit. But when %TYPE enforce all lifedependency, then I have guaranteed so change on original object will be propagated to depend object. With all advantagesand disadvantages.<br /><br /></div><div class="gmail_quote">Postgres uses %TYPE in create time only semantic -but it is not big issue in PLpgSQL, because the creation time there is often - every first execution in session.<br /><br/></div><div class="gmail_quote">The usage of %TYPE outer PL/pgSQL is probably only in FK. But nothing similar is instandard, and I don't see a reason, why we should to implement it. In this moment I don't see any important use case.<br/></div><div class="gmail_quote"><br /></div><div class="gmail_quote">Pavel<br /> <br /></div><div class="gmail_quote"><blockquoteclass="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br/> regards, tom lane<br /></blockquote></div><br /></div></div>
2016-03-17 1:02 GMT+01:00 David G. Johnston <david.g.johnston@gmail.com>:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 3/3/16 4:51 AM, Pavel Stehule wrote:
>> CREATE TABLE a(a int);
>> CREATE TABLE b(a a.a%TYPE)
>>
>> And the people expecting the living relation between table a and table
>> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> a.a or drop a?
>>
>> So this is reason, why I don't would this feature in SQL side.
> I don't buy that. plpgsql doesn't work that way, so why would this?
> *especially* with the %TYPE decorator.
Yeah. The %TYPE decorator doesn't work like that in the core parser
either: when you use it, the referenced type is determined immediately
and then it's just as if you'd written that type name to begin with.I'm missing something here...%TYPE ends up getting parsed repeatedly and so appears to be change if the variable upon which it is based changes - even if once parsed it remains constant for the lifetime of the function's evaluation.I guess what is being said is that the "constant" behavior in SQL ends up being permanent because a given statement is only ever conceptually parsed and executed a single time - unlike a function body. The nature of any solution would still have the same characteristics within a function because the inherent re-parsing nature and not because of any direct capability of %TYPE itself.I do not see a reason for any of these "type operators" to work
differently.
Another analogy that might help make the point is
set search_path = a;
create table myschema.tab(f1 mytype);
set search_path = b;
If there are types "mytype" in both schemas a and b, is myschema.tab.f1
now of type b.mytype? No. The meaning of the type reference is
determined when the command executes, and then you're done.And its no different than our treatment of "*"CREATE VIEW test_viewSELECT *FROM temp_table;Adding columns to temp_table doesn't impact which columns the view returns.
yes, but there is strong limit. You can append column, but you cannot to alter existing column.
Pavel
David J.
Good summary. Is there a TODO item here? --------------------------------------------------------------------------- On Tue, Mar 15, 2016 at 08:17:07PM -0400, Tom Lane wrote: > Pavel Stehule <pavel.stehule@gmail.com> writes: > >> Robert Haas <robertmhaas@gmail.com> writes: > >>> That's not a dumb idea. I think %TYPE is an Oracle-ism, and it > >>> doesn't seem to have been their best-ever design decision. > > > Using %TYPE has sense in PostgreSQL too. > > It's certainly useful functionality; the question is whether this > particular syntax is an appropriate base for extended features. > > As I see it, what we're talking about here could be called type operators: > given a type name or some other kind of SQL expression, produce the name > of a related type. The existing things of that sort are %TYPE and [] > (we don't really implement [] as a type operator, but a user could > reasonably think of it as one). This patch proposes to make %TYPE and [] > composable into a single operator, and then it proposes to add ELEMENT OF > as a different operator; and these things are only implemented in plpgsql. > > My concern is basically that I don't want to stop there. I think we want > more type operators in future, such as the rowtype-related operators > I sketched upthread; and I think we will want these operators anywhere > that you can write a type name. > > Now, in the core grammar we have [] which can be attached to any type > name, and we have %TYPE but it only works in very limited contexts. > There's a fundamental problem with extending %TYPE to be used anywhere > a type name can: consider > > select 'foo'::x%type from t; > > It's ambiguous whether this is an invocation of %TYPE syntax or whether % > is meant to be a regular operator and TYPE the name of a variable. Now, > we could remove that ambiguity by promoting TYPE to be a fully reserved > word (it is unreserved today). But that's not very palatable, and even > if we did reserve TYPE, I think we'd still need a lexer kluge to convert > %TYPE into a single token, else bison will have lookahead problems. > That sort of kluge is ugly, costs performance, and tends to have > unforeseen side-effects. > > So my opinion is that rather than extending %TYPE, we need a new syntax > that is capable of being used in more general contexts. > > There's another problem with the proposal as given: it adds a prefix > type operator (ELEMENT OF) where before we only had postfix ones. > That means there's an ambiguity about which one binds tighter. This is > not a big deal right now, since there'd be little point in combining > ELEMENT OF and [] in the same operation, but it's going to create a mess > when we try to add additional type operators. You're going to need to > allow parentheses to control binding order. I also find it unsightly > that the prefix operator looks so little like the postfix operators > syntactically, even though they do very similar sorts of things. > > In short there basically isn't much to like about these syntax details. > > I also do not like adding the feature to plpgsql first. At best, that's > going to be code we throw away when we implement the same functionality > in the core's typename parser. At worst, we'll have a permanent > incompatibility because we find we can't make the core parser use exactly > the same syntax. (For example, it's possible we'd find out we have to > make ELEMENT a fully-reserved word in order to use this ELEMENT OF syntax. > Or maybe it's fine; but until we've tried to cram it into the Typename > production, we won't know. I'm a bit suspicious of expecting it to be > fine, though, since AFAICS this patch breaks the ability to use "element" > as a plain type name in a plpgsql variable declaration. Handwritten > parsing code like this tends to be full of such gotchas.) > > In short, I think we should reject this implementation and instead try > to implement the type operators we want in the core grammar's Typename > production, from which plpgsql will pick it up automatically. That is > going to require some other syntax than this. As I said, I'm not > particularly pushing the function-like syntax I wrote upthread; but > I want to see something that is capable of supporting all those features > and can be extended later if we think of other type operators we want. > > regards, tom lane > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
2016-04-25 19:40 GMT+02:00 Bruce Momjian <bruce@momjian.us>:
Good summary. Is there a TODO item here?
no, it is not
Regars
Pavel
---------------------------------------------------------------------------
On Tue, Mar 15, 2016 at 08:17:07PM -0400, Tom Lane wrote:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
> >> Robert Haas <robertmhaas@gmail.com> writes:
> >>> That's not a dumb idea. I think %TYPE is an Oracle-ism, and it
> >>> doesn't seem to have been their best-ever design decision.
>
> > Using %TYPE has sense in PostgreSQL too.
>
> It's certainly useful functionality; the question is whether this
> particular syntax is an appropriate base for extended features.
>
> As I see it, what we're talking about here could be called type operators:
> given a type name or some other kind of SQL expression, produce the name
> of a related type. The existing things of that sort are %TYPE and []
> (we don't really implement [] as a type operator, but a user could
> reasonably think of it as one). This patch proposes to make %TYPE and []
> composable into a single operator, and then it proposes to add ELEMENT OF
> as a different operator; and these things are only implemented in plpgsql.
>
> My concern is basically that I don't want to stop there. I think we want
> more type operators in future, such as the rowtype-related operators
> I sketched upthread; and I think we will want these operators anywhere
> that you can write a type name.
>
> Now, in the core grammar we have [] which can be attached to any type
> name, and we have %TYPE but it only works in very limited contexts.
> There's a fundamental problem with extending %TYPE to be used anywhere
> a type name can: consider
>
> select 'foo'::x%type from t;
>
> It's ambiguous whether this is an invocation of %TYPE syntax or whether %
> is meant to be a regular operator and TYPE the name of a variable. Now,
> we could remove that ambiguity by promoting TYPE to be a fully reserved
> word (it is unreserved today). But that's not very palatable, and even
> if we did reserve TYPE, I think we'd still need a lexer kluge to convert
> %TYPE into a single token, else bison will have lookahead problems.
> That sort of kluge is ugly, costs performance, and tends to have
> unforeseen side-effects.
>
> So my opinion is that rather than extending %TYPE, we need a new syntax
> that is capable of being used in more general contexts.
>
> There's another problem with the proposal as given: it adds a prefix
> type operator (ELEMENT OF) where before we only had postfix ones.
> That means there's an ambiguity about which one binds tighter. This is
> not a big deal right now, since there'd be little point in combining
> ELEMENT OF and [] in the same operation, but it's going to create a mess
> when we try to add additional type operators. You're going to need to
> allow parentheses to control binding order. I also find it unsightly
> that the prefix operator looks so little like the postfix operators
> syntactically, even though they do very similar sorts of things.
>
> In short there basically isn't much to like about these syntax details.
>
> I also do not like adding the feature to plpgsql first. At best, that's
> going to be code we throw away when we implement the same functionality
> in the core's typename parser. At worst, we'll have a permanent
> incompatibility because we find we can't make the core parser use exactly
> the same syntax. (For example, it's possible we'd find out we have to
> make ELEMENT a fully-reserved word in order to use this ELEMENT OF syntax.
> Or maybe it's fine; but until we've tried to cram it into the Typename
> production, we won't know. I'm a bit suspicious of expecting it to be
> fine, though, since AFAICS this patch breaks the ability to use "element"
> as a plain type name in a plpgsql variable declaration. Handwritten
> parsing code like this tends to be full of such gotchas.)
>
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically. That is
> going to require some other syntax than this. As I said, I'm not
> particularly pushing the function-like syntax I wrote upthread; but
> I want to see something that is capable of supporting all those features
> and can be extended later if we think of other type operators we want.
>
> regards, tom lane
>
>--> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +