Re: [PATCHES] plpgsql CASE statement - last version
Pavel Stehule [EMAIL PROTECTED] writes: I am sending little bit smarter version - without redundant parsing. Applied with corrections --- you had some memory management problems in particular. One thing that I think might annoy people is that you've handled CASE x WHEN a, b, c THEN ... by producing the equivalent of IF x IN (a, b, c). This means that all three of the a, b, c expressions will be evaluated even if a matches. The SQL spec doesn't appear to promise short-circuit evaluation in such a case, but I suspect somebody out there might have a problem someday. It didn't seem tremendously easy to fix though. I suppose anyone who does have a problem can rewrite as CASE x WHEN a THEN ... WHEN b THEN ... WHEN c THEN ... at the cost of duplicating their THEN code. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
Hello I am sending little bit smarter version - without redundant parsing. When test expression is defined, then expressions in WHEN part are modified like $x in ( origin_expression ) $x is referenced to invisible *case* variable that carries result of test expression. Regards Pavel Stehule 2008/5/3 Pavel Stehule [EMAIL PROTECTED]: Hello 2008/5/3 Tom Lane [EMAIL PROTECTED]: Pavel Stehule [EMAIL PROTECTED] writes: 2008/5/2 Heikki Linnakangas [EMAIL PROTECTED]: 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; 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. I agree with Heikki: this patch is seriously ugly, and slower for execution isn't a good enough reason for saddling us with having to maintain such a kluge in the parser. I don't really see why you should need to have multiple expression evaluations, anyhow. Can't you evaluate the test expression once and inject its value into the comparisons using CaseTestExpr, the same way the core CASE-expression code works? I have to look on this way. Regards Pavel Stehule regards, tom lane *** ./doc/src/sgml/plpgsql.sgml.orig 2008-05-03 02:11:36.0 +0200 --- ./doc/src/sgml/plpgsql.sgml 2008-05-06 11:05:05.0 +0200 *** *** 1601,1606 --- 1601,1622 paraliteralIF ... THEN ... ELSEIF ... THEN ... ELSE// /listitem /itemizedlist + + and four forms of literalCASE/: + itemizedlist + listitem + paraliteralCASE ... WHEN ... THEN ... END CASE// + /listitem + listitem + paraliteralCASE ... WHEN ... THEN ... ELSE ... END CASE// + /listitem + listitem + paraliteralCASE WHEN ... THEN ... END CASE// + /listitem + listitem + paraliteralCASE WHEN ... THEN ... ELSE ... END CASE// + /listitem + /itemizedlist /para sect3 *** *** 1751,1756 --- 1767,1838 literalELSEIF/ is an alias for literalELSIF/. /para /sect3 + + sect3 + titleSimple literalCASE/ statement/title + synopsis + CASE replaceableexpression/replaceable + WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN + replaceablestatements/replaceable + optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN + replaceablestatements/replaceable + optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN + replaceablestatements/replaceable + ... /optional/optional + optional ELSE + replaceablestatements/replaceable /optional + END CASE; + /synopsis + para + Provide conditional execution based on equality of operands. If no case is matched, + then is ELSE clause executed. If statement doesn't contains ELSE clause, + then literalCASE_NOT_FOUND/literal exception is raised. + /para + paraHere is example: + programlisting + CASE a + WHEN 1, 2 THEN + msg := 'one or two'; + ELSE + msg := 'other value than one or two'; + END CASE; + /programlisting + /para + /sect3 + + sect3 + titleSearched literalCASE/ statement/title + synopsis + CASE + WHEN replaceableboolean-expression/replaceable THEN + replaceablestatements/replaceable + optional WHEN replaceableboolean-expression/replaceable THEN + replaceablestatements/replaceable + optional WHEN replaceableboolean-expression/replaceable THEN + replaceablestatements/replaceable + ... /optional/optional + optional ELSE + replaceablestatements/replaceable /optional + END CASE; + /synopsis + para + Provide conditional execution based on truth of + replaceableboolean-expression/replaceable. If no case is matched, + then is ELSE clause executed. If statement doesn't contains ELSE clause, + then literalCASE_NOT_FOUND/literal exception is raised. + /para + para Here is example: + programlisting + CASE + WHEN a BETWEEN 0 AND 10 THEN + msg := 'value is between zero and ten'; + WHEN a BETWEEN 11 AND 20 THEN + msg := 'value is between eleven and twenty'; + END CASE; + /programlisting + /para + + /sect3 /sect2 sect2 id=plpgsql-control-structures-loops *** ./src/backend/catalog/sql_feature_packages.txt.orig 2008-05-06 11:01:18.0 +0200 --- ./src/backend/catalog/sql_feature_packages.txt 2008-05-06 11:05:05.0 +0200 *** *** 41,46 --- 41,48 F671 Enhanced integrity management F701 Enhanced integrity
Re: [PATCHES] plpgsql CASE statement - last version
Hello 2008/5/3 Tom Lane [EMAIL PROTECTED]: Pavel Stehule [EMAIL PROTECTED] writes: 2008/5/2 Heikki Linnakangas [EMAIL PROTECTED]: 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; 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. I agree with Heikki: this patch is seriously ugly, and slower for execution isn't a good enough reason for saddling us with having to maintain such a kluge in the parser. I don't really see why you should need to have multiple expression evaluations, anyhow. Can't you evaluate the test expression once and inject its value into the comparisons using CaseTestExpr, the same way the core CASE-expression code works? I have to look on this way. Regards Pavel Stehule regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
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. 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. 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? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
Hello 2008/5/2 Heikki Linnakangas [EMAIL PROTECTED]: 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
Pavel Stehule [EMAIL PROTECTED] writes: 2008/5/2 Heikki Linnakangas [EMAIL PROTECTED]: 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; 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. I agree with Heikki: this patch is seriously ugly, and slower for execution isn't a good enough reason for saddling us with having to maintain such a kluge in the parser. I don't really see why you should need to have multiple expression evaluations, anyhow. Can't you evaluate the test expression once and inject its value into the comparisons using CaseTestExpr, the same way the core CASE-expression code works? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
Hello
2008/5/1 Jaime Casanova [EMAIL PROTECTED]:
On Sat, Apr 5, 2008 at 6:57 AM, Pavel Stehule [EMAIL PROTECTED] 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.
you really compile this one? i get a complain because
read_sql_construct is called with 8 arguments and it should have only
7..
yes, I did it. 8 arguments are from EXECUTE USING patch
http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.108;r2=1.109;f=h
Pavel
.
+ expr = read_sql_construct(',', K_THEN, 0, THEN,
+ SELECT , true, true, tok);
gram.y: In function 'plpgsql_yyparse':
gram.y:1697: warning: passing argument 5 of 'read_sql_construct' makes
integer from pointer without a cast
gram.y:1697: warning: passing argument 7 of 'read_sql_construct' makes
pointer from integer without a cast
gram.y:1697: error: too many arguments to function 'read_sql_construct'
--
regards,
Jaime Casanova
Soporte de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 087171157
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
On Thu, May 1, 2008 at 7:59 AM, Pavel Stehule [EMAIL PROTECTED] wrote: Hello 2008/5/1 Jaime Casanova [EMAIL PROTECTED]: On Sat, Apr 5, 2008 at 6:57 AM, Pavel Stehule [EMAIL PROTECTED] 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. you really compile this one? i get a complain because read_sql_construct is called with 8 arguments and it should have only 7.. yes, I did it. 8 arguments are from EXECUTE USING patch http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.108;r2=1.109;f=h so, i need to see that patch first? patch that doesn't apply because of changes in files (specially definitions moved to other files, but i haven't checked all the .rej yet) -- regards, Jaime Casanova Soporte de PostgreSQL Guayaquil - Ecuador Cel. (593) 087171157 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
On Thu, May 1, 2008 at 8:11 AM, Jaime Casanova [EMAIL PROTECTED] wrote: On Thu, May 1, 2008 at 7:59 AM, Pavel Stehule [EMAIL PROTECTED] wrote: Hello 2008/5/1 Jaime Casanova [EMAIL PROTECTED]: On Sat, Apr 5, 2008 at 6:57 AM, Pavel Stehule [EMAIL PROTECTED] 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. you really compile this one? i get a complain because read_sql_construct is called with 8 arguments and it should have only 7.. yes, I did it. 8 arguments are from EXECUTE USING patch http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.108;r2=1.109;f=h so, i need to see that patch first? patch that doesn't apply because of changes in files (specially definitions moved to other files, but i haven't checked all the .rej yet) sorry, you mean already applied RETURN QUERY, right? i was thinking in pending patch RETURN QUERY EXECUTE... i will check again my files but i'm sure i have updated tu CVS TIP before try your patch -- Atentamente, Jaime Casanova Soporte de PostgreSQL Guayaquil - Ecuador Cel. (593) 087171157 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
On Thu, May 1, 2008 at 8:14 AM, Jaime Casanova [EMAIL PROTECTED] wrote: On Thu, May 1, 2008 at 8:11 AM, Jaime Casanova [EMAIL PROTECTED] wrote: On Thu, May 1, 2008 at 7:59 AM, Pavel Stehule [EMAIL PROTECTED] wrote: Hello 2008/5/1 Jaime Casanova [EMAIL PROTECTED]: On Sat, Apr 5, 2008 at 6:57 AM, Pavel Stehule [EMAIL PROTECTED] 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. you really compile this one? i get a complain because read_sql_construct is called with 8 arguments and it should have only 7.. yes, I did it. 8 arguments are from EXECUTE USING patch http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.108;r2=1.109;f=h so, i need to see that patch first? patch that doesn't apply because of changes in files (specially definitions moved to other files, but i haven't checked all the .rej yet) sorry, you mean already applied RETURN QUERY, right? i was thinking in pending patch RETURN QUERY EXECUTE... i will check again my files but i'm sure i have updated tu CVS TIP before try your patch ok, you're right... sorry for the noise... i will try it again -- regards, Jaime Casanova Soporte de PostgreSQL Guayaquil - Ecuador Cel. (593) 087171157 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] plpgsql CASE statement - last version
On Sat, Apr 5, 2008 at 6:57 AM, Pavel Stehule [EMAIL PROTECTED] 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.
you really compile this one? i get a complain because
read_sql_construct is called with 8 arguments and it should have only
7...
+ expr = read_sql_construct(',', K_THEN, 0, THEN,
+ SELECT , true, true, tok);
gram.y: In function 'plpgsql_yyparse':
gram.y:1697: warning: passing argument 5 of 'read_sql_construct' makes
integer from pointer without a cast
gram.y:1697: warning: passing argument 7 of 'read_sql_construct' makes
pointer from integer without a cast
gram.y:1697: error: too many arguments to function 'read_sql_construct'
--
regards,
Jaime Casanova
Soporte de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 087171157
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] plpgsql CASE statement - last version
Hello
I found some bugs when I used base_lexer, so I returned back own
lexer. It's only little bit longer, but simpler.
Regards
Pavel Stehule
*** ./doc/src/sgml/plpgsql.sgml.orig 2008-04-04 12:07:12.0 +0200
--- ./doc/src/sgml/plpgsql.sgml 2008-04-04 21:55:08.0 +0200
***
*** 1590,1595
--- 1590,1611
paraliteralIF ... THEN ... ELSEIF ... THEN ... ELSE//
/listitem
/itemizedlist
+
+ and four forms of literalCASE/:
+ itemizedlist
+ listitem
+ paraliteralCASE ... WHEN ... THEN ... END CASE//
+ /listitem
+ listitem
+ paraliteralCASE ... WHEN ... THEN ... ELSE ... END CASE//
+ /listitem
+ listitem
+ paraliteralCASE WHEN ... THEN ... END CASE//
+ /listitem
+ listitem
+ paraliteralCASE WHEN ... THEN ... ELSE ... END CASE//
+ /listitem
+ /itemizedlist
/para
sect3
***
*** 1740,1745
--- 1756,1827
literalELSEIF/ is an alias for literalELSIF/.
/para
/sect3
+
+ sect3
+ titleSimple literalCASE/ statement/title
+ synopsis
+ CASE replaceableexpression/replaceable
+ WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+ replaceablestatements/replaceable
+ optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+ replaceablestatements/replaceable
+ optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+ replaceablestatements/replaceable
+ ... /optional/optional
+ optional ELSE
+ replaceablestatements/replaceable /optional
+ END CASE;
+ /synopsis
+ para
+ Provide conditional execution based on equality of operands. If no case is matched,
+ then is ELSE clause executed. If statement doesn't contains ELSE clause,
+ then literalCASE_NOT_FOUND/literal exception is raised.
+ /para
+ paraHere is example:
+ programlisting
+ CASE a
+ WHEN 1, 2 THEN
+ msg := 'one or two';
+ ELSE
+ msg := 'other value than one or two';
+ END CASE;
+ /programlisting
+ /para
+ /sect3
+
+ sect3
+ titleSearched literalCASE/ statement/title
+ synopsis
+ CASE
+ WHEN replaceableboolean-expression/replaceable THEN
+ replaceablestatements/replaceable
+ optional WHEN replaceableboolean-expression/replaceable THEN
+ replaceablestatements/replaceable
+ optional WHEN replaceableboolean-expression/replaceable THEN
+ replaceablestatements/replaceable
+ ... /optional/optional
+ optional ELSE
+ replaceablestatements/replaceable /optional
+ END CASE;
+ /synopsis
+ para
+ Provide conditional execution based on truth of
+ replaceableboolean-expression/replaceable. If no case is matched,
+ then is ELSE clause executed. If statement doesn't contains ELSE clause,
+ then literalCASE_NOT_FOUND/literal exception is raised.
+ /para
+ para Here is example:
+ programlisting
+ CASE
+ WHEN a BETWEEN 0 AND 10 THEN
+ msg := 'value is between zero and ten';
+ WHEN a BETWEEN 11 AND 20 THEN
+ msg := 'value is between eleven and twenty';
+ END CASE;
+ /programlisting
+ /para
+
+ /sect3
/sect2
sect2 id=plpgsql-control-structures-loops
*** ./src/backend/catalog/sql_feature_packages.txt.orig 2008-04-04 22:47:55.0 +0200
--- ./src/backend/catalog/sql_feature_packages.txt 2008-04-04 22:59:55.0 +0200
***
*** 41,46
--- 41,48
F671 Enhanced integrity management
F701 Enhanced integrity management
F812 Core
+ P004 PSM
+ P008 PSM
S011 Core
S023 Basic object support
S024 Enhanced object support
*** ./src/backend/catalog/sql_features.txt.orig 2008-04-04 22:45:52.0 +0200
--- ./src/backend/catalog/sql_features.txt 2008-04-04 23:05:31.0 +0200
***
*** 297,302
--- 297,304
F831 Full cursor update NO
F831 Full cursor update 01 Updatable scrollable cursors NO
F831 Full cursor update 02 Updatable ordered cursors NO
+ P004 Extended CASE statement YES
+ P008 Comma-separated predicates in simple CASE statement YES
S011 Distinct data types NO
S011 Distinct data types 01 USER_DEFINED_TYPES view NO
S023 Basic structured types NO
*** ./src/include/utils/errcodes.h.orig 2008-04-02 14:02:06.0 +0200
--- ./src/include/utils/errcodes.h 2008-04-03 07:49:40.0 +0200
***
*** 107,112
--- 107,113
/* Class 22 - Data Exception */
#define ERRCODE_DATA_EXCEPTIONMAKE_SQLSTATE('2','2', '0','0','0')
+ #define ERRCODE_CASE_NOT_FOUNDERRCODE_DATA_EXCEPTION
#define ERRCODE_ARRAY_ELEMENT_ERROR MAKE_SQLSTATE('2','2', '0','2','E')
/* SQL99's actual definition of array element error is subscript error */
#define
