Lists: | pgsql-hackerspgsql-patches |
---|
From: | "Marko Kreen" <markokr(at)gmail(dot)com> |
---|---|
To: | "Postgres Hackers" <pgsql-hackers(at)postgresql(dot)org> |
Cc: | "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com> |
Subject: | review: table function support |
Date: | 2008-07-09 11:05:03 |
Message-ID: | e51f66da0807090405gd5ad4c3h892986181cf09d7a@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Generally, the patch looks fine. There are few issues still:
- plpgsql: the result columns _do_ create local variables.
AIUI, they should not?
- pg_dump: is the psql_assert() introduction necessary, considering it
is used only in one place?
- There should be regression test for plpgsql too, that test if
the behaviour is correct.
- The documentation should mention behaviour difference from OUT
parameters.
Wishlist (probably out of scope for this patch):
- 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
From: | "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | "Marko Kreen" <markokr(at)gmail(dot)com> |
Cc: | pgsql-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: review: table function support |
Date: | 2008-07-10 08:58:18 |
Message-ID: | 162867790807100158j6461ee76g639367c0547ec3bf@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Hello,
I am sending actualized patch
Regards
Pavel Stehule
2008/7/9 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> 2008/7/9 Marko Kreen <markokr(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
tabfce1.1.diff | text/x-patch | 25.9 KB |
From: | "Marko Kreen" <markokr(at)gmail(dot)com> |
---|---|
To: | "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | pgsql-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: review: table function support |
Date: | 2008-07-10 12:10:39 |
Message-ID: | e51f66da0807100510r3694cf63ubffaef81657502cc@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
On 7/10/08, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> I am sending actualized patch
>
> Regards
> Pavel Stehule
>
> 2008/7/9 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>
> > 2008/7/9 Marko Kreen <markokr(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
tbl.comments.diff | text/x-patch | 1.0 KB |
From: | "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | "Marko Kreen" <markokr(at)gmail(dot)com> |
Cc: | pgsql-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: review: table function support |
Date: | 2008-07-10 13:38:37 |
Message-ID: | 162867790807100638n47c00eb4w304473a247a20aa7@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
2008/7/10 Marko Kreen <markokr(at)gmail(dot)com>:
> On 7/10/08, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> I am sending actualized patch
>>
>> Regards
>> Pavel Stehule
>>
>> 2008/7/9 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>>
>> > 2008/7/9 Marko Kreen <markokr(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
tabfce1.2.diff | text/x-patch | 25.9 KB |
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Marko Kreen <markokr(at)gmail(dot)com>, pgsql-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: review: table function support |
Date: | 2008-08-23 03:24:31 |
Message-ID: | 200808230324.m7N3OVL07504@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Added to September commit fest.
---------------------------------------------------------------------------
Pavel Stehule wrote:
> 2008/7/10 Marko Kreen <markokr(at)gmail(dot)com>:
> > On 7/10/08, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> >> I am sending actualized patch
> >>
> >> Regards
> >> Pavel Stehule
> >>
> >> 2008/7/9 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> >>
> >> > 2008/7/9 Marko Kreen <markokr(at)gmail(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +