Обсуждение: a few small bugs in plpgsql
Hello, today I found a few bugs: a) parser allow a labels on invalid positions with strange runtime bug: postgres=# CREATE OR REPLACE FUNCTION foo() RETURNS void AS $$ BEGIN FOR i IN 1..2 <<<invalidLabel>> LOOP RAISE NOTICE '%',i; END LOOP; END; $$ LANGUAGE plpgsql; CREATE FUNCTION ERROR: column "invalidlabel" does not exist LINE 2: <<<invalidLabel>> ^ QUERY: SELECT 2 <<<invalidLabel>> CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable postgres=# b) SRF functions must not be finished by RETURN statement - I know, so there is outer default block, but it looks like inconsistency for SRF functions, because you can use a RETURN NEXT without RETURN. It maybe isn't bug - but I am filling it as inconsistency. postgres=# CREATE OR REPLACE FUNCTION fg(OUT i int) RETURNS SETOF int AS $$ BEGIN FOR i IN 1..3 LOOP fg.i := i; RETURN NEXT; END LOOP; END; $$ LANGUAGE plpgsql; CREATE FUNCTION postgres=# select fg();fg ---- 1 2 3 (3 rows) Regards Pavel Stehule
On Thu, Oct 7, 2010 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > Hello, > > today I found a few bugs: > > a) parser allow a labels on invalid positions with strange runtime bug: > > postgres=# CREATE OR REPLACE FUNCTION foo() > RETURNS void AS $$ > BEGIN > FOR i IN 1..2 > <<<invalidLabel>> > LOOP > RAISE NOTICE '%',i; > END LOOP; > END; > $$ LANGUAGE plpgsql; > CREATE FUNCTION > > ERROR: column "invalidlabel" does not exist > LINE 2: <<<invalidLabel>> > ^ > QUERY: SELECT 2 > <<<invalidLabel>> > CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable > postgres=# I'm not sure if I'd call that a bug, but it does look like a somewhat odd error message, at least at first glance. > b) SRF functions must not be finished by RETURN statement - I know, so > there is outer default block, but it looks like inconsistency for SRF > functions, because you can use a RETURN NEXT without RETURN. It maybe > isn't bug - but I am filling it as inconsistency. > > postgres=# CREATE OR REPLACE FUNCTION fg(OUT i int) > RETURNS SETOF int AS $$ > BEGIN > FOR i IN 1..3 > LOOP fg.i := i; > RETURN NEXT; > END LOOP; > END; > $$ LANGUAGE plpgsql; > CREATE FUNCTION > > postgres=# select fg(); > fg > ---- > 1 > 2 > 3 > (3 rows) I don't see what's wrong with this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
>> b) SRF functions must not be finished by RETURN statement - I know, so
>> there is outer default block, but it looks like inconsistency for SRF
>> functions, because you can use a RETURN NEXT without RETURN. It maybe
>> isn't bug - but I am filling it as inconsistency.
Hmmm. Is there any likelyhood we'll go back to requiring the final
RETURN in the future?
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
Robert Haas <robertmhaas@gmail.com> writes: > On Thu, Oct 7, 2010 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> b) SRF functions must not be finished by RETURN statement - I know, so >> there is outer default block, but it looks like inconsistency for SRF >> functions, because you can use a RETURN NEXT without RETURN. It maybe >> isn't bug - but I am filling it as inconsistency. > I don't see what's wrong with this. Back around 8.0 we intentionally changed plpgsql to not require a final RETURN in cases where RETURN isn't used to supply the result value: http://archives.postgresql.org/pgsql-hackers/2005-04/msg00152.php http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=e00ee887612da0dab02f1a56e33d8ae821710e14 Even if there were a good argument for going back to the old way, backwards-compatibility would win the day, I think. Being strict about this --- in *either* direction --- would break a lot of code. regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
> a) parser allow a labels on invalid positions with strange runtime bug:
> postgres=# CREATE OR REPLACE FUNCTION foo()
> RETURNS void AS $$
> BEGIN
> FOR i IN 1..2
> <<<invalidLabel>>
> LOOP
> RAISE NOTICE '%',i;
> END LOOP;
> END;
> $$ LANGUAGE plpgsql;
> CREATE FUNCTION
> ERROR: column "invalidlabel" does not exist
> LINE 2: <<<invalidLabel>>
> ^
> QUERY: SELECT 2
> <<<invalidLabel>>
> CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable
That isn't a bug, because the construct isn't a label, and wouldn't be
even if you'd gotten the number of <'s right ;-). What you have is an
expression "2 <<< invalidLabel >>", which given the right operator
definitions could be perfectly valid. plpgsql labels can't appear in
the middle of an SQL expression.
regards, tom lane
Hello 2010/10/8 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> a) parser allow a labels on invalid positions with strange runtime bug: > >> postgres=# CREATE OR REPLACE FUNCTION foo() >> RETURNS void AS $$ >> BEGIN >> FOR i IN 1..2 >> <<<invalidLabel>> >> LOOP >> RAISE NOTICE '%',i; >> END LOOP; >> END; >> $$ LANGUAGE plpgsql; >> CREATE FUNCTION > >> ERROR: column "invalidlabel" does not exist >> LINE 2: <<<invalidLabel>> >> ^ >> QUERY: SELECT 2 >> <<<invalidLabel>> >> CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable > > That isn't a bug, because the construct isn't a label, and wouldn't be > even if you'd gotten the number of <'s right ;-). What you have is an > expression "2 <<< invalidLabel >>", which given the right operator > definitions could be perfectly valid. plpgsql labels can't appear in > the middle of an SQL expression. > I see it now - I did a bug <<<some>>, but with correct text there is same behave CREATE OR REPLACE FUNCTION foo() RETURNS void AS $$ BEGIN FOR i IN 1..10 <<label>> LOOP RAISE NOTICE '%', i; END LOOP; END; $$ LANGUAGE plpgsql; CREATE FUNCTION Now I understand to interpretation. But there is little bit difficult to understand to error message. Can be message enhanced to show a complete expression? Some like postgres=# select foo(); ERROR: column "label" does not exist Detail: Expr 10 <<label>> LINE 2: <<label>> Regards Pavel > regards, tom lane >
2010/10/8 Tom Lane <tgl@sss.pgh.pa.us>: > Robert Haas <robertmhaas@gmail.com> writes: >> On Thu, Oct 7, 2010 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> b) SRF functions must not be finished by RETURN statement - I know, so >>> there is outer default block, but it looks like inconsistency for SRF >>> functions, because you can use a RETURN NEXT without RETURN. It maybe >>> isn't bug - but I am filling it as inconsistency. > >> I don't see what's wrong with this. > > Back around 8.0 we intentionally changed plpgsql to not require a final > RETURN in cases where RETURN isn't used to supply the result value: > http://archives.postgresql.org/pgsql-hackers/2005-04/msg00152.php > http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=e00ee887612da0dab02f1a56e33d8ae821710e14 > > Even if there were a good argument for going back to the old way, > backwards-compatibility would win the day, I think. Being strict > about this --- in *either* direction --- would break a lot of code. > > regards, tom lane ok, understand - thank you. I think so it was not a best decision - the RETURN statement helps with higher verbosity, but I can accept so there are not way to back. Regards Pavel
Pavel Stehule <pavel.stehule@gmail.com> writes:
> Now I understand to interpretation. But there is little bit difficult
> to understand to error message. Can be message enhanced to show a
> complete expression?
It does already:
regression=# select foo();
ERROR: column "label" does not exist
LINE 2: <<label>> ^
QUERY: SELECT 10 <<label>>
CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable
The "query" is SELECT followed by whatever plpgsql thought the
expression was.
regards, tom lane
2010/10/8 Tom Lane <tgl@sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule@gmail.com> writes: >> Now I understand to interpretation. But there is little bit difficult >> to understand to error message. Can be message enhanced to show a >> complete expression? > > It does already: > > regression=# select foo(); > ERROR: column "label" does not exist > LINE 2: <<label>> > ^ > QUERY: SELECT 10 the keyword QUERY is misleading :( but you have a true - there are all necessary information. Regards Pavel Stehule > <<label>> > CONTEXT: PL/pgSQL function "foo" line 3 at FOR with integer loop variable > > The "query" is SELECT followed by whatever plpgsql thought the > expression was. > > regards, tom lane >