Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

Lists: pgsql-hackers
From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-10-19 07:52:28
Message-ID: CAFj8pRCQREdqf_Oov0-8xENUf-VxyQRptgAAat0rPNU7B+BHRA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-10-30 11:01:13
Message-ID: CAFj8pRCZivm3XsoqWTdRO0a9emJognoPL=mNSW7jCeOMcf67xw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2015-10-19 9:52 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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 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?
>
>
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)

Regards

Pavel

> Regards
>
> Pavel
>
>
>

Attachment Content-Type Size
plpgsql-polymorphic-row-var.patch text/x-patch 13.9 KB

From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-21 00:06:51
Message-ID: 5677429B.4090105@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-21 05:51:43
Message-ID: CAFj8pRDvGyU-E1n7Nkbe0aewCjaZpcYqpQGjQtJEv3kX8Bw7hg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2015-12-21 1:06 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)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%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
>


From: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-21 15:21:08
Message-ID: 567818E4.3030205@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

Attachment Content-Type Size
plpgsql-polymorphic-row-var-2.patch text/x-patch 7.6 KB

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-21 21:42:43
Message-ID: CAFj8pRA9nLzFq9upEQ6XQoZL_SBvoJsDPokDVvDuP37ULekTNQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2015-12-21 16:21 GMT+01:00 Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)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
>


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-22 08:59:07
Message-ID: CAFj8pRDA_qZT+ZBaBKbbtnwqPBdgmX_Jx-4voeH7QoUiYdgW3Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2015-12-21 16:21 GMT+01:00 Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)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.

Regards

Pavel

>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>

Attachment Content-Type Size
plpgsql-ref-element-array-type.patch text/x-patch 19.8 KB

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2015-12-24 03:02:58
Message-ID: CAB7nPqTH5Yw2P1Bpq4y_B95KxeN-xbMv4BDJkQ1fgmvKKGp42Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Dec 22, 2015 at 5:59 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hi
>
> 2015-12-21 16:21 GMT+01:00 Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)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


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-18 20:37:33
Message-ID: 20160118203733.GA123075@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> 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


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-18 20:51:47
Message-ID: 20160118205147.GA129226@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-18 21:21:35
Message-ID: CA+TgmobqT_wn_m+2-hBc9Tu2-CM3BeBE5394LCHTs2R15jyi+A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-18 21:35:41
Message-ID: CAFj8pRAzQyQgnecbEA_2pQFeCN7itj-yUuDJf_e7b=caDds8Zw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-01-18 22:21 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:

> On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)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
>


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-18 21:48:00
Message-ID: CA+TgmoZ0BYP9+YXk6rBfSoAWSqSwsyd_j7NKVpMMYeJtJ2S2Qg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-19 09:53:06
Message-ID: CAFj8pRDfyokA6XuScMVeCGMVT4WLjaRdaEuLcYgObqTiEFn5Ew@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-01-18 22:48 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:

> On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-19 23:34:14
Message-ID: CA+TgmoY9mCGVwBy+_K1XGwob=fpUWsN7T9PfAjU6s=UPxKZ-UA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-20 05:30:21
Message-ID: CAFj8pRDubAdsGPr6YtCb92hFfqOhMOtmF5eLVhMbyOKVOR0DLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-01-20 0:34 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:

> On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-01-27 17:58:11
Message-ID: CAFj8pRBvcS3fPN1_bUcaEupyzPHne6cb+ryt+JY-dGOTkuNexg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2016-01-18 21:37 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)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
>

Attachment Content-Type Size
plpgsql-ref-element-array-type-02.patch text/x-patch 21.3 KB

From: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-02-19 09:41:43
Message-ID: 56C6E357.4080604@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-02-21 08:31:14
Message-ID: CAFj8pRBDoXuqrTg-6qCOWECJmKDKBzHOMgoE868TZxqVGDoAXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

I am sending updated version - the changes are related to fix comments.

2016-02-19 10:41 GMT+01:00 Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)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

>
>
>
>>
>> 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.

>
>
>>
> 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

>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>

Attachment Content-Type Size
plpgsql-ref-element-array-type-03.patch text/x-patch 21.4 KB

From: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-02-24 09:48:43
Message-ID: 56CD7C7B.6060304@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-02-24 18:50:27
Message-ID: CAFj8pRCXsCaHGxG_k-naV48Sgp3ngh3LYP36bwJ+jJM8koxdww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2016-02-24 10:48 GMT+01:00 Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)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

>
>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-02-24 21:18:10
Message-ID: 56CE1E12.5030007@gmx.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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.


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-02 21:52:28
Message-ID: CAFj8pRCacWg=hEPethghc_WgnQ0RUeVwvQCDTa1FhtE2Xo-EGw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e(at)gmx(dot)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.
>
>


From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-02 23:27:50
Message-ID: 56D776F6.2080304@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-03 10:45:25
Message-ID: CAFj8pRBSGbNCFcdJYObitARZznwxXYFeP=d_2uxsK+HW2tOPPQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e(at)gmx(dot)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

Attachment Content-Type Size
plpgsql-ref-element-array-type-04.patch text/x-patch 22.6 KB

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-03 10:51:06
Message-ID: CAFj8pRDYJ=rx9J_e28142=Bujjk0S-Go5oNMZbWg1Gy6TVa0Qg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

2016-03-03 0:27 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)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
>


From: Joe Conway <mail(at)joeconway(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 14:15:30
Message-ID: 56E6C782.2050607@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 03/03/2016 05:45 AM, Pavel Stehule wrote:
> 2016-02-24 22:18 GMT+01:00 Peter Eisentraut <peter_e(at)gmx(dot)net
> <mailto:peter_e(at)gmx(dot)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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 14:54:18
Message-ID: 10593.1457967258@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Joe Conway <mail(at)joeconway(dot)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


From: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Joe Conway <mail(at)joeconway(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 15:59:54
Message-ID: 56E6DFFA.9090502@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 14.03.2016 17:54, Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 16:04:41
Message-ID: 31754.1457971481@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 19:27:08
Message-ID: CA+TgmoaJUNYREvo=HxUJazxh+tV0SGPqOE+eA6qUKEVqMgbQBw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 19:38:02
Message-ID: 12264.1457984282@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 21:38:59
Message-ID: CAFj8pRBsTp_ODUG2fg7QL+JbZw+n32AMgqBe=9nEf_LOFW=AUQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-14 20:38 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

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
>


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-14 21:57:16
Message-ID: 6066.1457992636@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 00:17:07
Message-ID: 20591.1458087427@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> Robert Haas <robertmhaas(at)gmail(dot)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


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 15:46:15
Message-ID: 56E97FC7.1040405@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 15:50:56
Message-ID: CAFj8pRCySh0-7zTb548eh=4v0LwL4yy+5SoOYzcQ33m1tvxGVg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-16 16:46 GMT+01:00 Joe Conway <mail(at)joeconway(dot)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
>
>


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 16:38:26
Message-ID: CAFj8pRB+ECM4hOhLD1ze-13uW=D9fhVi2ao5KLPvT_tYRU_ttg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-16 16:50 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

>
>
> 2016-03-16 16:46 GMT+01:00 Joe Conway <mail(at)joeconway(dot)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

>
> Regards
>
> Pavel
>
>
>> Joe
>>
>> --
>> Crunchy Data - http://crunchydata.com
>> PostgreSQL Support for Secure Enterprises
>> Consulting, Training, & Open Source Development
>>
>>
>


From: Joe Conway <mail(at)joeconway(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 16:54:40
Message-ID: 56E98FD0.8060104@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 03/16/2016 09:38 AM, Pavel Stehule wrote:
> 2016-03-16 16:50 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> <mailto:pavel(dot)stehule(at)gmail(dot)com>>:
> 2016-03-16 16:46 GMT+01:00 Joe Conway <mail(at)joeconway(dot)com
> <mailto:mail(at)joeconway(dot)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


From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 19:53:40
Message-ID: 56E9B9C4.6060104@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 20:05:51
Message-ID: 56E9BC9F.8030601@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 20:55:14
Message-ID: CAFj8pRBrM8h5bcggOLnoSu98TFHQwA_yePuKM92QbqCye6+Dbw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-16 20:53 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)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
>


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-16 23:39:17
Message-ID: 19355.1458171557@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)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


From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-17 00:02:40
Message-ID: CAKFQuwbthufj8SZK329pnZtQ_9hXY=kMEnhea+SWbTeKEwPViA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Mar 16, 2016 at 4:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)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.​


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-17 04:43:53
Message-ID: CAFj8pRCBa+W7AJqJMNbqYdJROx8s3qes4aZZLQb3F8mTR8+ZXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-17 0:39 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)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.
>

This is valid for PostgreSQL. I am not sure if it is true in Oracle, if
%TYPE means only reference to type, or %TYPE holds reference to original
object - and when you change the original object, then the function is
invalidated.

Using %TYPE with create time only semantic has not big practical benefit.
But when %TYPE enforce all life dependency, then I have guaranteed so
change on original object will be propagated to depend object. With all
advantages and disadvantages.

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.

The usage of %TYPE outer PL/pgSQL is probably only in FK. But nothing
similar is in standard, and I don't see a reason, why we should to
implement it. In this moment I don't see any important use case.

Pavel

>
> regards, tom lane
>


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-03-17 04:44:46
Message-ID: CAFj8pRDj29LyYWyznZbpvyXqCCL6EQOngRGOC2ERJF3AqHiCqw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-03-17 1:02 GMT+01:00 David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>:

> On Wed, Mar 16, 2016 at 4:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)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.
>

yes, but there is strong limit. You can append column, but you cannot to
alter existing column.

Pavel

>
> David J.​
>
>
>
>


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-04-25 17:40:22
Message-ID: 20160425174022.GB2305@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Good summary. Is there a TODO item here?

---------------------------------------------------------------------------

On Tue, Mar 15, 2016 at 08:17:07PM -0400, Tom Lane wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> >> Robert Haas <robertmhaas(at)gmail(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)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 +


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Subject: Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types
Date: 2016-04-26 14:38:26
Message-ID: CAFj8pRDCm3v7z9BJJjkSfhZ=Dx7jNyUxtfvZ-wUof8=KbVP65w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2016-04-25 19:40 GMT+02:00 Bruce Momjian <bruce(at)momjian(dot)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(dot)stehule(at)gmail(dot)com> writes:
> > >> Robert Haas <robertmhaas(at)gmail(dot)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(at)postgresql(dot)org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
>
> --
> Bruce Momjian <bruce(at)momjian(dot)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 +
>