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 |
Thread: | |
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 | Date | Subject | |
---|---|---|---|
Next Message | Dean Rasheed | 2008-07-10 12:17:04 | Re: Auto-explain patch |
Previous Message | Sushant Sinha | 2008-07-10 12:06:27 | initdb in current cvs head broken? |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2008-07-10 13:38:37 | Re: review: table function support |
Previous Message | Gregory Stark | 2008-07-10 10:52:43 | Re: WITH RECURSIVE updated to CVS TIP |