Skip site navigation (1) Skip section navigation (2)

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: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers

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


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


pgsql-hackers by date

Next:From: Magnus HaganderDate: 2011-10-28 06:09:28
Subject: Re: pg_upgrade if 'postgres' database is dropped
Previous:From: Tom LaneDate: 2011-10-28 04:38:38
Subject: Re: pg_upgrade if 'postgres' database is dropped

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group