Re: plpgsql CASE statement - last version - Mailing list pgsql-patches

From Pavel Stehule
Subject Re: plpgsql CASE statement - last version
Date
Msg-id 162867790805020609uf5f180as6acfa2a539417250@mail.gmail.com
Whole thread Raw
In response to Re: plpgsql CASE statement - last version  ("Heikki Linnakangas" <heikki@enterprisedb.com>)
Responses Re: plpgsql CASE statement - last version  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Hello

2008/5/2 Heikki Linnakangas <heikki@enterprisedb.com>:
> Pavel Stehule wrote:
>>
>> Hello
>>
>> I found some bugs when I used base_lexer, so I returned back own
>> lexer. It's only little bit longer, but simpler.
>
> Hmm. I don't like having to lex the expressions again. It just doesn't feel
> right.

me too. But when I tested to use base_lexer routines, I had to write
similar number of lines and add new separate file
http://archives.postgresql.org/pgsql-patches/2008-04/msg00131.php -
main problems of this version is enhanced strings, because base_lexer
don't forward this information.

>
> How about taking a completely different strategy, and implement the
> CASE-WHEN construct fully natively in plpgsql, instead of trying to convert
> it to a single SQL CASE-WHEN expression? It's not a very good match anyway;
> you have to do tricks to convert the comma-separated lists of WHEN
> expressions to WHEN-THEN clauses, and you can't use the THEN-clauses as is,
> but you have to replace them with offsets into the array. I think
> implementing the logic in pl_exec.c would lead to cleaner code.
>

It was first variant. It's  simpler for parsing and slower for
execution :(. It means more than once expression evaluation and for
simple case value casting and comparation.

> FWIW, the current approach gives pretty cryptic CONTEXT information in error
> messages as well. For example, this pretty simple case-when example:
>
> postgres=# create or replace FUNCTION case_test(int)
> returns text as $$
> begin
>  case $1
>    when 1 then
>      return 'one';
>    when 'invalid' then
>      return 'two';
>    when 3,4,5 then
>      return 'three, four or five';
>  end case;
> end;
> $$ language plpgsql immutable;
> CREATE FUNCTION
>
> gives this pretty hard-to-understand error message:
>
> postgres=# SELECT case_test(1);
> ERROR:  invalid input syntax for integer: "invalid"
> CONTEXT:  SQL statement "SELECT CASE   $1  WHEN  1 THEN  1  WHEN 'invalid'
> THEN  2  WHEN  3 THEN  3  WHEN 4 THEN  3  WHEN 5 THEN  3  END "
> PL/pgSQL function "case_test" line 2 at unknown
>
> BTW, what does PL/SQL or PSM say about the type-compatibility of the CASE
> and the WHENs? We're very lenient in assignments, how should this behave?
>

what I know, it have to be compareable

case $1
    when 1 then
     return 'one';
   when 'invalid' then
     return 'two';

is same as

if expr1 = 1 then\
  return 'one'
elsif expr1 = 'invalid' then
  ....

I have to agree, so this message is ugly, but on second hand I can
check this error in compile time. When I move it to pl_exec I can
detect this bug much later, so current solution is safe. So, when I
read error message I see correct message - only CONTEXT is crazy (but
it is strange not only in this case)/ I haven't idea how I can solve
it - maybe notice in documentation. We can add hint to this message.
Other way (mix read_sql_construct and add_expr) has other problem with
nested case statements. Maybe safe to somewhere positions of
placeholders. But It will not be simple too, because width of
placeholders can be different.

Regards
Pavel Stehule

> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>

pgsql-patches by date:

Previous
From: "Heikki Linnakangas"
Date:
Subject: Re: plpgsql CASE statement - last version
Next
From: "George Gensure"
Date:
Subject: Re: pg_postmaster_reload_time() patch