Add generate_series(date, date) and generate_series(date, date, integer)
This patch addresses a personal need: nearly every time I use
generate_series for timestamps, I end up casting the result into date or
the ISO string thereof. Like such:
SELECT d.dt::date as dt
FROM generate_series('2015-01-01'::date,
'2016-01-04'::date,
interval '1 day') AS d(dt);
That's less than elegant.
With this patch, we can do this:
SELECT d.date_val FROM
generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
date_val
------------
1991-09-24
1991-09-25
1991-09-26
1991-09-27
1991-09-28
1991-09-29
1991-09-30
1991-10-01
(8 rows)
SELECT d.date_val FROM
generate_series('1991-09-24'::date,'1991-10-01'::date,7) as d(date_val);
date_val
------------
1991-09-24
1991-10-01
(2 rows)
SELECT d.date_val FROM
generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
date_val
------------
1999-12-31
1999-12-30
1999-12-29
(3 rows)
One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinity....it tries to do it. I didn't wait to
see if it finished.
For the date series, I put in checks to return an empty set:
SELECT d.date_val FROM
generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
date_val
----------
(0 rows)
SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date)
as d(date_val);
date_val
----------
(0 rows)
Notes:
- I borrowed the int4 implementation's check for step-size of 0 for POLA
reasons. However, it occurred to me that the function might be leakproof if
the behavior where changed to instead return an empty set. I'm not sure
that leakproof is a goal in and of itself.
First attempt at this patch attached. The examples above are copied from
the new test cases.
Attachments:
0001-add_generate_series_date.difftext/plain; charset=US-ASCII; name=0001-add_generate_series_date.diffDownload+189-0
On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker <corey.huinker@gmail.com> wrote:
This patch addresses a personal need: nearly every time I use
generate_series for timestamps, I end up casting the result into date or the
ISO string thereof. Like such:[...]
One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinity....it tries to do it. I didn't wait to
see if it finished.
Well, I would think that this is a bug that we had better address and
backpatch. It does not make much sense to use infinity for timestamps,
but letting it run infinitely is not good either.
For the date series, I put in checks to return an empty set:
SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date)
as d(date_val);
date_val
----------
(0 rows)SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date)
as d(date_val);
date_val
----------
(0 rows)
Wouldn't a proper error be more adapted?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier <michael.paquier@gmail.com> writes:
On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker <corey.huinker@gmail.com> wrote:
One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinity....it tries to do it. I didn't wait to
see if it finished.
Well, I would think that this is a bug that we had better address and
backpatch. It does not make much sense to use infinity for timestamps,
but letting it run infinitely is not good either.
Meh. Where would you cut it off? AD 10000000000? A few zeroes either
way doesn't really make much difference.
If it didn't respond to SIGINT, that would be an issue, but otherwise
this doesn't seem much more exciting than any other way to create a
query that will run longer than you want to wait.
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
If it didn't respond to SIGINT, that would be an issue, but otherwise
this doesn't seem much more exciting than any other way to create a
query that will run longer than you want to wait.regards, tom lane
It responded to SIGINT, so yeah, meh.
I can see value in aligning the behavior of infinity queries between date
and timestamp, but I have no strong opinion about which behavior is better:
it's either set step = 0 or an ereport(), no biggie if we want to handle
the condition, I rip out the DATE_NOT_FINITE() checks.
Incidentally, is there a reason behind the tendency of internal functions
to avoid parameter defaults in favor of multiple pg_proc entries? I copied
the existing behavior of the int4 generate_series, but having one entry
with the defaults seemed more self-documenting.
Corey Huinker <corey.huinker@gmail.com> writes:
Incidentally, is there a reason behind the tendency of internal functions
to avoid parameter defaults in favor of multiple pg_proc entries?
Yes: you can't specify defaults in pg_proc.h.
We work around that where absolutely necessary, see the last part of
system_views.sql. But it's messy enough, and bug-prone enough, to be
better avoided --- for example, it's very easy for the redeclaration
in system_view.sql to forget a STRICT or other similar marking.
Personally I'd say that every one of the existing cases that simply has
a default for the last argument was a bad idea, and would better have
been done in the traditional way with two pg_proc.h entries.
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 01/25/2016 07:22 AM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker <corey.huinker@gmail.com> wrote:
One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinity....it tries to do it. I didn't wait to
see if it finished.Well, I would think that this is a bug that we had better address and
backpatch. It does not make much sense to use infinity for timestamps,
but letting it run infinitely is not good either.Meh. Where would you cut it off? AD 10000000000? A few zeroes either
way doesn't really make much difference.
Why cut off? Why not to check if any of the input values is an infinity
and simply error out in that case?
If it didn't respond to SIGINT, that would be an issue, but
otherwise this doesn't seem much more exciting than any other way to
create a query that will run longer than you want to wait.
I disagree. Sure, it's possible to construct queries that take forever,
but that's difficult (or impossible) to detect at query start. I don't
think that means we should not guard against cases that are provably
infinite and can't possibly work.
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather
error out than wait till the end of the universe.
So +1 from me to checking for infinity.
regard
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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, Jan 25, 2016 at 2:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Corey Huinker <corey.huinker@gmail.com> writes:
Incidentally, is there a reason behind the tendency of internal functions
to avoid parameter defaults in favor of multiple pg_proc entries?Yes: you can't specify defaults in pg_proc.h.
We work around that where absolutely necessary, see the last part of
system_views.sql. But it's messy enough, and bug-prone enough, to be
better avoided --- for example, it's very easy for the redeclaration
in system_view.sql to forget a STRICT or other similar marking.Personally I'd say that every one of the existing cases that simply has
a default for the last argument was a bad idea, and would better have
been done in the traditional way with two pg_proc.h entries.regards, tom lane
...which answers my follow up question where make_interval was getting the
defaults I saw in the table but not the .h file. Thanks!
On Mon, Jan 25, 2016 at 6:55 PM, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
On 01/25/2016 07:22 AM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinity....it tries to do it. I didn't wait
to
see if it finished.Well, I would think that this is a bug that we had better address and
backpatch. It does not make much sense to use infinity for timestamps,
but letting it run infinitely is not good either.Meh. Where would you cut it off? AD 10000000000? A few zeroes either
way doesn't really make much difference.Why cut off? Why not to check if any of the input values is an infinity and
simply error out in that case?
That's exactly my point.
If it didn't respond to SIGINT, that would be an issue, but
otherwise this doesn't seem much more exciting than any other way to
create a query that will run longer than you want to wait.I disagree. Sure, it's possible to construct queries that take forever, but
that's difficult (or impossible) to detect at query start. I don't think
that means we should not guard against cases that are provably infinite and
can't possibly work.Imagine for example a script that in some rare cases passes happens to pass
infinity into generate_series() - in that case I'd much rather error out
than wait till the end of the universe.
Or wait until all the memory of the system is consumed. In the worst
case the OOM killer would come by, say 'Hi!' and do the police work,
but that's not cool either.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25 January 2016 at 09:55, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather error
out than wait till the end of the universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 26.01.2016 07:52, Simon Riggs wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather error
out than wait till the end of the universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support them in
(infinite) future? ;)
+1 from me to check infinity also.
Greetings,
Torsten
--
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, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
<mailinglists@toco-domains.de> wrote:
On 26.01.2016 07:52, Simon Riggs wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather error
out than wait till the end of the universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support them in
(infinite) future? ;)+1 from me to check infinity also.
Something like the patch attached would be fine? This wins a backpatch
because the query continuously running eats memory, no?
--
Michael
Attachments:
series-timestamp-inf-v1.patchtext/x-patch; charset=US-ASCII; name=series-timestamp-inf-v1.patchDownload+38-0
On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
<mailinglists@toco-domains.de> wrote:On 26.01.2016 07:52, Simon Riggs wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rathererror
out than wait till the end of the universe.
So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support them in
(infinite) future? ;)+1 from me to check infinity also.
Something like the patch attached would be fine? This wins a backpatch
because the query continuously running eats memory, no?
--
Michael--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I'll modify my generate_series_date to give similar errors.
I have no opinion on backpatch.
+1 for flippant references to infinity.
On Tue, Jan 26, 2016 at 09:53:26PM +0900, Michael Paquier wrote:
On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
<mailinglists@toco-domains.de> wrote:On 26.01.2016 07:52, Simon Riggs wrote:
Imagine for example a script that in some rare cases passes
happens to pass infinity into generate_series() - in that case
I'd much rather error out than wait till the end of the
universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support
them in (infinite) future? ;)+1 from me to check infinity also.
Something like the patch attached would be fine? This wins a
backpatch because the query continuously running eats memory, no?
+1 for back-patching. There's literally no case where an infinite
input could be correct as the start or end of an interval for
generate_series.
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
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
On 26-01-2016 09:53, Michael Paquier wrote:
Something like the patch attached would be fine? This wins a backpatch
because the query continuously running eats memory, no?
+1. Although it breaks compatibility, a function that just eats
resources is not correct.
--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Simon Riggs wrote:
On 25 January 2016 at 09:55, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather error
out than wait till the end of the universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
In the future we may get lazy evaluation of functions. When and if that
happens we may want to remove this limitation so that you can get a
streaming result producing timestamp values continuously.
(I don't necessarily think you'd use generate_series in such cases.
Maybe you'd want clock_timestamp to generate the present value at each
call, for example. But there may be uses for synthetic values as well.)
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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
Revised patch:
Differences in this patch vs my first one:
- infinite bounds generate errors identical to Michael's timestamp patch
(though I did like Simon's highly optimistic error message).
- Bounds checking moved before memory context allocation
- arg variable "finish" renamed "stop" to match parameter name in
documentation and error message
On Tue, Jan 26, 2016 at 12:47 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:
Show quoted text
On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier <
michael.paquier@gmail.com> wrote:On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
<mailinglists@toco-domains.de> wrote:On 26.01.2016 07:52, Simon Riggs wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rathererror
out than wait till the end of the universe.
So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support them in
(infinite) future? ;)+1 from me to check infinity also.
Something like the patch attached would be fine? This wins a backpatch
because the query continuously running eats memory, no?
--
Michael--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersI'll modify my generate_series_date to give similar errors.
I have no opinion on backpatch.
+1 for flippant references to infinity.
Attachments:
v2.generate_series_date.patchapplication/octet-stream; name=v2.generate_series_date.patchDownload+189-0
On 26.01.2016 13:53, Michael Paquier wrote:
Imagine for example a script that in some rare cases passes happens to
pass infinity into generate_series() - in that case I'd much rather error
out than wait till the end of the universe.So +1 from me to checking for infinity.
+1
ERROR infinite result sets are not supported, yet
Maybe we should skip the "yet". Or do we really plan to support them in
(infinite) future? ;)+1 from me to check infinity also.
Something like the patch attached would be fine? This wins a backpatch
because the query continuously running eats memory, no?
Looks fine to me.
(Minor mention: is there no newline needed between the tests?)
Greetings,
Torsten
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, failed
Everything looks good except for two minor issues:
1) There should be an informative comment for this struct:
+/* Corey BEGIN */
+typedef struct
+{
+ DateADT current;
+ DateADT stop;
+ int32 step;
+} generate_series_date_fctx;2) There's an extra linefeed after the struct. Needed?
Regards,
-David
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Doh, I left that comment to myself in there. :)
The corresponding structs in timestamp.c and int.c have no comment, so
suggestions of what should be there are welcome. In the interim I put in
this:
/* state for generate_series_date(date,date,[step]) */
Extra linefeed after struct removed.
Do you have any insight as to why the documentation test failed?
In the mean time, here's the updated patch.
On Tue, Feb 2, 2016 at 11:41 AM, David Steele <david@pgmasters.net> wrote:
Show quoted text
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, failedEverything looks good except for two minor issues:
1) There should be an informative comment for this struct:
+/* Corey BEGIN */ +typedef struct +{ + DateADT current; + DateADT stop; + int32 step; +} generate_series_date_fctx;2) There's an extra linefeed after the struct. Needed?
Regards,
-DavidThe new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
v3.generate_series_date.patchapplication/octet-stream; name=v3.generate_series_date.patchDownload+188-0
On 2/2/16 1:01 PM, Corey Huinker wrote:
Doh, I left that comment to myself in there. :)
If I had a dime for every time I've done that...
The corresponding structs in timestamp.c and int.c have no comment, so
suggestions of what should be there are welcome. In the interim I put in
this:/* state for generate_series_date(date,date,[step]) */
I think that's fine -- whoever commits it may feel otherwise.
Do you have any insight as to why the documentation test failed?
In the mean time, here's the updated patch.
I didn't pass the docs on account of the wonky comment since I consider
code comments to be part of the docs. The sgml docs build fine and look
good to me.
I'll retest and update the review accordingly.
--
-David
david@pgmasters.net
