[PATCHES] plpgsql, return can contains any expression
Hello, this patch allow any expression in return statement and it does simply casting from non type record into typed records. Allows same casting as assign statement. Regards Pavel Stehule From: Bruce Momjian <[EMAIL PROTECTED]> To: Pavel Stehule <[EMAIL PROTECTED]> CC: [EMAIL PROTECTED], [EMAIL PROTECTED] Subject: Re: near future plpgsql? Date: Wed, 23 Aug 2006 08:25:18 -0400 (EDT) Pavel Stehule wrote: > Hello, > > I had talk with Neil and I knew so Neil forgot on my patch > http://archives.postgresql.org/pgsql-patches/2005-11/msg00045.php . I want > to resend this patch again, but this patch is in conflict with cancelled > "lookahead" patch. Can I work on it? My patch was sent before feature > frease. But I prefere apply it after "lookahead" patch. > > your recommendation? Yes, please send an updated version. Sorry we forgot about it. I try to keep patches around until someone does apply it, but didn't do that in this case. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + _ Citite se osamele? Poznejte nekoho vyjmecneho diky Match.com. http://www.msn.cz/ *** ./doc/src/sgml/plpgsql.sgml.orig 2006-08-23 14:29:07.0 +0200 --- ./doc/src/sgml/plpgsql.sgml 2006-08-23 14:31:43.0 +0200 *** *** 1586,1599 - When returning a scalar type, any expression can be used. The - expression's result will be automatically cast into the - function's return type as described for assignments. To return a - composite (row) value, you must write a record or row variable - as the expression. - - - If you declared the function with output parameters, write just RETURN with no expression. The current values of the output parameter variables will be returned. --- 1586,1591 *** ./src/pl/plpgsql/src/gram.y.orig 2006-08-23 10:39:35.0 +0200 --- ./src/pl/plpgsql/src/gram.y 2006-08-23 14:37:20.0 +0200 *** *** 2001,2006 --- 2001,2007 make_return_stmt(int lineno) { PLpgSQL_stmt_return *new; + int tok; new = palloc0(sizeof(PLpgSQL_stmt_return)); new->cmd_type = PLPGSQL_STMT_RETURN; *** *** 2026,2032 } else if (plpgsql_curr_compile->fn_retistuple) { ! switch (yylex()) { case K_NULL: /* we allow this to support RETURN NULL in triggers */ --- 2027,2034 } else if (plpgsql_curr_compile->fn_retistuple) { ! tok = yylex(); ! switch (tok) { case K_NULL: /* we allow this to support RETURN NULL in triggers */ *** *** 2040,2051 new->retvarno = yylval.rec->recno; break; default: ! yyerror("RETURN must specify a record or row variable in function returning tuple"); break; } ! if (yylex() != ';') ! yyerror("RETURN must specify a record or row variable in function returning tuple"); } else { --- 2042,2059 new->retvarno = yylval.rec->recno; break; + case T_WORD: + case '(': + plpgsql_push_back_token(tok); + new->expr = plpgsql_read_expression(';',";"); + break; + default: ! yyerror("RETURN must specify a value in function returning tuple"); break; } ! if (!new->expr && yylex() != ';') ! yyerror("RETURN must specify a value in function returning tuple"); } else { *** *** 2065,2070 --- 2073,2079 make_return_next_stmt(int lineno) { PLpgSQL_stmt_return_next *new; + int tok; if (!plpgsql_curr_compile->fn_retset) yyerror("cannot use RETURN NEXT in a non-SETOF function"); *** *** 2083,2089 } else if (plpgsql_curr_compile->fn_retistuple) { ! switch (yylex()) { case T_ROW: new->retvarno = yylval.row->rowno; --- 2092,2099 } else if (plpgsql_curr_compile->fn_retistuple) { ! tok = yylex(); ! switch (tok) { case T_ROW: new->retvarno = yylval.row->rowno; *** *** 2093,2104 new->retvarno = yylval.rec->recno; break; default: ! yyerror("RETURN NEXT must specify a record or row variable in function returning tuple"); break; } ! if (yylex() != ';') ! yyerror("RETURN NEXT must specify a record or row variable in function returning tuple"); } else new->expr = plpgsql_read_expression(';', ";"); --- 2103,2120 new->retvarno = yylval.rec->recno; break; + case T_WORD: + case '(': + plpgsql_push_back_token(tok); + new->expr = plpgsql_read_expression(';',";"); + break; + default: ! yyerror("RETURN NEXT must a value in function returning tuple"); break; } ! if (!new->expr && yylex() != ';') ! yyerror("RETURN NEXT must specify a value in function returning tuple"); } else
[PATCHES] plpgsql, return can contains any expression
Hello This patch allows using any row expression in return statement and does transformation from untyped row to composite types if it's necessary. Regards Pavel Stehule _ Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/ *** ./doc/src/sgml/plpgsql.sgml.orig 2006-08-23 14:29:07.0 +0200 --- ./doc/src/sgml/plpgsql.sgml 2006-08-23 14:31:43.0 +0200 *** *** 1586,1599 - When returning a scalar type, any expression can be used. The - expression's result will be automatically cast into the - function's return type as described for assignments. To return a - composite (row) value, you must write a record or row variable - as the expression. - - - If you declared the function with output parameters, write just RETURN with no expression. The current values of the output parameter variables will be returned. --- 1586,1591 *** ./src/pl/plpgsql/src/gram.y.orig 2006-08-23 10:39:35.0 +0200 --- ./src/pl/plpgsql/src/gram.y 2006-08-24 13:49:51.0 +0200 *** *** 2001,2010 --- 2001,2013 make_return_stmt(int lineno) { PLpgSQL_stmt_return *new; + int tok; new = palloc0(sizeof(PLpgSQL_stmt_return)); new->cmd_type = PLPGSQL_STMT_RETURN; new->lineno = lineno; + new->compat_tupdesc = PLPGSQL_TUPLE_DESC_UNKNOWN; + new->pcache = NULL; new->expr = NULL; new->retvarno = -1; *** *** 2026,2032 } else if (plpgsql_curr_compile->fn_retistuple) { ! switch (yylex()) { case K_NULL: /* we allow this to support RETURN NULL in triggers */ --- 2029,2036 } else if (plpgsql_curr_compile->fn_retistuple) { ! tok = yylex(); ! switch (tok) { case K_NULL: /* we allow this to support RETURN NULL in triggers */ *** *** 2040,2051 new->retvarno = yylval.rec->recno; break; default: ! yyerror("RETURN must specify a record or row variable in function returning tuple"); break; } ! if (yylex() != ';') ! yyerror("RETURN must specify a record or row variable in function returning tuple"); } else { --- 2044,2061 new->retvarno = yylval.rec->recno; break; + case T_WORD: + case '(': + plpgsql_push_back_token(tok); + new->expr = plpgsql_read_expression(';',";"); + break; + default: ! yyerror("RETURN must specify a value in function returning tuple"); break; } ! if (!new->expr && yylex() != ';') ! yyerror("RETURN must specify a value in function returning tuple"); } else { *** *** 2065,2070 --- 2075,2081 make_return_next_stmt(int lineno) { PLpgSQL_stmt_return_next *new; + int tok; if (!plpgsql_curr_compile->fn_retset) yyerror("cannot use RETURN NEXT in a non-SETOF function"); *** *** 2072,2077 --- 2083,2090 new = palloc0(sizeof(PLpgSQL_stmt_return_next)); new->cmd_type = PLPGSQL_STMT_RETURN_NEXT; new->lineno = lineno; + new->compat_tupdesc = PLPGSQL_TUPLE_DESC_UNKNOWN; + new->pcache = NULL; new->expr = NULL; new->retvarno = -1; *** *** 2083,2089 } else if (plpgsql_curr_compile->fn_retistuple) { ! switch (yylex()) { case T_ROW: new->retvarno = yylval.row->rowno; --- 2096,2103 } else if (plpgsql_curr_compile->fn_retistuple) { ! tok = yylex(); ! switch (tok) { case T_ROW: new->retvarno = yylval.row->rowno; *** *** 2093,2104 new->retvarno = yylval.rec->recno; break; default: ! yyerror("RETURN NEXT must specify a record or row variable in function returning tuple"); break; } ! if (yylex() != ';') ! yyerror("RETURN NEXT must specify a record or row variable in function returning tuple"); } else new->expr = plpgsql_read_expression(';', ";"); --- 2107,2124 new->retvarno = yylval.rec->recno; break; + case T_WORD: + case '(': + plpgsql_push_back_token(tok); + new->expr = plpgsql_read_expression(';',";"); + break; + default: ! yyerror("RETURN NEXT must a value in function returning tuple"); break; } ! if (!new->expr && yylex() != ';') ! yyerror("RETURN NEXT must specify a value in function returning tuple"); } else new->expr = plpgsql_read_expression(';', ";"); *** ./src/pl/plpgsql/src/pl_exec.c.orig 2006-08-23 11:30:40.0 +0200 --- ./src/pl/plpgsql/src/pl_exec.c 2006-08-24 16:39:05.0 +0200 *** *** 155,160 --- 155,162 static void exec_set_found(PLpgSQL_execstate *estate, bool state); static void free_var(PLpgSQL_var *var); + static void exec_return_coerce_tuple(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt); + /* -- * p
Re: [PATCHES] plpgsql, return can contains any expression
While this patch has new regression tests, it doesn't have new expected output for it. Please update the patch to supply that. Thanks. --- Pavel Stehule wrote: > Hello > > This patch allows using any row expression in return statement and does > transformation from untyped row to composite types if it's necessary. > > Regards > Pavel Stehule > > _ > Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/ [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plpgsql, return can contains any expression
Hello I am sending corrected version. regards Pavel Stehule While this patch has new regression tests, it doesn't have new expected output for it. Please update the patch to supply that. Thanks. --- Pavel Stehule wrote: > Hello > > This patch allows using any row expression in return statement and does > transformation from untyped row to composite types if it's necessary. > > Regards > Pavel Stehule > > _ > Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/ [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly _ Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/ return.diff Description: application/binary ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plpgsql, return can contains any expression
I am going to need this in 24 hours or it might not make it into 8.2. --- bruce wrote: > > While this patch has new regression tests, it doesn't have new expected > output for it. Please update the patch to supply that. Thanks. > > --- > > Pavel Stehule wrote: > > Hello > > > > This patch allows using any row expression in return statement and does > > transformation from untyped row to composite types if it's necessary. > > > > Regards > > Pavel Stehule > > > > _ > > Chcete sdilet sve obrazky a hudbu s prateli? http://messenger.msn.cz/ > > [ Attachment, skipping... ] > > > > > ---(end of broadcast)--- > > TIP 6: explain analyze is your friend > > -- > Bruce Momjian [EMAIL PROTECTED] > EnterpriseDBhttp://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] plpgsql, return can contains any expression
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > This patch allows using any row expression in return statement and does > transformation from untyped row to composite types if it's necessary. This patch doesn't seem to cope with cases where the supplied tuple has the wrong number of columns, and it doesn't look like it's being careful about dropped columns either. Also, that's a mighty bizarre-looking choice of cache memory context in coerce_to_tuple ... but then again, why are you bothering with a cache at all for temporary arrays? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpgsql, return can contains any expression
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > This patch allows using any row expression in return statement and does > transformation from untyped row to composite types if it's necessary. This patch doesn't seem to cope with cases where the supplied tuple has the wrong number of columns, and it doesn't look like it's being careful about dropped columns either. Also, that's a mighty bizarre-looking choice of cache memory context in coerce_to_tuple ... but then again, why are you bothering with a cache at all for temporary arrays? I am sorry, Tom. But I don't understand. I can check number of columns, ofcourse and I'll do it. What cache for temporary arrays do you mean? Best regards Pavel Stehule _ Emotikony a pozadi programu MSN Messenger ozivi vasi konverzaci. http://messenger.msn.cz/ ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
"Pavel Stehule" <[EMAIL PROTECTED]> writes: >> This patch doesn't seem to cope with cases where the supplied tuple has >> the wrong number of columns, and it doesn't look like it's being careful >> about dropped columns either. Also, that's a mighty bizarre-looking >> choice of cache memory context in coerce_to_tuple ... but then again, >> why are you bothering with a cache at all for temporary arrays? > I am sorry, Tom. But I don't understand. I can check number of columns, > ofcourse and I'll do it. What cache for temporary arrays do you mean? I thought that making coerce_to_tuple depend on estate->err_func was pretty bizarre, and that there was no need for any "cache" at all for arrays that need only live as long as the function runs. All you are saving here is a palloc/pfree cycle, which is not worth the obscurantism and risk of bugs (are you sure natts can never change?). BTW, if you want this patch to make it into 8.2, it needs to be fixed and resubmitted *very* soon. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
"Pavel Stehule" <[EMAIL PROTECTED]> writes: >> This patch doesn't seem to cope with cases where the supplied tuple has >> the wrong number of columns, and it doesn't look like it's being careful >> about dropped columns either. Also, that's a mighty bizarre-looking >> choice of cache memory context in coerce_to_tuple ... but then again, >> why are you bothering with a cache at all for temporary arrays? > I am sorry, Tom. But I don't understand. I can check number of columns, > ofcourse and I'll do it. What cache for temporary arrays do you mean? I thought that making coerce_to_tuple depend on estate->err_func was pretty bizarre, and that there was no need for any "cache" at all for arrays that need only live as long as the function runs. All you are saving here is a palloc/pfree cycle, which is not worth the obscurantism and risk of bugs (are you sure natts can never change?). No, cache there is ugly. But I don't have idea about more efective implementation of it :-(. First version of this patch was more clean. and little bit slow. This cache save 10%. BTW, if you want this patch to make it into 8.2, it needs to be fixed and resubmitted *very* soon. I understand, but I am not able work on it in next four days. And I need help with it from Neil. It will be for 8.3. Thank you Pavel _ Emotikony a pozadi programu MSN Messenger ozivi vasi konverzaci. http://messenger.msn.cz/ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
OK, where are we on this patch? --- Pavel Stehule wrote: > > > > > >"Pavel Stehule" <[EMAIL PROTECTED]> writes: > > >> This patch doesn't seem to cope with cases where the supplied tuple has > > >> the wrong number of columns, and it doesn't look like it's being > >careful > > >> about dropped columns either. Also, that's a mighty bizarre-looking > > >> choice of cache memory context in coerce_to_tuple ... but then again, > > >> why are you bothering with a cache at all for temporary arrays? > > > > > I am sorry, Tom. But I don't understand. I can check number of columns, > > > ofcourse and I'll do it. What cache for temporary arrays do you mean? > > > >I thought that making coerce_to_tuple depend on estate->err_func was > >pretty bizarre, and that there was no need for any "cache" at all for > >arrays that need only live as long as the function runs. All you are > >saving here is a palloc/pfree cycle, which is not worth the obscurantism > >and risk of bugs (are you sure natts can never change?). > > No, cache there is ugly. But I don't have idea about more efective > implementation of it :-(. First version of this patch was more clean. and > little bit slow. This cache save 10%. > > > > >BTW, if you want this patch to make it into 8.2, it needs to be fixed > >and resubmitted *very* soon. > > I understand, but I am not able work on it in next four days. And I need > help with it from Neil. It will be for 8.3. > > Thank you > Pavel > > _ > Emotikony a pozadi programu MSN Messenger ozivi vasi konverzaci. > http://messenger.msn.cz/ > > > ---(end of broadcast)--- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
OK, where are we on this patch? without changes. This task have to do anybody who better know PostgreSQL cache system than me. Regards Pavel --- Pavel Stehule wrote: > > > > > >"Pavel Stehule" <[EMAIL PROTECTED]> writes: > > >> This patch doesn't seem to cope with cases where the supplied tuple has > > >> the wrong number of columns, and it doesn't look like it's being > >careful > > >> about dropped columns either. Also, that's a mighty bizarre-looking > > >> choice of cache memory context in coerce_to_tuple ... but then again, > > >> why are you bothering with a cache at all for temporary arrays? > > > > > I am sorry, Tom. But I don't understand. I can check number of columns, > > > ofcourse and I'll do it. What cache for temporary arrays do you mean? > > > >I thought that making coerce_to_tuple depend on estate->err_func was > >pretty bizarre, and that there was no need for any "cache" at all for > >arrays that need only live as long as the function runs. All you are > >saving here is a palloc/pfree cycle, which is not worth the obscurantism > >and risk of bugs (are you sure natts can never change?). > > No, cache there is ugly. But I don't have idea about more efective > implementation of it :-(. First version of this patch was more clean. and > little bit slow. This cache save 10%. > > > > >BTW, if you want this patch to make it into 8.2, it needs to be fixed > >and resubmitted *very* soon. > > I understand, but I am not able work on it in next four days. And I need > help with it from Neil. It will be for 8.3. > > Thank you > Pavel > > _ > Emotikony a pozadi programu MSN Messenger ozivi vasi konverzaci. > http://messenger.msn.cz/ > > > ---(end of broadcast)--- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate _ Najdete si svou lasku a nove pratele na Match.com. http://www.msn.cz/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
Pavel Stehule wrote: > >OK, where are we on this patch? > > without changes. This task have to do anybody who better know PostgreSQL > cache system than me. How about you submit a version without any caching, but which works correctly; and we worry about optimizations later? > >--- > > > >Pavel Stehule wrote: > >> > >> > >> > > >> >"Pavel Stehule" <[EMAIL PROTECTED]> writes: > >> > >> This patch doesn't seem to cope with cases where the supplied tuple > >has > >> > >> the wrong number of columns, and it doesn't look like it's being > >> >careful > >> > >> about dropped columns either. Also, that's a mighty > >bizarre-looking > >> > >> choice of cache memory context in coerce_to_tuple ... but then > >again, > >> > >> why are you bothering with a cache at all for temporary arrays? > >> > > >> > > I am sorry, Tom. But I don't understand. I can check number of > >columns, > >> > > ofcourse and I'll do it. What cache for temporary arrays do you > >mean? > >> > > >> >I thought that making coerce_to_tuple depend on estate->err_func was > >> >pretty bizarre, and that there was no need for any "cache" at all for > >> >arrays that need only live as long as the function runs. All you are > >> >saving here is a palloc/pfree cycle, which is not worth the > >obscurantism > >> >and risk of bugs (are you sure natts can never change?). > >> > >> No, cache there is ugly. But I don't have idea about more efective > >> implementation of it :-(. First version of this patch was more clean. > >and > >> little bit slow. This cache save 10%. > >> > >> > > >> >BTW, if you want this patch to make it into 8.2, it needs to be fixed > >> >and resubmitted *very* soon. > >> > >> I understand, but I am not able work on it in next four days. And I need > >> help with it from Neil. It will be for 8.3. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] plpgsql, return can contains any expression
Pavel Stehule wrote: > >OK, where are we on this patch? > > without changes. This task have to do anybody who better know PostgreSQL > cache system than me. How about you submit a version without any caching, but which works correctly; and we worry about optimizations later? I can update and send simple version. Regards Pavel Stehule _ Najdete si svou lasku a nove pratele na Match.com. http://www.msn.cz/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq