UNNEST with multiple args, and TABLE with multiple funcs
Summary:
This patch implements a method for expanding multiple SRFs in parallel
that does not have the surprising LCM behaviour of SRFs-in-select-list.
(Functions returning fewer rows are padded with nulls instead.)
It then uses this method combined with a parse-time hack to implement
the (intended to be) spec-conforming behaviour of UNNEST with multiple
parameters, including flattening of composite results.
The upshot is that given a table like this:
postgres=# select * from t1;
a | b | c
---------------+-------------------+----------------------------------------------
{11,12,13} | {wombat} |
{5,10} | {foo,bar} | {"(123,xyzzy)","(456,plugh)","(789,plover)"}
{21,31,41,51} | {fred,jim,sheila} | {"(111,xyzzy)","(222,plugh)"}
(3 rows)
(where column "c" is an array of a composite type with 2 cols, "x" and "y")
You can do this:
postgres=# select u.* from t1, unnest(a,b,c) with ordinality as u;
?column? | ?column? | x | y | ordinality
----------+----------+-----+--------+------------
11 | wombat | | | 1
12 | | | | 2
13 | | | | 3
5 | foo | 123 | xyzzy | 1
10 | bar | 456 | plugh | 2
| | 789 | plover | 3
21 | fred | 111 | xyzzy | 1
31 | jim | 222 | plugh | 2
41 | sheila | | | 3
51 | | | | 4
(10 rows)
Or for an example of general combination of functions:
postgres=# select * from table(generate_series(10,20,5), unnest(array['fred','jim']));
?column? | ?column?
----------+----------
10 | fred
15 | jim
20 |
(3 rows)
Implementation Details:
The spec syntax for table function calls, <table function derived table>
in <table reference>, looks like TABLE(func(args...)) AS ...
This patch implements that, plus an extension: it allows multiple
functions, TABLE(func1(...), func2(...), func3(...)) [WITH ORDINALITY]
and defines this as meaning that the functions are to be evaluated in
parallel.
This is implemented by changing RangeFunction, function RTEs, and the
FunctionScan node to take lists of function calls rather than a single
function. The calling convention for SRFs is completely unchanged; each
function returns its own rows (or a tuplestore in materialize mode) just
as before, and FunctionScan combines the results into a single output
tuple (keeping track of which functions are exhausted in order to
correctly fill in nulls on a backwards scan).
Then, a hack in the parser converts unnest(...) appearing as a
func_table (and only there) into a list of unnest() calls, one for each
parameter. So
select ... from unnest(a,b,c)
is converted to
select ... from TABLE(unnest(a),unnest(b),unnest(c))
and if unnest appears as part of an existing list inside TABLE(), it's
expanded to multiple entries there too.
This parser hackery is of course somewhat ugly. But given the objective
of implementing the spec's unnest syntax, it seems to be the least ugly
of the possible approaches. (The hard part of doing it any other way
would be generating the description of the result type; composite array
parameters expand into multiple result columns.)
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list. This should also
address the points that Josh brought up in discussion of ORDINALITY
regarding use of SRF-in-select to unnest multiple arrays.
(As a side issue, this patch also sets up pathkeys for ordinality along
the lines of a patch I suggested to Greg a while back in response to
his.)
Current patch status:
This is a first working cut: no docs, no tests, not enough comments, the
deparse logic probably needs more work (it deparses correctly but the
formatting may be suboptimal). However all the functionality is believed
to be in place.
--
Andrew (irc:RhodiumToad)
Attachments:
table-functions.patchtext/plain; charset=us-asciiDownload+1149-959
On 08/13/2013 06:54 AM, Andrew Gierth wrote:
Summary:
This patch implements a method for expanding multiple SRFs in parallel
that does not have the surprising LCM behaviour of SRFs-in-select-list.
(Functions returning fewer rows are padded with nulls instead.)
BTW, if anyone is unsure of the use-case for this, I have some uses for it:
1. denormalized data stored in same-length arrays (usually for
compression reasons)
2. use with PL/Python-Numpy and PL/R functions which return multiple
arrays or 2D arrays.
In other words, I have *lots* of uses for this functionality, and I
think the analytics crowd will like it. Which means that I need to get
on testing it, of course ...
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WMae4b0a9dc19d8a46bcd277b8a430661c5008151b6fb8eca9c56088135197e7ae5681ea64088042914b002cae35ee6618@asav-2.01.com
On 08/14/2013 08:22 AM, Josh Berkus wrote:
On 08/13/2013 06:54 AM, Andrew Gierth wrote:
Summary:
This patch implements a method for expanding multiple SRFs in parallel
that does not have the surprising LCM behaviour of SRFs-in-select-list.
(Functions returning fewer rows are padded with nulls instead.)BTW, if anyone is unsure of the use-case for this, I have some uses for it:
1. denormalized data stored in same-length arrays (usually for
compression reasons)2. use with PL/Python-Numpy and PL/R functions which return multiple
arrays or 2D arrays.In other words, I have *lots* of uses for this functionality, and I
think the analytics crowd will like it. Which means that I need to get
on testing it, of course ...
Similarly, I see uses for this come up a lot, and usually have to work
around it with ugly invocations of multiple SRFs in the SELECT list in a
subquery.
I was thinking of implementing multi-argument unnest directly with `any`
parameters if I could get it to work, but hadn't started on it yet.
This looks like a really clever approach and it handles multiple
spec-compliance items. I'll grab the patch and try it out.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
2013-08-13 15:54 keltez�ssel, Andrew Gierth �rta:
Summary:
This patch implements a method for expanding multiple SRFs in parallel
that does not have the surprising LCM behaviour of SRFs-in-select-list.
(Functions returning fewer rows are padded with nulls instead.)It then uses this method combined with a parse-time hack to implement
the (intended to be) spec-conforming behaviour of UNNEST with multiple
parameters, including flattening of composite results.The upshot is that given a table like this:
postgres=# select * from t1;
a | b | c
---------------+-------------------+----------------------------------------------
{11,12,13} | {wombat} |
{5,10} | {foo,bar} | {"(123,xyzzy)","(456,plugh)","(789,plover)"}
{21,31,41,51} | {fred,jim,sheila} | {"(111,xyzzy)","(222,plugh)"}
(3 rows)(where column "c" is an array of a composite type with 2 cols, "x" and "y")
You can do this:
postgres=# select u.* from t1, unnest(a,b,c) with ordinality as u;
?column? | ?column? | x | y | ordinality
----------+----------+-----+--------+------------
11 | wombat | | | 1
12 | | | | 2
13 | | | | 3
5 | foo | 123 | xyzzy | 1
10 | bar | 456 | plugh | 2
| | 789 | plover | 3
21 | fred | 111 | xyzzy | 1
31 | jim | 222 | plugh | 2
41 | sheila | | | 3
51 | | | | 4
(10 rows)Or for an example of general combination of functions:
postgres=# select * from table(generate_series(10,20,5), unnest(array['fred','jim']));
?column? | ?column?
----------+----------
10 | fred
15 | jim
20 |
(3 rows)Implementation Details:
The spec syntax for table function calls, <table function derived table>
in <table reference>, looks like TABLE(func(args...)) AS ...This patch implements that, plus an extension: it allows multiple
functions, TABLE(func1(...), func2(...), func3(...)) [WITH ORDINALITY]
and defines this as meaning that the functions are to be evaluated in
parallel.This is implemented by changing RangeFunction, function RTEs, and the
FunctionScan node to take lists of function calls rather than a single
function. The calling convention for SRFs is completely unchanged; each
function returns its own rows (or a tuplestore in materialize mode) just
as before, and FunctionScan combines the results into a single output
tuple (keeping track of which functions are exhausted in order to
correctly fill in nulls on a backwards scan).Then, a hack in the parser converts unnest(...) appearing as a
func_table (and only there) into a list of unnest() calls, one for each
parameter. Soselect ... from unnest(a,b,c)
is converted to
select ... from TABLE(unnest(a),unnest(b),unnest(c))
and if unnest appears as part of an existing list inside TABLE(), it's
expanded to multiple entries there too.This parser hackery is of course somewhat ugly. But given the objective
of implementing the spec's unnest syntax, it seems to be the least ugly
of the possible approaches. (The hard part of doing it any other way
would be generating the description of the result type; composite array
parameters expand into multiple result columns.)
Harder maybe but it may still be cleaner in the long run.
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.
This should also
address the points that Josh brought up in discussion of ORDINALITY
regarding use of SRF-in-select to unnest multiple arrays.(As a side issue, this patch also sets up pathkeys for ordinality along
the lines of a patch I suggested to Greg a while back in response to
his.)Current patch status:
This is a first working cut: no docs, no tests, not enough comments, the
deparse logic probably needs more work (it deparses correctly but the
formatting may be suboptimal). However all the functionality is believed
to be in place.
With this last paragraph in mind, I am trying a little review.
* Is the patch in a patch format which has context? (eg: context diff format)
Yes.
* Does it apply cleanly to the current git master?
Applies with some offsets on a few files but without fuzz.
* Does it include reasonable tests, necessary doc patches, etc?
No, as told by the patch author.
* Does the patch actually implement what it's supposed to do?
Yes.
* Do we want that?
Yes.
* Do we already have it?
No.
* Does it follow SQL spec, or the community-agreed behavior?
The SQL spec says these:
In 7.6 <table reference>
<table primary> ::=
...
| <table function derived table> [ AS ] <correlation name> [ <left paren> <derived column
list> <right paren> ]
also later in the same section:
<table function derived table> ::=
TABLE <left paren> <collection value expression> <right paren>
In 6.26 <value expression>
<collection value expression> ::=
<array value expression> | <multiset value expression>
In 6.36 <array value expression>
<array value expression> ::= <array concatenation> | <array primary>
<array concatenation> ::= <array value expression 1> <concatenation operator> <array primary>
<array value expression 1> ::= <array value expression>
<array primary> ::= <array value function> | <value expression primary>
6.3 <value expression primary>
<value expression primary> ::=
<parenthesized value expression>
| <nonparenthesized value expression primary>
<parenthesized value expression> ::= <left paren> <value expression> <right paren>
<nonparenthesized value expression primary> ::=
<unsigned value specification>
| <column reference>
| <set function specification>
| <window function>
| <nested window function>
| <scalar subquery>
| <case expression>
| <cast specification>
| <field reference>
| <subtype treatment>
| <method invocation>
| <static method invocation>
| <new specification>
| <attribute or method reference>
| <reference resolution>
| <collection value constructor>
| <array element reference>
| <multiset element reference>
| <next value expression>
| <routine invocation>
collection value constructor> ::=
| <array value constructor>
| <multiset value constructor>
So, the FROM TABLE(...) AS (...) syntax is a big can of worms and
I haven't even quoted <multiset value expression>.
As far as I can tell, these should also be allowed but isn't:
zozo=# select * from table('a'::text) as x;
ERROR: syntax error at or near "'a'"
LINE 1: select * from table('a'::text) as x;
^
zozo=# select x.* from t1, table(t1.a) as x;
ERROR: syntax error at or near ")"
LINE 1: select x.* from t1, table(t1.a) as x;
^
zozo=# select x.* from table((6)) as x(a int4);
ERROR: syntax error at or near "("
LINE 1: select x.* from table((6)) as x(a int4);
^
zozo=# select x.* from table(values (6)) as x(a int4);
ERROR: syntax error at or near "("
LINE 1: select x.* from table(values (6)) as x(a int4);
^
zozo=# select x.* from table(values(6)) as x(a int4);
ERROR: syntax error at or near "("
LINE 1: select x.* from table(values(6)) as x(a int4);
^
What the patch implements is only the last choice for
<nonparenthesized value expression primary>: <routine invocation>
When you add documentation, it would be nice to mention it.
Also, the grammar extension is a start for adding all the other
standard choices for TABLE().
* Does it include pg_dump support (if applicable)?
n/a
* Are there dangers?
I can't see any. 8-)
* Have all the bases been covered?
My previous comments about the TABLE() syntax says it all.
You can interpret it either way. :-)
* Does the feature work as advertised?
Yes.
* Are there corner cases the author has failed to consider?
I don't know.
* Are there any assertion failures or crashes?
No.
* Does the patch slow down simple tests?
No.
* If it claims to improve performance, does it?
It certainly improves writing queries, as functions inside
unnest() get processed in one scan.
* Does it slow down other things?
I don't think so.
* Does it follow the project coding guidelines?
Yes.
* Are there portability issues?
No.
* Will it work on Windows/BSD etc?
It should, the code uses standard internal PostgreSQL APIs
and extends them. No new system call.
* Are the comments sufficient and accurate?
According to the author, no.
* Does it do what it says, correctly?
Yes.
* Does it produce compiler warnings?
No.
* Can you make it crash?
No.
* Is everything done in a way that fits together coherently with other features/modules?
I think so
* Are there interdependencies that can cause problems?
I don't know.
Best regards,
Zolt�n B�sz�rm�nyi
--
----------------------------------
Zolt�n B�sz�rm�nyi
Cybertec Sch�nig & Sch�nig GmbH
Gr�hrm�hlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello
Harder maybe but it may still be cleaner in the long run.
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.
Although I would to remove SRF from targetlist, I don't think so this hurry
strategy is good idea. We should to provide new functionality and old
functionality one year as minimum, and we should to announce so this
feature is deprecated - and maybe use a GUC for disabling, warning and
deprecating. More, I would to see 9.4 release:). x.4 are happy PostgreSQL
releases :)
Regards
Pavel
On 08/19/2013 09:23 AM, Boszormenyi Zoltan wrote:
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.
That's not realistic. We'd have to deprecate that syntax and
repeatedly and loudly warn people about it for at least 3 years after
the release of 9.3. You're talking about asking people to refactor
hundreds or thousands of lines of code which makes current use of things
like regex_match() in the target list.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WMb5ad31c9b5bd6ad97e5b57b87f614846692f94a88c2f81b9508eef3ab287bdd890818105e624d645e0907e6379c8a60a@asav-1.01.com
2013-08-19 20:03 keltezéssel, Josh Berkus írta:
On 08/19/2013 09:23 AM, Boszormenyi Zoltan wrote:
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.That's not realistic.
I know. I am such an agent provocateur. :-)
We'd have to deprecate that syntax and
repeatedly and loudly warn people about it for at least 3 years after
the release of 9.3. You're talking about asking people to refactor
hundreds or thousands of lines of code which makes current use of things
like regex_match() in the target list.
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Boszormenyi Zoltan wrote:
This parser hackery is of course somewhat ugly. But given the objective
of implementing the spec's unnest syntax, it seems to be the least ugly
of the possible approaches. (The hard part of doing it any other way
would be generating the description of the result type; composite array
parameters expand into multiple result columns.)Harder maybe but it may still be cleaner in the long run.
I'm not so sure.
As far as I'm concerned, though, the situation is fairly simple: there
are no proposals on the table for any mechanism that would allow the
deduction of a return type structure for multi-arg unnest, I have
tried and failed to come up with a usable alternative proposal, and
there is no prospect of one resulting from any other work that I know
about. So the parser hack is the way it goes, and anyone who doesn't
like it is welcome to suggest a better idea.
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.
I want to make this ABSOLUTELY clear: I am not advocating removing
SRF-in-targetlist in the near future and I will not support anyone who
does. Please do not use this code as an argument for that (at least
until a few releases have elapsed). All I'm interested in at this
point is providing an alternative with better semantics.
The SQL spec says these:
In 7.6 <table reference>
[mega-snip]
As far as I can tell, these should also be allowed but isn't:
No, they're not allowed. You missed this rule (in 7.6 Syntax Rules):
2) If a <table primary> TP simply contains a <table function derived
table> TFDT, then:
a) The <collection value expression> immediately contained in TFDT
shall be a <routine invocation>.
In other words, func(...) is the only allowed form for the collection
value expression inside TABLE( ). (Same rule exists in 201x, but
numbered 6 rather than 2.)
Largely as a matter of consistency, the patch does presently allow
expressions that are not <routine invocation>s but which are part of
the func_expr_windowless production, so things like TABLE(USER) work.
(This is because historically these are allowed in the FROM clause as
tables.) I'm not sure this is a good idea in general; should it be
tightened up to only allow func_application?
* If it claims to improve performance, does it?
It certainly improves writing queries, as functions inside
unnest() get processed in one scan.
I'm not making any specific performance claims, but I have tested it
against the idea of doing separate function scans with a full join on
ordinality columns, and my approach is faster (1.5x to 2x in my tests)
even with pathkey support and with elimination of extra materialize
nodes (by allowing mark/restore in FunctionScan).
------
Since the original patch was posted I have done further work on it,
including some tests. I have also come up with an additional
possibility: that of allowing multiple SRFs that return RECORD with
column definition lists, and SRFs-returning-RECORD combined with
ORDINALITY, by extending the syntax further:
select * from TABLE(func1() AS (a text, b integer),
func2() AS (c integer, d text));
select * from TABLE(func1() AS (a text, b integer))
WITH ORDINALITY AS f(a,b,o);
-- shame to have to duplicate the column names, but avoiding that would
-- not have been easy
This removes the restriction of the previous ORDINALITY patch that
prevented its use with SRFs that needed coldef lists.
I'm open to other suggestions on the syntax for this.
(My implementation of this works by making the column definition list
a property of the function call, rather than of the RTE or the
FunctionScan node. This eliminates a few places where TYPEFUNC_RECORD
had to be handled as a special case.)
--
Andrew. (irc:RhodiumToad)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2013-08-19 22:04 keltez�ssel, Andrew Gierth �rta:
Boszormenyi Zoltan wrote:
This parser hackery is of course somewhat ugly. But given the objective
of implementing the spec's unnest syntax, it seems to be the least ugly
of the possible approaches. (The hard part of doing it any other way
would be generating the description of the result type; composite array
parameters expand into multiple result columns.)Harder maybe but it may still be cleaner in the long run.
I'm not so sure.
As far as I'm concerned, though, the situation is fairly simple: there
are no proposals on the table for any mechanism that would allow the
deduction of a return type structure for multi-arg unnest, I have
tried and failed to come up with a usable alternative proposal, and
there is no prospect of one resulting from any other work that I know
about. So the parser hack is the way it goes, and anyone who doesn't
like it is welcome to suggest a better idea.Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.I want to make this ABSOLUTELY clear: I am not advocating removing
SRF-in-targetlist in the near future and I will not support anyone who
does. Please do not use this code as an argument for that (at least
until a few releases have elapsed). All I'm interested in at this
point is providing an alternative with better semantics.The SQL spec says these:
In 7.6 <table reference>
[mega-snip]
As far as I can tell, these should also be allowed but isn't:
No, they're not allowed. You missed this rule (in 7.6 Syntax Rules):
2) If a <table primary> TP simply contains a <table function derived
table> TFDT, then:a) The <collection value expression> immediately contained in TFDT
shall be a <routine invocation>.In other words, func(...) is the only allowed form for the collection
value expression inside TABLE( ). (Same rule exists in 201x, but
numbered 6 rather than 2.)
You are right, I missed it in the standard. Sorry.
Largely as a matter of consistency, the patch does presently allow
expressions that are not <routine invocation>s but which are part of
the func_expr_windowless production, so things like TABLE(USER) work.
(This is because historically these are allowed in the FROM clause as
tables.) I'm not sure this is a good idea in general; should it be
tightened up to only allow func_application?* If it claims to improve performance, does it?
It certainly improves writing queries, as functions inside
unnest() get processed in one scan.I'm not making any specific performance claims, but I have tested it
against the idea of doing separate function scans with a full join on
ordinality columns, and my approach is faster (1.5x to 2x in my tests)
even with pathkey support and with elimination of extra materialize
nodes (by allowing mark/restore in FunctionScan).------
Since the original patch was posted I have done further work on it,
including some tests. I have also come up with an additional
possibility: that of allowing multiple SRFs that return RECORD with
column definition lists, and SRFs-returning-RECORD combined with
ORDINALITY, by extending the syntax further:select * from TABLE(func1() AS (a text, b integer),
func2() AS (c integer, d text));select * from TABLE(func1() AS (a text, b integer))
WITH ORDINALITY AS f(a,b,o);-- shame to have to duplicate the column names, but avoiding that would
-- not have been easyThis removes the restriction of the previous ORDINALITY patch that
prevented its use with SRFs that needed coldef lists.
Very nice.
I'm open to other suggestions on the syntax for this.
It's consistent with straight SRFs in FROM, the function
parameters are attached to the functions themselves.
I think it's good as is.
(My implementation of this works by making the column definition list
a property of the function call,
Which also makes it easier on the eyes and brain when reading
someone else's SQL.
rather than of the RTE or the
FunctionScan node. This eliminates a few places where TYPEFUNC_RECORD
had to be handled as a special case.)
This is the other plus. No special casing is good.
The only minus is that you haven't attached the new patch. :-)
Best regards,
Zolt�n B�sz�rm�nyi
--
----------------------------------
Zolt�n B�sz�rm�nyi
Cybertec Sch�nig & Sch�nig GmbH
Gr�hrm�hlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 08/20/2013 02:03 AM, Josh Berkus wrote:
On 08/19/2013 09:23 AM, Boszormenyi Zoltan wrote:
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.That's not realistic. We'd have to deprecate that syntax and
repeatedly and loudly warn people about it for at least 3 years after
the release of 9.3. You're talking about asking people to refactor
hundreds or thousands of lines of code which makes current use of things
like regex_match() in the target list.
Agreed. Even three years is optimistic; after that long it could
probably be made into an ERROR by default with a backward-compat GUC,
but certainly not removed.
I'm still running into people running 8.2 and having issues upgrading
due to the 8.3 removal of implicit casts from text, and even the removal
of add_missing_from .
If we want people to upgrade this century it's worth minimising the
amount of unnecessary breakage. SRF-in-SELECT might be ugly, but simply
ripping it out certainly counts as unnecessary breakage.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Aug 19, 2013 at 07:45:23PM +0200, Pavel Stehule wrote:
Hello
Harder maybe but it may still be cleaner in the long run.
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.Although I would to remove SRF from targetlist, I don't think so this hurry
strategy is good idea. We should to provide new functionality and old
functionality one year as minimum, and we should to announce so this
feature is deprecated
We could do this in 9.3, but all it would be is an announcement, i.e.
no code change of any nature.
- and maybe use a GUC for disabling, warning and deprecating.
With utmost respect, I think the general idea of setting SQL grammar
via GUC is a really bad one. When we've done so in the past, it's
done more harm than good, and we should not repeat it.
More, I would to see 9.4 release:).
Same here! :)
x.4 are happy PostgreSQL releases :)
Each one has been at least baseline happy for me since 7.1. Some have
made me overjoyed, though.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2013/8/20 David Fetter <david@fetter.org>
On Mon, Aug 19, 2013 at 07:45:23PM +0200, Pavel Stehule wrote:
Hello
Harder maybe but it may still be cleaner in the long run.
Overall, it's my intention here to remove as many as feasible of the
old
reasons why one might use an SRF in the select list.
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.Although I would to remove SRF from targetlist, I don't think so this
hurry
strategy is good idea. We should to provide new functionality and old
functionality one year as minimum, and we should to announce so this
feature is deprecatedWe could do this in 9.3, but all it would be is an announcement, i.e.
no code change of any nature.- and maybe use a GUC for disabling, warning and deprecating.
With utmost respect, I think the general idea of setting SQL grammar
via GUC is a really bad one. When we've done so in the past, it's
done more harm than good, and we should not repeat it.
so as minumum is controlling warning via GUC, we should to help with
identification of problematic queries.
Regards
Pavel
Show quoted text
More, I would to see 9.4 release:).
Same here! :)
x.4 are happy PostgreSQL releases :)
Each one has been at least baseline happy for me since 7.1. Some have
made me overjoyed, though.Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.icsRemember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
2013-08-20 08:13 keltezéssel, Pavel Stehule írta:
2013/8/20 David Fetter <david@fetter.org <mailto:david@fetter.org>>
On Mon, Aug 19, 2013 at 07:45:23PM +0200, Pavel Stehule wrote:
Hello
Harder maybe but it may still be cleaner in the long run.
Overall, it's my intention here to remove as many as feasible of the old
reasons why one might use an SRF in the select list.
Indeed, it's a big nail in the coffin for SRFs-in-targetlist. Having
WITH ORDINALITY and this feature, I would vote for removing
SRF-in-targetlist and call the release PostgreSQL 10.0.Although I would to remove SRF from targetlist, I don't think so this hurry
strategy is good idea. We should to provide new functionality and old
functionality one year as minimum, and we should to announce so this
feature is deprecatedWe could do this in 9.3, but all it would be is an announcement, i.e.
no code change of any nature.- and maybe use a GUC for disabling, warning and deprecating.
To really ensure backward compatibility, this sentence should read as
"add a GUC for disabling *the* warning and deprecating." :->
As I said, I am such an agent provocateur.
Let this side track die and concentrate on the merits of the patch itself. :-)
With utmost respect, I think the general idea of setting SQL grammar
via GUC is a really bad one. When we've done so in the past, it's
done more harm than good, and we should not repeat it.so as minumum is controlling warning via GUC, we should to help with identification of
problematic queries.Regards
Pavel
More, I would to see 9.4 release:).
Same here! :)
x.4 are happy PostgreSQL releases :)
Each one has been at least baseline happy for me since 7.1. Some have
made me overjoyed, though.Cheers,
David.
--
David Fetter <david@fetter.org <mailto:david@fetter.org>> http://fetter.org/
Phone: +1 415 235 3778 <tel:%2B1%20415%20235%203778> AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com <mailto:david.fetter@gmail.com>
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
<http://www.tripit.com/feed/ical/people/david74/tripit.ics>Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
On Tue, 2013-08-13 at 13:54 +0000, Andrew Gierth wrote:
Summary:
This patch implements a method for expanding multiple SRFs in parallel
that does not have the surprising LCM behaviour of SRFs-in-select-list.
(Functions returning fewer rows are padded with nulls instead.)
Fails to build in contrib:
pg_stat_statements.c -MMD -MP -MF .deps/pg_stat_statements.Po
pg_stat_statements.c: In function ‘JumbleRangeTable’:
pg_stat_statements.c:1459:27: error: ‘RangeTblEntry’ has no member named ‘funcexpr’
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Latest version of patch. This should be it as far as code goes; there
may be some more regression test work, and a doc patch will be
forthcoming.
This version supports, in addition to the previous stuff:
SELECT ... FROM TABLE(func() AS (colname coltype, ...));
i.e. the column definition list required for functions that return
arbitrary RECORD results can go inside the TABLE() construct. This
allows more than one such function in a call:
SELECT ... FROM TABLE(func1() AS (a integer), func2() AS (b text));
or mixing RECORD functions with ORDINALITY:
SELECT ... FROM TABLE(func1() AS (c text)) WITH ORDINALITY;
The existing FROM func() AS f(c text) is still supported of course,
and the variation FROM TABLE(func()) AS f(c text) is also supported
but only when there's exactly one function and no ORDINALITY.
Other changes:
- function dependence on executor parameters is now tracked
per-function, so that on rescan, only affected functions are
re-executed, and others are simply rescanned from the existing
tuplestore
- some cases where deparse or other code broke because an element
of funcexprs was not actually a FuncExpr have been fixed
- fixed the pg_stat_statements issue
A change I _didn't_ include, but did test, was adding mark/restore to
FunctionScan to allow mergejoins on ordinality columns to work without
needing extra nodes (which I did to do some performance tests referred
to in a previous message). I took this code back out because it didn't
seem to make much difference: the planner often (not always) adds the
Materialize node even when it's not needed, in the belief that it is
faster; the overhead of the extra node doesn't seem serious; and the
case is of limited applicability (only useful when joining against
something other than a function using the ordinal column alone).
--
Andrew (irc:RhodiumToad)
Attachments:
table-functions-2.patchtext/plain; charset=us-asciiDownload+2393-1940
2013-08-27 01:24 keltez�ssel, Andrew Gierth �rta:
Latest version of patch. This should be it as far as code goes; there
may be some more regression test work, and a doc patch will be
forthcoming.This version supports, in addition to the previous stuff:
[snip]
In my limited testing, it works well, and the patch looks good.
When adding regression tests, can you please add intentional
syntax error cases to exercise all the new ereport()s?
Best regards,
Zolt�n B�sz�rm�nyi
--
----------------------------------
Zolt�n B�sz�rm�nyi
Cybertec Sch�nig & Sch�nig GmbH
Gr�hrm�hlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Boszormenyi Zoltan <zb@cybertec.at> writes:
When adding regression tests, can you please add intentional
syntax error cases to exercise all the new ereport()s?
Please do not add test cases merely to prove that. Yeah, you should
probably have exercised each error case in devel testing, but that does
not mean that every future run of the regression tests needs to do it too.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, 2013-08-27 at 09:44 -0400, Tom Lane wrote:
Boszormenyi Zoltan <zb@cybertec.at> writes:
When adding regression tests, can you please add intentional
syntax error cases to exercise all the new ereport()s?Please do not add test cases merely to prove that. Yeah, you should
probably have exercised each error case in devel testing, but that does
not mean that every future run of the regression tests needs to do it too.
I disagree. The next person who wants to hack on this feature should be
given the confidence that he's not breaking behavior that the last guy
put in.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, 2013-08-26 at 23:24 +0000, Andrew Gierth wrote:
Latest version of patch. This should be it as far as code goes; there
may be some more regression test work, and a doc patch will be
forthcoming.
In src/include/optimizer/paths.h, you are using "operator" as a function
argument name, which breaks cpluspluscheck. Try using opid or
operatorid.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Aug 28, 2013 at 12:09:05AM -0400, Peter Eisentraut wrote:
On Tue, 2013-08-27 at 09:44 -0400, Tom Lane wrote:
Boszormenyi Zoltan <zb@cybertec.at> writes:
When adding regression tests, can you please add intentional
syntax error cases to exercise all the new ereport()s?Please do not add test cases merely to prove that. Yeah, you should
probably have exercised each error case in devel testing, but that does
not mean that every future run of the regression tests needs to do it too.I disagree. The next person who wants to hack on this feature should be
given the confidence that he's not breaking behavior that the last guy
put in.
+1. I wouldn't make full error-outcome test coverage a condition of patch
acceptance. However, when an author chooses to submit high-quality tests with
that level of detail, our source tree is the place to archive them. I share
Tom's desire for a Makefile target that completes quickly and checks only
those behaviors most likely to break, but not at the cost of letting deep test
coverage dissipate in a mailing list attachment or in the feature author's
home directory.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
