[Fwd: SETOF input parameters (was Re: [HACKERS] proposal: array utility functions phase 1)]

Joe Conway <mail@joeconway.com> writes:

create or replace function test2() returns setof foot as 'select * from
foot order by 1 asc' language 'sql';
create or replace function test(setof foot) returns foot as 'select
$1.f1, $1.f2' language 'sql';

Uh, where exactly are you storing the information that the function
accepts a setof argument?

(We probably should be rejecting the above syntax at the moment, but
I suspect the parser just fails to notice the setof marker.)

A more serious objection is that this doesn't really address the
fundamental issue, namely that you can't drive a SRF from the results of
a query, except indirectly via single-purpose function definitions (like
test2() in your example).

I'm leaning more and more to the thought that we should reconsider the
Berkeley approach.

Another line of thought is to consider the possibilities of subselects
in the target list. For example,

SELECT ..., (SELECT ... FROM mysrf(a, b)) FROM foo WHERE ...;

I believe it's already the case that foo.a and foo.b can be transmitted
as arguments to mysrf() with this notation. The restriction is that the
sub-select can only return a single value (one row, one column) to the
outer query. It doesn't seem too outlandish to allow multiple columns
to be pulled up into the outer SELECT's result list given the above
syntax. I'm less sure about allowing multiple rows though. Any
thoughts?

regards, tom lane

Tom Lane wrote:

A more serious objection is that this doesn't really address the
fundamental issue, namely that you can't drive a SRF from the results of
a query, except indirectly via single-purpose function definitions (like
test2() in your example).

True enough. I've struggled trying to come up with a better way.

I'm leaning more and more to the thought that we should reconsider the
Berkeley approach.

The problem with the Berkley approach is what to do if there are two SRFs in
the target list.

Suppose

f(t1.x) returns:
1 a z
2 b y

and g(t2.y) returns:
3 q
5 w
7 e

and *without* the SRFs the query
select * from t1 join t2 on t1.id = t2.id;
would return:
id | x | id | y
------+------+------+------
4 | k | 4 | d
6 | v | 6 | u

What do we do for
select f(t1.x), g(t2.y), * from t1 join t2 on t1.id = t2.id;
?

Should we return 2 x 2 x 3 rows? Or do we impose a limit of 1 SRF in the
target list?

Another line of thought is to consider the possibilities of subselects
in the target list. For example,

SELECT ..., (SELECT ... FROM mysrf(a, b)) FROM foo WHERE ...;
I believe it's already the case that foo.a and foo.b can be transmitted
as arguments to mysrf() with this notation. The restriction is that the
sub-select can only return a single value (one row, one column) to the
outer query. It doesn't seem too outlandish to allow multiple columns
to be pulled up into the outer SELECT's result list given the above
syntax. I'm less sure about allowing multiple rows though.

This suffers from the same problem if there can be more than one subselect in
the target list (if multiple rows is allowed).

Any thoughts?

Is it too ugly to allow:
select ... from (select mysrf(foo.a, foo.b) from foo) as t;

where the Berkley syntax is restricted to where both are true:
1. a single target -- the srf
2. in a FROM clause subselect

In this case we could still use the column reference syntax too:
select ... from (select mysrf(foo.a, foo.b) from foo) as t(f1 int, f2 text);

But not allow the Berkley syntax for multi-row, multi-column SRFs otherwise.

What do you think?

Joe

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

I'm leaning more and more to the thought that we should reconsider the
Berkeley approach.

The problem with the Berkley approach is what to do if there are two SRFs in
the target list.

Agreed. The Berkeley code (or more accurately, the descendant code
that's in our source tree) generates the cross product of the rows
output by the SRFs, but I've never understood why that should be a good
approach to take. I could live with just rejecting multiple SRFs in the
same targetlist --- at least till someone comes up with a convincing
semantics for such a thing.

Is it too ugly to allow:
select ... from (select mysrf(foo.a, foo.b) from foo) as t;

where the Berkley syntax is restricted to where both are true:
1. a single target -- the srf
2. in a FROM clause subselect

Point 2 doesn't mean anything I think. Given your point 1 then the
select mysrf() ... is well-defined regardless of context.

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

The problem with the Berkley approach is what to do if there are two SRFs in
the target list.

Agreed. The Berkeley code (or more accurately, the descendant code
that's in our source tree) generates the cross product of the rows
output by the SRFs, but I've never understood why that should be a good
approach to take. I could live with just rejecting multiple SRFs in the
same targetlist --- at least till someone comes up with a convincing
semantics for such a thing.

I would like to start spending some time digging in to this. Any pointers or
thoughts on the best way to implement it? A little direction might save me
days of wheel spinning :-).

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

Agreed. The Berkeley code (or more accurately, the descendant code
that's in our source tree) generates the cross product of the rows
output by the SRFs, but I've never understood why that should be a good
approach to take. I could live with just rejecting multiple SRFs in the
same targetlist --- at least till someone comes up with a convincing
semantics for such a thing.

I would like to start spending some time digging in to this. Any pointers or
thoughts on the best way to implement it? A little direction might save me
days of wheel spinning :-).

Implement what exactly?

The code that presently does the dirty work is in ExecTargetList(), if
that's what you're looking for...

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

Agreed. The Berkeley code (or more accurately, the descendant code
that's in our source tree) generates the cross product of the rows
output by the SRFs, but I've never understood why that should be a good
approach to take. I could live with just rejecting multiple SRFs in the
same targetlist --- at least till someone comes up with a convincing
semantics for such a thing.

I would like to start spending some time digging in to this. Any pointers or
thoughts on the best way to implement it? A little direction might save me
days of wheel spinning :-).

Implement what exactly?

Well, I want to allow a single table function (or srf if you prefer) in the
target list as discussed above.

Currently, when you try it, record_out() gets called from printtup() when the
srf is encountered, which generates an ERROR. The behavior in 7.2.x is to
return a pointer when the composite type is output. I think that to make this
work as discussed, the target list needs to be "expanded" for the composite
type (similar to expanding a "*" I would think), so I was starting to look at
transformTargetList() and ExpandAllTables().

The code that presently does the dirty work is in ExecTargetList(), if
that's what you're looking for...

OK -- i'll check that out too.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

Implement what exactly?

Well, I want to allow a single table function (or srf if you prefer) in the
target list as discussed above.

Currently, when you try it, record_out() gets called from printtup() when the
srf is encountered, which generates an ERROR.

Oh, you're thinking about the multi-column aspect of it, not the
multi-row aspect. You really ought to keep those strictly separate;
their design and implementation problems are quite different IMHO.
I find it quite confusing to refer to both cases as "SRFs".

I think there once was support for multi-column function results in
SELECT; at least that's the best understanding I can muster of the old
"fjoin" code. But that's been broken since the Postgres95 SQL rewrite
(if not earlier), and I finally ripped out the last shreds of it just a
couple weeks ago. I don't think it could have been revived short of a
complete rewrite anyway, so I'm not shedding a tear. You could look at
old releases to see how it worked, though you might have to go back to
Postgres 4.2 or even before to find something that "worked".

My thought for implementation would not be to revive Fjoin as it was;
there are just too many places that deal with targetlists to make it
practical to have an alternative targetlist datastructure for this.
(The reason Fjoin has been too broken to contemplate reviving for all
these years is exactly that there were too many places that had never
coped with it.) I'd think more about adding a level of projection
(probably embedded in a Result plan node) that expands out the SRF tuple
result into individual columns.

Before that, though, you'd better put forward a workable user interface
for this; I'd wonder in particular what the names of the expanded-out
columns will be, and whether they'd be accessible from places that can
normally see output column names (such as ORDER BY). And what if a
multi-column function appears in the targetlist of a sub-SELECT?

On the implementation level, I think you will need to face up to the
problem of allowing a tuple value to be embedded as a column of a larger
tuple. It might be possible to avoid that, but I think it will take
some monstrous kluges to do so. (The existing pretend-a-TupleTableSlot-
pointer-is-a-Datum approach is already a monstrous kluge, not to mention
a source of memory leaks.) Once you've fixed that, the existing
FieldSelect expression node probably is all the run-time mechanism
needed to support the projection step I suggested above.

regards, tom lane

(moved from PATCHES back to HACKERS)

Tom Lane wrote:

Oh, you're thinking about the multi-column aspect of it, not the
multi-row aspect. You really ought to keep those strictly separate;
their design and implementation problems are quite different IMHO.
I find it quite confusing to refer to both cases as "SRFs".

[...snip...]

Before that, though, you'd better put forward a workable user interface
for this; I'd wonder in particular what the names of the expanded-out
columns will be, and whether they'd be accessible from places that can
normally see output column names (such as ORDER BY). And what if a
multi-column function appears in the targetlist of a sub-SELECT?

I've put some thought into *two* proposals for how targetlist functions should
behave -- one for a function that returns multiple rows, and one for a
function that returns multiple columns. The need for this was highlighted
recently when I submitted a proposal for array utility functions; see:
http://archives.postgresql.org/pgsql-hackers/2002-12/msg00461.php

At this point I don't have a clear idea how the latter would be implemented
(or if it even *can be* implemented with reasonable effort), but I wanted to
try to get agreement on the interface behavior before getting too caught up in
how to make it work. I think the former is reasonably straightforward (but
could well be wrong).

This is fairly long, so if you're not interested please delete now and accept
my apologies :-)

Proposals are below. Thoughts?

Thanks,

Joe

=================================================================
User interface proposal for multi-row function targetlist entries
=================================================================
1. Only one targetlist entry may return a set.
2. Each targetlist item (other than the set returning one) is
repeated for each item in the returned set.

Examples illustrating the need (these work on cvs HEAD):

CREATE TABLE bar(f1 int, f2 text, f3 int);
INSERT INTO bar VALUES(1, 'Hello', 42);
INSERT INTO bar VALUES(2, 'Happy', 45);

CREATE TABLE foo(a int, b text);
INSERT INTO foo VALUES(42, 'World');
INSERT INTO foo VALUES(42, 'Everyone');
INSERT INTO foo VALUES(45, 'Birthday');
INSERT INTO foo VALUES(45, 'New Year');

CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF text AS '
SELECT b FROM foo WHERE a = $1
' language 'sql';

regression=# SELECT f1, f2, getfoo(f3) AS f4 FROM bar;
f1 | f2 | f4
----+-------+----------
1 | Hello | World
1 | Hello | Everyone
2 | Happy | Birthday
2 | Happy | New Year
(4 rows)

Note that this is exatly how things currently work, i.e. there
is no restriction to the number of set returning targetlist entries.
This lack of restriction leads to strange and unexpected results (at
least IMHO). Continuing the example:

CREATE TABLE foo2(a int, b text);
INSERT INTO foo2 VALUES(42, '!!!!');
INSERT INTO foo2 VALUES(42, '????');
INSERT INTO foo2 VALUES(42, '####');
INSERT INTO foo2 VALUES(45, '$$$$');

CREATE OR REPLACE FUNCTION getfoo2(int) RETURNS SETOF text AS '
SELECT b FROM foo2 WHERE a = $1
' language 'sql';

Now, what *should* the following return if we allow multiple set
returning functions in the targetlist? Here's what it currently
does:

regression=# SELECT f1, f2, getfoo(f3) AS f4, getfoo2(f3) AS f5 FROM bar;
f1 | f2 | f4 | f5
----+-------+----------+------
1 | Hello | World | !!!!
1 | Hello | Everyone | ????
1 | Hello | World | ####
1 | Hello | Everyone | !!!!
1 | Hello | World | ????
1 | Hello | Everyone | ####
2 | Happy | Birthday | $$$$
2 | Happy | New Year | $$$$
(8 rows)

Not very useful as there is no way to prevent the apparent cartesian
join. But now try:

TRUNCATE TABLE foo2;
INSERT INTO foo2 VALUES(42, '!!!!');
INSERT INTO foo2 VALUES(42, '????');
INSERT INTO foo2 VALUES(45, '####');
INSERT INTO foo2 VALUES(45, '$$$$');

regression=# SELECT f1, f2, getfoo(f3) AS f4, getfoo2(f3) AS f5 FROM bar;
f1 | f2 | f4 | f5
----+-------+----------+------
1 | Hello | World | !!!!
1 | Hello | Everyone | ????
2 | Happy | Birthday | ####
2 | Happy | New Year | $$$$
(4 rows)

Hmmm, what happened to that cartesian join?

Under the proposal the two previous scenarios are disallowed with an ERROR.

============================================================================
User interface proposal for multi-column function targetlist entries
============================================================================
1. One, or more, targetlist entries may be a multi-column (composite) type.
2. For functions declared to return a named composite type, the
column names and types are as prescribed by the type unless overridden
in an alias definition.
3. For functions declared to return a "record" type, a column
definition list would be required as an alias at runtime.
4. Any alias provided for a composite returning function must match
the number of columns returned, and types if provided.
5. The composite function column names would be accessible from
places that can normally see output column names (such as ORDER BY).
6. When a composite function appears in the targetlist of a sub-SELECT,
the function's columns should be available outside the sub-SELECT in
the same manner as the other targetlist entries in the sub-SELECT.

Examples (these are all contrived):

CREATE TABLE bar(f1 int, f2 int);
INSERT INTO bar VALUES(1, 2);
INSERT INTO bar VALUES(2, 3);

CREATE TABLE foo(a int, b text);
INSERT INTO foo VALUES(1, 'a');
INSERT INTO foo VALUES(2, 'b');
INSERT INTO foo VALUES(3, 'c');
INSERT INTO foo VALUES(4, 'd');

CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF foo AS '
SELECT * FROM foo WHERE a = $1
' language 'sql';

SELECT f1, getfoo(f2) AS f3 FROM bar;
ERROR: function getfoo alias does not match its declared type

SELECT f1, getfoo(f2) AS (f3, f4) FROM bar;
f1 | f3 | f4
----+----+-----
1 | 2 | b
2 | 3 | c
(2 rows)

SELECT f1, getfoo(f2) AS (f3 int, f4 text) FROM bar;
f1 | f3 | f4
----+----+-----
1 | 2 | b
2 | 3 | c
(2 rows)

SELECT f1, getfoo(f2) FROM bar;
f1 | a | b
----+---+-----
1 | 2 | b
2 | 3 | c
(2 rows)

DROP FUNCTION getfoo(int);
CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF record AS '
SELECT * FROM foo WHERE a = $1
' language 'sql';

SELECT f1, getfoo(f2) AS f3 FROM bar;
ERROR: A column definition list is required for functions returning RECORD

SELECT f1, getfoo(f2) AS (f3 int, f4 text) FROM bar;
f1 | f3 | f4
----+----+-----
1 | 2 | b
2 | 3 | c
(2 rows)

Joe Conway wrote:

=================================================================
User interface proposal for multi-row function targetlist entries
=================================================================
1. Only one targetlist entry may return a set.
2. Each targetlist item (other than the set returning one) is
repeated for each item in the returned set.

Having gotten no objections (actually, no response at all), I can only assume
no one had heartburn with this change. The attached patch covers the first of
the two proposals, i.e. restricting the target list to only one set returning
function.

It compiles cleanly, and passes all regression tests. If there are no
objections, please apply.

Any suggestions on where this should be documented (other than maybe sql-select)?

Thanks,

Joe

p.s. Here's what the previous example now looks like:
CREATE TABLE bar(f1 int, f2 text, f3 int);
INSERT INTO bar VALUES(1, 'Hello', 42);
INSERT INTO bar VALUES(2, 'Happy', 45);

CREATE TABLE foo(a int, b text);
INSERT INTO foo VALUES(42, 'World');
INSERT INTO foo VALUES(42, 'Everyone');
INSERT INTO foo VALUES(45, 'Birthday');
INSERT INTO foo VALUES(45, 'New Year');

CREATE TABLE foo2(a int, b text);
INSERT INTO foo2 VALUES(42, '!!!!');
INSERT INTO foo2 VALUES(42, '????');
INSERT INTO foo2 VALUES(42, '####');
INSERT INTO foo2 VALUES(45, '$$$$');

CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF text AS '
SELECT b FROM foo WHERE a = $1
' language 'sql';

CREATE OR REPLACE FUNCTION getfoo2(int) RETURNS SETOF text AS '
SELECT b FROM foo2 WHERE a = $1
' language 'sql';

regression=# SELECT f1, f2, getfoo(f3) AS f4 FROM bar;
f1 | f2 | f4
----+-------+----------
1 | Hello | World
1 | Hello | Everyone
2 | Happy | Birthday
2 | Happy | New Year
(4 rows)

regression=# SELECT f1, f2, getfoo(f3) AS f4, getfoo2(f3) AS f5 FROM bar;
ERROR: Only one target list entry may return a set result

Joe Conway writes:

=================================================================
User interface proposal for multi-row function targetlist entries
=================================================================
1. Only one targetlist entry may return a set.
2. Each targetlist item (other than the set returning one) is
repeated for each item in the returned set.

Since we have set functions in the FROM list, what do set functions in the
target list give us?

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

Since we have set functions in the FROM list, what do set functions in the
target list give us?

The full discussion is a bit long, but it starts here:
http://archives.postgresql.org/pgsql-hackers/2002-12/msg00461.php
and picks up again here:
http://archives.postgresql.org/pgsql-patches/2002-12/msg00166.php

The short answer is we need a way to allow a "table function" to fire
multiple times given one or more columns from a table as input.

Joe

Joe Conway writes:

The short answer is we need a way to allow a "table function" to fire
multiple times given one or more columns from a table as input.

Has there been an evaluation of the SQL standard and other databases how
this is handled? I can't believe we're operating in ground-breaking
territory here. What concerns me is that we're inserting table-generating
syntax elements into the select list, which is where they've never
belonged.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

I can't believe we're operating in ground-breaking
territory here.

We're not. This functionality has always been in Postgres, right back
to the PostQUEL days. Joe is trying to clean it up to the point where
it has explainable, defensible semantics. But he's not adding something
that wasn't there before.

What concerns me is that we're inserting table-generating
syntax elements into the select list, which is where they've never
belonged.

Do you see another way to pass non-constant arguments to the
table-generating function?

regards, tom lane

Tom Lane writes:

What concerns me is that we're inserting table-generating
syntax elements into the select list, which is where they've never
belonged.

Do you see another way to pass non-constant arguments to the
table-generating function?

SELECT * FROM table1 AS t1, table2 AS t2, func1(t1.col, t2.col) ... ?

That's a syntax that would make sense to me.

With sufficiently blurred vision one might even find SQL99's clause

<collection derived table> ::=
UNNEST <left paren> <collection value expression> <right paren>

<table primary> ::=
<table or query name> [ [ AS ] <correlation name>
[ <left paren> <derived column list> <right paren> ] ]
| <derived table> [ AS ] <correlation name>
[ <left paren> <derived column list> <right paren> ]
| <lateral derived table> [ AS ] <correlation name>
[ <left paren> <derived column list> <right paren> ]
| <collection derived table> [ AS ] <correlation name>
[ <left paren> <derived column list> <right paren> ]
| <only spec>
[ [ AS ] <correlation name>
[ <left paren> <derived column list> <right paren> ] ]
| <left paren> <joined table> <right paren>

applicable. Or maybe not.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

Tom Lane writes:

Do you see another way to pass non-constant arguments to the
table-generating function?

SELECT * FROM table1 AS t1, table2 AS t2, func1(t1.col, t2.col) ... ?

That's a syntax that would make sense to me.

That syntax makes no sense whatsoever to me. You are imputing a
causal connection between FROM elements that are at the same level,
which is just totally contrary to any sane understanding of SQL
semantics. Exactly which t1.col value(s) do you see the above syntax
as passing to the func()? Your answer had better not mention the
WHERE clause, because the input tables have to be determined before
WHERE has anything to operate on.

With sufficiently blurred vision one might even find SQL99's clause
<collection derived table> ::=
UNNEST <left paren> <collection value expression> <right paren>
applicable. Or maybe not.

Hm. I'm not sure what UNNEST does, but now that you bring SQL99 into
the picture, what about WITH? That might solve the problem, because
(I think) WITH tables are logically determined before the main SELECT
begins to execute.

regards, tom lane

Tom Lane writes:

With sufficiently blurred vision one might even find SQL99's clause
<collection derived table> ::=
UNNEST <left paren> <collection value expression> <right paren>
applicable. Or maybe not.

Hm. I'm not sure what UNNEST does, but now that you bring SQL99 into
the picture, what about WITH? That might solve the problem, because
(I think) WITH tables are logically determined before the main SELECT
begins to execute.

The SQL 200x draft defines a new clause TABLE ( <collection value
expression> ) as a possible <table primary>, where the <collection value
expression> is required to be a function call. At the end this just boils
down to UNNEST, though. UNNEST is defined in terms of a hairy recursive
join subquery with a LATERAL( ) around it. LATERAL(subquery) is the same
as just (subquery) except that the scope clauses are different. So I
think this is probably what we ought to look at.

--
Peter Eisentraut peter_e@gmx.net

I wrote:

The SQL 200x draft defines a new clause TABLE ( <collection value
expression> ) as a possible <table primary>, where the <collection value
expression> is required to be a function call. At the end this just boils
down to UNNEST, though. UNNEST is defined in terms of a hairy recursive
join subquery with a LATERAL( ) around it. LATERAL(subquery) is the same
as just (subquery) except that the scope clauses are different. So I
think this is probably what we ought to look at.

I have stared at this some more and it is indeed what we're looking for.
The hairy recursive join is only so that they can get the WITH ORDINALITY
feature (which basically adds a "line number" column to the output) in
there in a closed form. If you simplify it, the command

SELECT ... FROM TABLE( func(...) ) ...

resolves to

SELECT ... FROM table_generated_by_func ...

As for the question of where nonconstant argument values come from, this
is addressed as well. The general form of this feature is the lateral
derived table, for example

SELECT ... FROM table1, LATERAL(select ...), table2 ...

as opposed to simply

SELECT ... FROM table1, (select ...), table2 ...

In the second form the subquery cannot access outside values. In the
first form, the subquery can access range variables in FROM items defined
to its left.

The table function calls are a special case of this, meaning that in

SELECT .. FROM table1, TABLE( func(args) ), table2 ...

the "args" can refer to table1 but not to table2.

Oracle implements exactly this feature. (See
<http://download-west.oracle.com/docs/cd/B10501_01/server.920/a96540/statements_103a.htm#2104992&gt;.)
If there are doubts about the semantics we could try it out there.

--
Peter Eisentraut peter_e@gmx.net

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Joe Conway wrote:

Joe Conway wrote:

=================================================================
User interface proposal for multi-row function targetlist entries
=================================================================
1. Only one targetlist entry may return a set.
2. Each targetlist item (other than the set returning one) is
repeated for each item in the returned set.

Having gotten no objections (actually, no response at all), I can only assume
no one had heartburn with this change. The attached patch covers the first of
the two proposals, i.e. restricting the target list to only one set returning
function.

It compiles cleanly, and passes all regression tests. If there are no
objections, please apply.

Any suggestions on where this should be documented (other than maybe sql-select)?

Thanks,

Joe

p.s. Here's what the previous example now looks like:
CREATE TABLE bar(f1 int, f2 text, f3 int);
INSERT INTO bar VALUES(1, 'Hello', 42);
INSERT INTO bar VALUES(2, 'Happy', 45);

CREATE TABLE foo(a int, b text);
INSERT INTO foo VALUES(42, 'World');
INSERT INTO foo VALUES(42, 'Everyone');
INSERT INTO foo VALUES(45, 'Birthday');
INSERT INTO foo VALUES(45, 'New Year');

CREATE TABLE foo2(a int, b text);
INSERT INTO foo2 VALUES(42, '!!!!');
INSERT INTO foo2 VALUES(42, '????');
INSERT INTO foo2 VALUES(42, '####');
INSERT INTO foo2 VALUES(45, '$$$$');

CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF text AS '
SELECT b FROM foo WHERE a = $1
' language 'sql';

CREATE OR REPLACE FUNCTION getfoo2(int) RETURNS SETOF text AS '
SELECT b FROM foo2 WHERE a = $1
' language 'sql';

regression=# SELECT f1, f2, getfoo(f3) AS f4 FROM bar;
f1 | f2 | f4
----+-------+----------
1 | Hello | World
1 | Hello | Everyone
2 | Happy | Birthday
2 | Happy | New Year
(4 rows)

regression=# SELECT f1, f2, getfoo(f3) AS f4, getfoo2(f3) AS f5 FROM bar;
ERROR: Only one target list entry may return a set result

Index: src/backend/parser/parse_clause.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_clause.c,v
retrieving revision 1.103
diff -c -r1.103 parse_clause.c
*** src/backend/parser/parse_clause.c	16 Dec 2002 18:39:22 -0000	1.103
--- src/backend/parser/parse_clause.c	12 Jan 2003 19:23:57 -0000
***************
*** 1121,1127 ****
* the end of the target list.	This target is given resjunk = TRUE so
* that it will not be projected into the final tuple.
*/
! 	target_result = transformTargetEntry(pstate, node, expr, NULL, true);
lappend(tlist, target_result);

return target_result;
--- 1121,1127 ----
* the end of the target list.	This target is given resjunk = TRUE so
* that it will not be projected into the final tuple.
*/
! 	target_result = transformTargetEntry(pstate, node, expr, NULL, true, NULL);
lappend(tlist, target_result);

return target_result;
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_target.c,v
retrieving revision 1.94
diff -c -r1.94 parse_target.c
*** src/backend/parser/parse_target.c	12 Dec 2002 20:35:13 -0000	1.94
--- src/backend/parser/parse_target.c	12 Jan 2003 19:25:16 -0000
***************
*** 42,54 ****
* colname	the column name to be assigned, or NULL if none yet set.
* resjunk	true if the target should be marked resjunk, ie, it is not
*			wanted in the final projected tuple.
*/
TargetEntry *
transformTargetEntry(ParseState *pstate,
Node *node,
Node *expr,
char *colname,
! 					 bool resjunk)
{
Oid			type_id;
int32		type_mod;
--- 42,57 ----
* colname	the column name to be assigned, or NULL if none yet set.
* resjunk	true if the target should be marked resjunk, ie, it is not
*			wanted in the final projected tuple.
+  * retset	if non-NULL, and the entry is a function expression, pass back
+  *			expr->funcretset
*/
TargetEntry *
transformTargetEntry(ParseState *pstate,
Node *node,
Node *expr,
char *colname,
! 					 bool resjunk,
! 					 bool *retset)
{
Oid			type_id;
int32		type_mod;
***************
*** 61,66 ****
--- 64,72 ----
if (IsA(expr, RangeVar))
elog(ERROR, "You can't use relation names alone in the target list, try relation.*.");

+ 	if (retset && IsA(expr, FuncExpr))
+ 		*retset = ((FuncExpr *) expr)->funcretset;
+ 
type_id = exprType(expr);
type_mod = exprTypmod(expr);

***************
*** 93,102 ****
--- 99,110 ----
List *
transformTargetList(ParseState *pstate, List *targetlist)
{
+ 	bool		retset = false;
List	   *p_target = NIL;

while (targetlist != NIL)
{
+ bool entry_retset = false;
ResTarget *res = (ResTarget *) lfirst(targetlist);

if (IsA(res->val, ColumnRef))
***************
*** 173,179 ****
res->val,
NULL,
res->name,
! 														false));
}
}
else if (IsA(res->val, InsertDefault))
--- 181,188 ----
res->val,
NULL,
res->name,
! 														false,
! 														&entry_retset));
}
}
else if (IsA(res->val, InsertDefault))
***************
*** 194,201 ****
res->val,
NULL,
res->name,
! 													false));
}

targetlist = lnext(targetlist);
}
--- 203,217 ----
res->val,
NULL,
res->name,
! 													false,
! 													&entry_retset));
}
+ 
+ 		if (retset && entry_retset)
+ 			elog(ERROR, "Only one target list entry may return a set result");
+ 
+ 		if (entry_retset)
+ 			retset = true;

targetlist = lnext(targetlist);
}
Index: src/include/parser/parse_target.h
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/include/parser/parse_target.h,v
retrieving revision 1.27
diff -c -r1.27 parse_target.h
*** src/include/parser/parse_target.h	18 Sep 2002 21:35:24 -0000	1.27
--- src/include/parser/parse_target.h	12 Jan 2003 19:08:56 -0000
***************
*** 20,26 ****
extern List *transformTargetList(ParseState *pstate, List *targetlist);
extern TargetEntry *transformTargetEntry(ParseState *pstate,
Node *node, Node *expr,
! 					 char *colname, bool resjunk);
extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle,
char *colname, int attrno,
List *indirection);
--- 20,26 ----
extern List *transformTargetList(ParseState *pstate, List *targetlist);
extern TargetEntry *transformTargetEntry(ParseState *pstate,
Node *node, Node *expr,
! 					 char *colname, bool resjunk, bool *retset);
extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle,
char *colname, int attrno,
List *indirection);

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Your patch has been added to the PostgreSQL unapplied patches list at:

This patch was objected to by Peter, IIRC, and I think I agree with him.
We should look at whether we can't solve the problem via SQL99 features
before pumping new life into that crufty old Berkeley syntax.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Your patch has been added to the PostgreSQL unapplied patches list at:

This patch was objected to by Peter, IIRC, and I think I agree with him.
We should look at whether we can't solve the problem via SQL99 features
before pumping new life into that crufty old Berkeley syntax.

I know I haven't had time to absorb Peter's suggestions and comment, but I
think the current behavior is broken, and this patch should be applied anyway
(this was only yhe first half of my proposal -- i.e. prevent more than one
targetlist srf). The only reason I can think to not apply it, is if you think
we should completely disallow targetlist set returning functions as part of
moving to SQL99.

Joe

Joe Conway <mail@joeconway.com> writes:

The only reason I can think to not apply it, is if you think
we should completely disallow targetlist set returning functions as part of
moving to SQL99.

I would like to eventually get rid of targetlist SRF's altogether.
I believe the feature represents a nontrivial drag on executor
performance and reliability even when it's not being used. (Look at all
the isDone cruft in execQual, the TupFromTlist hoop-jumping, the places
that are missing TupFromTlist hoop-jumping and should have it, etc.)
Obviously we can't do that until we have a fully functional alternative,
which FROM-list SRF's aren't. But if there is a chance of getting rid
of them via SQL99 extensions then I'd like to pursue that direction.

In the meantime, I don't see any need to spend any effort on cleaning up
what we're likely to discard altogether later...

regards, tom lane

Patch applied. Thanks.

---------------------------------------------------------------------------

Joe Conway wrote:

Joe Conway wrote:

=================================================================
User interface proposal for multi-row function targetlist entries
=================================================================
1. Only one targetlist entry may return a set.
2. Each targetlist item (other than the set returning one) is
repeated for each item in the returned set.

Having gotten no objections (actually, no response at all), I can only assume
no one had heartburn with this change. The attached patch covers the first of
the two proposals, i.e. restricting the target list to only one set returning
function.

It compiles cleanly, and passes all regression tests. If there are no
objections, please apply.

Any suggestions on where this should be documented (other than maybe sql-select)?

Thanks,

Joe

p.s. Here's what the previous example now looks like:
CREATE TABLE bar(f1 int, f2 text, f3 int);
INSERT INTO bar VALUES(1, 'Hello', 42);
INSERT INTO bar VALUES(2, 'Happy', 45);

CREATE TABLE foo(a int, b text);
INSERT INTO foo VALUES(42, 'World');
INSERT INTO foo VALUES(42, 'Everyone');
INSERT INTO foo VALUES(45, 'Birthday');
INSERT INTO foo VALUES(45, 'New Year');

CREATE TABLE foo2(a int, b text);
INSERT INTO foo2 VALUES(42, '!!!!');
INSERT INTO foo2 VALUES(42, '????');
INSERT INTO foo2 VALUES(42, '####');
INSERT INTO foo2 VALUES(45, '$$$$');

CREATE OR REPLACE FUNCTION getfoo(int) RETURNS SETOF text AS '
SELECT b FROM foo WHERE a = $1
' language 'sql';

CREATE OR REPLACE FUNCTION getfoo2(int) RETURNS SETOF text AS '
SELECT b FROM foo2 WHERE a = $1
' language 'sql';

regression=# SELECT f1, f2, getfoo(f3) AS f4 FROM bar;
f1 | f2 | f4
----+-------+----------
1 | Hello | World
1 | Hello | Everyone
2 | Happy | Birthday
2 | Happy | New Year
(4 rows)

regression=# SELECT f1, f2, getfoo(f3) AS f4, getfoo2(f3) AS f5 FROM bar;
ERROR: Only one target list entry may return a set result

Index: src/backend/parser/parse_clause.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_clause.c,v
retrieving revision 1.103
diff -c -r1.103 parse_clause.c
*** src/backend/parser/parse_clause.c	16 Dec 2002 18:39:22 -0000	1.103
--- src/backend/parser/parse_clause.c	12 Jan 2003 19:23:57 -0000
***************
*** 1121,1127 ****
* the end of the target list.	This target is given resjunk = TRUE so
* that it will not be projected into the final tuple.
*/
! 	target_result = transformTargetEntry(pstate, node, expr, NULL, true);
lappend(tlist, target_result);

return target_result;
--- 1121,1127 ----
* the end of the target list.	This target is given resjunk = TRUE so
* that it will not be projected into the final tuple.
*/
! 	target_result = transformTargetEntry(pstate, node, expr, NULL, true, NULL);
lappend(tlist, target_result);

return target_result;
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_target.c,v
retrieving revision 1.94
diff -c -r1.94 parse_target.c
*** src/backend/parser/parse_target.c	12 Dec 2002 20:35:13 -0000	1.94
--- src/backend/parser/parse_target.c	12 Jan 2003 19:25:16 -0000
***************
*** 42,54 ****
* colname	the column name to be assigned, or NULL if none yet set.
* resjunk	true if the target should be marked resjunk, ie, it is not
*			wanted in the final projected tuple.
*/
TargetEntry *
transformTargetEntry(ParseState *pstate,
Node *node,
Node *expr,
char *colname,
! 					 bool resjunk)
{
Oid			type_id;
int32		type_mod;
--- 42,57 ----
* colname	the column name to be assigned, or NULL if none yet set.
* resjunk	true if the target should be marked resjunk, ie, it is not
*			wanted in the final projected tuple.
+  * retset	if non-NULL, and the entry is a function expression, pass back
+  *			expr->funcretset
*/
TargetEntry *
transformTargetEntry(ParseState *pstate,
Node *node,
Node *expr,
char *colname,
! 					 bool resjunk,
! 					 bool *retset)
{
Oid			type_id;
int32		type_mod;
***************
*** 61,66 ****
--- 64,72 ----
if (IsA(expr, RangeVar))
elog(ERROR, "You can't use relation names alone in the target list, try relation.*.");

+ 	if (retset && IsA(expr, FuncExpr))
+ 		*retset = ((FuncExpr *) expr)->funcretset;
+ 
type_id = exprType(expr);
type_mod = exprTypmod(expr);

***************
*** 93,102 ****
--- 99,110 ----
List *
transformTargetList(ParseState *pstate, List *targetlist)
{
+ 	bool		retset = false;
List	   *p_target = NIL;

while (targetlist != NIL)
{
+ bool entry_retset = false;
ResTarget *res = (ResTarget *) lfirst(targetlist);

if (IsA(res->val, ColumnRef))
***************
*** 173,179 ****
res->val,
NULL,
res->name,
! 														false));
}
}
else if (IsA(res->val, InsertDefault))
--- 181,188 ----
res->val,
NULL,
res->name,
! 														false,
! 														&entry_retset));
}
}
else if (IsA(res->val, InsertDefault))
***************
*** 194,201 ****
res->val,
NULL,
res->name,
! 													false));
}

targetlist = lnext(targetlist);
}
--- 203,217 ----
res->val,
NULL,
res->name,
! 													false,
! 													&entry_retset));
}
+ 
+ 		if (retset && entry_retset)
+ 			elog(ERROR, "Only one target list entry may return a set result");
+ 
+ 		if (entry_retset)
+ 			retset = true;

targetlist = lnext(targetlist);
}
Index: src/include/parser/parse_target.h
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/include/parser/parse_target.h,v
retrieving revision 1.27
diff -c -r1.27 parse_target.h
*** src/include/parser/parse_target.h	18 Sep 2002 21:35:24 -0000	1.27
--- src/include/parser/parse_target.h	12 Jan 2003 19:08:56 -0000
***************
*** 20,26 ****
extern List *transformTargetList(ParseState *pstate, List *targetlist);
extern TargetEntry *transformTargetEntry(ParseState *pstate,
Node *node, Node *expr,
! 					 char *colname, bool resjunk);
extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle,
char *colname, int attrno,
List *indirection);
--- 20,26 ----
extern List *transformTargetList(ParseState *pstate, List *targetlist);
extern TargetEntry *transformTargetEntry(ParseState *pstate,
Node *node, Node *expr,
! 					 char *colname, bool resjunk, bool *retset);
extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle,
char *colname, int attrno,
List *indirection);

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Patch applied. Thanks.

This was *not* agreed to, please revert it.

regards, tom lane

OK, patch reverted.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Patch applied. Thanks.

This was *not* agreed to, please revert it.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073