Re: Review: [PL/pgSQL] %TYPE and array declaration - second patch

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Wojciech Muła <wojciech_mula(at)poczta(dot)onet(dot)pl>
Subject: Re: Review: [PL/pgSQL] %TYPE and array declaration - second patch
Date: 2011-10-28 04:43:50
Message-ID: CAFj8pRDNQTLhxBvUi5hMn1zaW=Tp0Hz7MDhLcS2+txUbU9_Q9w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

2011/10/28 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> there is final Wojciech's patch
>

this is just small note about length of this patch. This patch was
significantly smaller then he solved problem with derivate types for
compound types - it should to solve problem described in this thread

http://stackoverflow.com/questions/7634704/declare-variable-of-composite-type-in-postgresql-using-type

Regards

Pavel Stehule

> I looked this over a little bit, but I don't see an answer to the
> question I put on the commitfest entry: why is this being done in
> plpgsql, and not somewhere in the core code?  The core has also got
> the concept of %TYPE, and could stand to have support for attaching []
> notation to that.  I'm not entirely sure if anything could be shared,
> but it would sure be worth looking at.  I've wished for some time that
> %TYPE not be handled directly in read_datatype() at all, but rather in
> the core parse_datatype() function.  This would require passing some
> sort of callback function to parse_datatype() to let it know what
> variables can be referenced in %TYPE, but we have parser callback
> functions just like that already.  One benefit we could get that way
> is that the core meaning of %TYPE (type of a table field name) would
> still be available in plpgsql, if the name didn't match any declared
> plpgsql variable.
>
> However, assuming that we're sticking to just changing plpgsql for the
> moment ... I cannot escape the feeling that this is a large patch with a
> small patch struggling to get out.  It should not require 500 net new
> lines of code to provide this functionality, when all we're doing is
> looking up the array type whose element type is the type the existing
> code can derive.  I would have expected to see the grammar passing one
> extra flag to plpgsql_parse_cwordtype and plpgsql_parse_cwordrowtype
> routines that were little changed except for the addition of a step to
> find the array type.
>
> Another complaint is that the grammar changes assume that the first
> token of decl_datatype must be T_WORD or T_CWORD, which means that
> it fails for cases such as unreserved plpgsql keywords.  This example
> should work, and does work in 9.1:
>
> create domain hint as int;
>
> create or replace function foo() returns void as $$
> declare
>  x hint;
> begin
> end
> $$ language plpgsql;
>
> but fails with this patch because "hint" is returned by the lexer as
> K_HINT not T_WORD.  You might be able to get around that by adding a
> production with unreserved_keyword --- but I'm unsure that that would
> cover every case that worked before, and in any case I do not see the
> point of changing the grammar production at all.  It gains almost
> nothing to have Bison parse the [] rather than doing it with C code in
> read_datatype().
>
>                        regards, tom lane
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2011-10-28 06:09:28 Re: pg_upgrade if 'postgres' database is dropped
Previous Message Tom Lane 2011-10-28 04:38:38 Re: pg_upgrade if 'postgres' database is dropped