Re: [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v2

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Steve Prentice <prentice(at)cisco(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v2
Date: 2009-05-22 19:41:00
Message-ID: 162867790905221241x6fbd789cu82ffc3456f3f371c@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

2009/5/21 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Steve Prentice <prentice(at)cisco(dot)com> writes:
>> This patch is intended to supplement Pavel's patch for named and mixed
>> notation support in 8.5. This patch makes it so a plpgsql function can
>> call another function with the same parameter names using the named
>> parameters notation.
>
> Well, plpgsql's parsing has always been a kluge, but I think this is
> really taking the kluge level a step too far.  It's only because AS
> is used in so few contexts that this can even pretend to work --- but
> there are still an awful lot of contexts where AS is used, and will
> likely be more in the future.  So I think it's pretty un-future-proof;
> and it certainly won't scale to any other contexts where we might wish
> that plpsql variables don't get substituted.
>
> It's probably time to bite the bullet and redo the parser as has been
> suggested in the past, ie fix things so that the main parser is used.
> Ideally I'd like to switch the name resolution priority to be more
> Oracle-like, but even if we don't do that it would be a great
> improvement to have actual syntactic knowledge behind the lookups.

I have fast hack, that is based on callback function from
transformExpr - it replace unknown ColumnRef node to Param node. It
works well, but I have two notes:

a) when we use main parser for identification, then valid_sql flag has
different sense. We have to valid all SQL statements - so we have to
change some logic or we have to develop levels of validations. This
have one big advantage - we are able to do complete validation.
Disadvantage - check is necessary before every first run.

b) Change priority (sql identifiers >> plpgsql identifiers) will have
two impacts:

** buggy applications will have different behave
** some an table's alters should change function's behave - so minimum
is reset plpgsql cache.

postgres=# create or replace function test(a int) returns int as $$
declare x int; y int; begin return a+20 as a; end; $$ language
plpgsql;
CREATE FUNCTION
Time: 2,485 ms
postgres=# select test(20);
test
------
40
(1 row)

Time: 1,473 ms
postgres=# create or replace function test(a int) returns int as $$
declare x int; y int; begin return a+20 as a from a; end; $$ language
plpgsql;
ERROR: relation "a" does not exist
LINE 1: SELECT a+20 as a from a
^ --< pointer is correct, look on it
some non proportional font
QUERY: SELECT a+20 as a from a
CONTEXT: SQL statement in PL/PgSQL function "test" near line 1

Attached patch is VERY UGLY, but it should to show possible direction.
I thing, so some similar should by in 8.5

??? Notes, Comments

Regards
Pavel Stehule

>
> Just for the record, you'd have to put the same kluge into the T_RECORD
> and T_ROW cases if we wanted to do it like this.
>
>                        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
>

Attachment Content-Type Size
mainparser.diff text/x-patch 7.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zdenek Kotala 2009-05-22 20:08:42 integer overflow in reloption.h
Previous Message Josh Berkus 2009-05-22 19:32:01 Re: Revisiting default_statistics_target