review: table function support

Started by Marko Kreenalmost 18 years ago5 messageshackers

Jump to latest

Marko Kreen

markokr@gmail.com

almost 18 years ago

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

Pavel Stehule

pavel.stehule@gmail.com

almost 18 years ago

In reply to: Marko Kreen (#1)

Re: review: table function support

Hello,

I am sending actualized patch

Regards
Pavel Stehule

2008/7/9 Pavel Stehule <pavel.stehule@gmail.com>:

Show quoted text

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

Import Notes

Reply to msg id not found: 162867790807090838k1265ff70r1f312b26e7c4fefa@mail.gmail.com

Marko Kreen

markokr@gmail.com

almost 18 years ago

In reply to: Pavel Stehule (#2)

Re: review: table function support

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

Pavel Stehule

pavel.stehule@gmail.com

almost 18 years ago

In reply to: Marko Kreen (#3)

Re: review: table function support

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

Show quoted text

--
marko

Bruce Momjian

bruce@momjian.us

over 17 years ago

In reply to: Pavel Stehule (#4)

Re: review: table function support

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. +

review: table function support

Attachments:

Attachments:

Attachments: