Thread: Re: review: table function support
Hello, I am sending actualized patch Regards Pavel Stehule 2008/7/9 Pavel Stehule <pavel.stehule@gmail.com>: > 2008/7/9 Marko Kreen <markokr@gmail.com>: >> Generally, the patch looks fine. There are few issues still: >> >> - plpgsql: the result columns _do_ create local variables. >> AIUI, they should not? > > it was my mistake - it doesn't do local variables - fixed >> >> - pg_dump: is the psql_assert() introduction necessary, considering it >> is used only in one place? > > removed - argmode variables is checked before >> >> - There should be regression test for plpgsql too, that test if >> the behaviour is correct. >> > > addeded >> - The documentation should mention behaviour difference from OUT >> parameters. > > I will do it. >> >> Wishlist (probably out of scope for this patch): > > this is in my wishlist too, but postgresql doesn't support types like > result of functions. >> >> - plpgsql: a way to create record variable for result row. Something like: >> >> CREATE FUNCTION foo(..) RETURNS TABLE (..) AS $$ >> DECLARE >> retval foo%ROWTYPE; >> >> >> Currently the OUT parameters are quite painful to use due to bad >> name resolving logic. Such feature would be perfect replacement. >> >> -- >> marko >> > I'll send patch early, thank you much > > Regards > Pavel Stehule >
Attachment
On 7/10/08, Pavel Stehule <pavel.stehule@gmail.com> wrote: > I am sending actualized patch > > Regards > Pavel Stehule > > 2008/7/9 Pavel Stehule <pavel.stehule@gmail.com>: > > > 2008/7/9 Marko Kreen <markokr@gmail.com>: > >> Generally, the patch looks fine. There are few issues still: > >> > >> - plpgsql: the result columns _do_ create local variables. > >> AIUI, they should not? > > > > it was my mistake - it doesn't do local variables - fixed > >> > >> - pg_dump: is the psql_assert() introduction necessary, considering it > >> is used only in one place? > > > > removed - argmode variables is checked before > >> > >> - There should be regression test for plpgsql too, that test if > >> the behaviour is correct. > >> > > > > addeded > >> - The documentation should mention behaviour difference from OUT > >> parameters. > > > > I will do it. > >> > >> Wishlist (probably out of scope for this patch): > > > > this is in my wishlist too, but postgresql doesn't support types like > > result of functions. > >> > >> - plpgsql: a way to create record variable for result row. Something like: > >> > >> CREATE FUNCTION foo(..) RETURNS TABLE (..) AS $$ > >> DECLARE > >> retval foo%ROWTYPE; > >> > >> > >> Currently the OUT parameters are quite painful to use due to bad > >> name resolving logic. Such feature would be perfect replacement. > >> > >> -- > >> marko > >> > > I'll send patch early, thank you much Ok, last items: - Attached is a patch that fixes couple C comments. - I think plpgsql 38.1.2 chapter of "Supported Argument and Result Data Types" should also have a mention of TABLE functions. Then I'm content with the patch. -- marko
Attachment
2008/7/10 Marko Kreen <markokr@gmail.com>: > On 7/10/08, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> I am sending actualized patch >> >> Regards >> Pavel Stehule >> >> 2008/7/9 Pavel Stehule <pavel.stehule@gmail.com>: >> >> > 2008/7/9 Marko Kreen <markokr@gmail.com>: >> >> Generally, the patch looks fine. There are few issues still: >> >> >> >> - plpgsql: the result columns _do_ create local variables. >> >> AIUI, they should not? >> > >> > it was my mistake - it doesn't do local variables - fixed >> >> >> >> - pg_dump: is the psql_assert() introduction necessary, considering it >> >> is used only in one place? >> > >> > removed - argmode variables is checked before >> >> >> >> - There should be regression test for plpgsql too, that test if >> >> the behaviour is correct. >> >> >> > >> > addeded >> >> - The documentation should mention behaviour difference from OUT >> >> parameters. >> > >> > I will do it. >> >> >> >> Wishlist (probably out of scope for this patch): >> > >> > this is in my wishlist too, but postgresql doesn't support types like >> > result of functions. >> >> >> >> - plpgsql: a way to create record variable for result row. Something like: >> >> >> >> CREATE FUNCTION foo(..) RETURNS TABLE (..) AS $$ >> >> DECLARE >> >> retval foo%ROWTYPE; >> >> >> >> >> >> Currently the OUT parameters are quite painful to use due to bad >> >> name resolving logic. Such feature would be perfect replacement. >> >> >> >> -- >> >> marko >> >> >> > I'll send patch early, thank you much > > Ok, last items: > > - Attached is a patch that fixes couple C comments. > > - I think plpgsql 38.1.2 chapter of "Supported Argument and Result Data > Types" should also have a mention of TABLE functions. > > Then I'm content with the patch. > applyed Regards and thank you very much Pavel > -- > marko >
Attachment
Added to September commit fest. --------------------------------------------------------------------------- Pavel Stehule wrote: > 2008/7/10 Marko Kreen <markokr@gmail.com>: > > On 7/10/08, Pavel Stehule <pavel.stehule@gmail.com> wrote: > >> I am sending actualized patch > >> > >> Regards > >> Pavel Stehule > >> > >> 2008/7/9 Pavel Stehule <pavel.stehule@gmail.com>: > >> > >> > 2008/7/9 Marko Kreen <markokr@gmail.com>: > >> >> Generally, the patch looks fine. There are few issues still: > >> >> > >> >> - plpgsql: the result columns _do_ create local variables. > >> >> AIUI, they should not? > >> > > >> > it was my mistake - it doesn't do local variables - fixed > >> >> > >> >> - pg_dump: is the psql_assert() introduction necessary, considering it > >> >> is used only in one place? > >> > > >> > removed - argmode variables is checked before > >> >> > >> >> - There should be regression test for plpgsql too, that test if > >> >> the behaviour is correct. > >> >> > >> > > >> > addeded > >> >> - The documentation should mention behaviour difference from OUT > >> >> parameters. > >> > > >> > I will do it. > >> >> > >> >> Wishlist (probably out of scope for this patch): > >> > > >> > this is in my wishlist too, but postgresql doesn't support types like > >> > result of functions. > >> >> > >> >> - plpgsql: a way to create record variable for result row. Something like: > >> >> > >> >> CREATE FUNCTION foo(..) RETURNS TABLE (..) AS $$ > >> >> DECLARE > >> >> retval foo%ROWTYPE; > >> >> > >> >> > >> >> Currently the OUT parameters are quite painful to use due to bad > >> >> name resolving logic. Such feature would be perfect replacement. > >> >> > >> >> -- > >> >> marko > >> >> > >> > I'll send patch early, thank you much > > > > Ok, last items: > > > > - Attached is a patch that fixes couple C comments. > > > > - I think plpgsql 38.1.2 chapter of "Supported Argument and Result Data > > Types" should also have a mention of TABLE functions. > > > > Then I'm content with the patch. > > > applyed > > Regards and thank you very much > > Pavel > > > -- > > marko > > [ Attachment, skipping... ] > > -- > Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +