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
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9c143b2..15ebe47 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14657,6 +14657,26 @@ AND
</entry>
</row>
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of one day
+ </entry>
+ </row>
+
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter>
+ </entry>
+ </row>
+
</tbody>
</tgroup>
</table>
@@ -14721,6 +14741,26 @@ SELECT * FROM generate_series('2008-03-01 00:00'::timestamp,
2008-03-03 22:00:00
2008-03-04 08:00:00
(9 rows)
+
+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)
</programlisting>
</para>
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 332db7e..7404a2f 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -30,6 +30,7 @@
#include "utils/datetime.h"
#include "utils/nabstime.h"
#include "utils/sortsupport.h"
+#include "funcapi.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -2811,3 +2812,97 @@ timetz_izone(PG_FUNCTION_ARGS)
PG_RETURN_TIMETZADT_P(result);
}
+
+/* Corey BEGIN */
+typedef struct
+{
+ DateADT current;
+ DateADT finish;
+ int32 step;
+} generate_series_date_fctx;
+
+
+/* generate_series_date()
+ * Generate the set of dates from start to finish by step
+ */
+Datum
+generate_series_date(PG_FUNCTION_ARGS)
+{
+ return generate_series_step_date(fcinfo);
+}
+
+Datum
+generate_series_step_date(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ generate_series_date_fctx *fctx;
+ DateADT result;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ DateADT start = PG_GETARG_DATEADT(0);
+ DateADT finish = PG_GETARG_DATEADT(1);
+ int32 step = 1;
+
+ /* see if we were given an explicit step size */
+ if (PG_NARGS() == 3)
+ step = PG_GETARG_INT32(2);
+ if (step == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("step size cannot equal zero")));
+
+ MemoryContext oldcontext;
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* allocate memory for user context */
+ fctx = (generate_series_date_fctx *)
+ palloc(sizeof(generate_series_date_fctx));
+
+ /*
+ * Use fctx to keep state from call to call. Seed current with the
+ * original start value
+ */
+ fctx->current = start;
+ fctx->finish = finish;
+ /* cannot iterate infinity */
+ if (DATE_NOT_FINITE(start) || DATE_NOT_FINITE(finish))
+ fctx->step = 0;
+ else
+ fctx->step = step;
+
+ funcctx->user_fctx = fctx;
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * get the saved state and use current as the result for this iteration
+ */
+ fctx = funcctx->user_fctx;
+ result = fctx->current;
+
+ if ((fctx->step > 0 && fctx->current <= fctx->finish) ||
+ (fctx->step < 0 && fctx->current >= fctx->finish))
+ {
+ /* increment current in preparation for next iteration */
+ fctx->current += fctx->step;
+
+ /* do when there is more left to send */
+ SRF_RETURN_NEXT(funcctx, DateADTGetDatum(result));
+ }
+ else
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
+
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 79e92ff..65293eb 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4013,6 +4013,11 @@ DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t
DESCR("non-persistent series generator");
DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ ));
DESCR("non-persistent series generator");
+DATA(insert OID = 2739 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1082 "1082 1082 23" _null_ _null_ "{start,finish,step}" _null_ _null_ generate_series_step_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+DATA(insert OID = 2740 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 2 0 1082 "1082 1082" _null_ _null_ "{start,finish}" _null_ _null_ generate_series_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+
/* boolean aggregates */
DATA(insert OID = 2515 ( booland_statefunc PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "16 16" _null_ _null_ _null_ _null_ _null_ booland_statefunc _null_ _null_ _null_ ));
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 1b962af..f427cde 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -205,4 +205,7 @@ extern Datum timetz_izone(PG_FUNCTION_ARGS);
extern Datum timetz_pl_interval(PG_FUNCTION_ARGS);
extern Datum timetz_mi_interval(PG_FUNCTION_ARGS);
+extern Datum generate_series_date(PG_FUNCTION_ARGS);
+extern Datum generate_series_step_date(PG_FUNCTION_ARGS);
+
#endif /* DATE_H */
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 56c5520..427f3df 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1452,3 +1452,42 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SET datestyle TO iso;
+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)
+
+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)
+
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index e40b4c4..b790e32 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -340,3 +340,10 @@ select make_date(2013, 11, -1);
select make_date(-44, 3, 15); -- perhaps we should allow this sometime?
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+
+SET datestyle TO iso;
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date,7) as d(date_val);
+SELECT d.date_val FROM generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
+SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date) as d(date_val);
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
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 1525d2a..2f9e8d0 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5405,6 +5405,16 @@ generate_series_timestamp(PG_FUNCTION_ARGS)
MemoryContext oldcontext;
Interval interval_zero;
+ /* handle infinite in start and stop values */
+ if (TIMESTAMP_NOT_FINITE(start))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("start value cannot be infinite")));
+ if (TIMESTAMP_NOT_FINITE(finish))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("stop value cannot be infinite")));
+
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
@@ -5486,6 +5496,16 @@ generate_series_timestamptz(PG_FUNCTION_ARGS)
MemoryContext oldcontext;
Interval interval_zero;
+ /* handle infinite in start and stop values */
+ if (TIMESTAMP_NOT_FINITE(start))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("start value cannot be infinite")));
+ if (TIMESTAMP_NOT_FINITE(finish))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("stop value cannot be infinite")));
+
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index e9f5e77..ac708e2 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -1592,3 +1592,8 @@ SELECT make_timestamp(2014,12,28,6,30,45.887);
Sun Dec 28 06:30:45.887 2014
(1 row)
+-- Tests for generate_series
+SELECT generate_series(now(), 'infinity'::timestamp, interval '1 hour');
+ERROR: stop value cannot be infinite
+SELECT generate_series('infinity'::timestamp, now(), interval '1 hour');
+ERROR: start value cannot be infinite
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index 40a2254..3ec7ddb 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -2487,3 +2487,8 @@ SELECT '2007-12-09 07:30:00 UTC'::timestamptz AT TIME ZONE 'VET';
Sun Dec 09 03:00:00 2007
(1 row)
+-- Tests for generate_series
+SELECT generate_series(now(), 'infinity'::timestamp, interval '1 hour');
+ERROR: stop value cannot be infinite
+SELECT generate_series('infinity'::timestamp, now(), interval '1 hour');
+ERROR: start value cannot be infinite
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index b22cd48..898d9cb 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -225,3 +225,7 @@ SELECT '' AS to_char_11, to_char(d1, 'FMIYYY FMIYY FMIY FMI FMIW FMIDDD FMID')
-- timestamp numeric fields constructor
SELECT make_timestamp(2014,12,28,6,30,45.887);
+
+-- Tests for generate_series
+SELECT generate_series(now(), 'infinity'::timestamp, interval '1 hour');
+SELECT generate_series('infinity'::timestamp, now(), interval '1 hour');
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index f4b455e..b565452 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -432,3 +432,7 @@ SELECT '2007-12-09 07:00:00 UTC'::timestamptz AT TIME ZONE 'VET';
SELECT '2007-12-09 07:00:01 UTC'::timestamptz AT TIME ZONE 'VET';
SELECT '2007-12-09 07:29:59 UTC'::timestamptz AT TIME ZONE 'VET';
SELECT '2007-12-09 07:30:00 UTC'::timestamptz AT TIME ZONE 'VET';
+
+-- Tests for generate_series
+SELECT generate_series(now(), 'infinity'::timestamp, interval '1 hour');
+SELECT generate_series('infinity'::timestamp, now(), interval '1 hour');
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
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9c143b2..15ebe47 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14657,6 +14657,26 @@ AND
</entry>
</row>
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of one day
+ </entry>
+ </row>
+
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter>
+ </entry>
+ </row>
+
</tbody>
</tgroup>
</table>
@@ -14721,6 +14741,26 @@ SELECT * FROM generate_series('2008-03-01 00:00'::timestamp,
2008-03-03 22:00:00
2008-03-04 08:00:00
(9 rows)
+
+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)
</programlisting>
</para>
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 332db7e..2b0cda5 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -30,6 +30,7 @@
#include "utils/datetime.h"
#include "utils/nabstime.h"
#include "utils/sortsupport.h"
+#include "funcapi.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -2811,3 +2812,103 @@ timetz_izone(PG_FUNCTION_ARGS)
PG_RETURN_TIMETZADT_P(result);
}
+
+/* Corey BEGIN */
+typedef struct
+{
+ DateADT current;
+ DateADT stop;
+ int32 step;
+} generate_series_date_fctx;
+
+
+/* generate_series_date()
+ * Generate the set of dates from start to stop by step
+ */
+Datum
+generate_series_date(PG_FUNCTION_ARGS)
+{
+ return generate_series_step_date(fcinfo);
+}
+
+Datum
+generate_series_step_date(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ generate_series_date_fctx *fctx;
+ DateADT result;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ DateADT start = PG_GETARG_DATEADT(0);
+ DateADT stop = PG_GETARG_DATEADT(1);
+ int32 step = 1;
+ MemoryContext oldcontext;
+
+ /* cannot iterate infinity */
+ if (DATE_NOT_FINITE(start))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("start value cannot be infinite")));
+ if (DATE_NOT_FINITE(stop))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("stop value cannot be infinite")));
+
+ /* see if we were given an explicit step size */
+ if (PG_NARGS() == 3)
+ {
+ step = PG_GETARG_INT32(2);
+ if (step == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("step size cannot equal zero")));
+ }
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* allocate memory for user context */
+ fctx = (generate_series_date_fctx *)
+ palloc(sizeof(generate_series_date_fctx));
+
+ /*
+ * Use fctx to keep state from call to call. Seed current with the
+ * original start value
+ */
+ fctx->current = start;
+ fctx->stop = stop;
+ fctx->step = step;
+
+ funcctx->user_fctx = fctx;
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * get the saved state and use current as the result for this iteration
+ */
+ fctx = funcctx->user_fctx;
+ result = fctx->current;
+
+ if ((fctx->step > 0 && fctx->current <= fctx->stop) ||
+ (fctx->step < 0 && fctx->current >= fctx->stop))
+ {
+ /* increment current in preparation for next iteration */
+ fctx->current += fctx->step;
+
+ /* do when there is more left to send */
+ SRF_RETURN_NEXT(funcctx, DateADTGetDatum(result));
+ }
+ else
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 79e92ff..65293eb 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4013,6 +4013,11 @@ DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t
DESCR("non-persistent series generator");
DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ ));
DESCR("non-persistent series generator");
+DATA(insert OID = 2739 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1082 "1082 1082 23" _null_ _null_ "{start,finish,step}" _null_ _null_ generate_series_step_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+DATA(insert OID = 2740 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 2 0 1082 "1082 1082" _null_ _null_ "{start,finish}" _null_ _null_ generate_series_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+
/* boolean aggregates */
DATA(insert OID = 2515 ( booland_statefunc PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "16 16" _null_ _null_ _null_ _null_ _null_ booland_statefunc _null_ _null_ _null_ ));
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 1b962af..f427cde 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -205,4 +205,7 @@ extern Datum timetz_izone(PG_FUNCTION_ARGS);
extern Datum timetz_pl_interval(PG_FUNCTION_ARGS);
extern Datum timetz_mi_interval(PG_FUNCTION_ARGS);
+extern Datum generate_series_date(PG_FUNCTION_ARGS);
+extern Datum generate_series_step_date(PG_FUNCTION_ARGS);
+
#endif /* DATE_H */
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 56c5520..f808a0d 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1452,3 +1452,36 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SET datestyle TO iso;
+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)
+
+SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
+ERROR: start value cannot be infinite
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date) as d(date_val);
+ERROR: stop value cannot be infinite
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index e40b4c4..b790e32 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -340,3 +340,10 @@ select make_date(2013, 11, -1);
select make_date(-44, 3, 15); -- perhaps we should allow this sometime?
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+
+SET datestyle TO iso;
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date,7) as d(date_val);
+SELECT d.date_val FROM generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
+SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date) as d(date_val);
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
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 139aa2b..7968426 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14657,6 +14657,26 @@ AND
</entry>
</row>
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of one day
+ </entry>
+ </row>
+
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter>
+ </entry>
+ </row>
+
</tbody>
</tgroup>
</table>
@@ -14721,6 +14741,26 @@ SELECT * FROM generate_series('2008-03-01 00:00'::timestamp,
2008-03-03 22:00:00
2008-03-04 08:00:00
(9 rows)
+
+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)
</programlisting>
</para>
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 332db7e..f3609d0 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -30,6 +30,7 @@
#include "utils/datetime.h"
#include "utils/nabstime.h"
#include "utils/sortsupport.h"
+#include "funcapi.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -2811,3 +2812,102 @@ timetz_izone(PG_FUNCTION_ARGS)
PG_RETURN_TIMETZADT_P(result);
}
+
+/* state for generate_series_date(date,date,[step]) */
+typedef struct
+{
+ DateADT current;
+ DateADT stop;
+ int32 step;
+} generate_series_date_fctx;
+
+/* generate_series_date()
+ * Generate the set of dates from start to stop by step
+ */
+Datum
+generate_series_date(PG_FUNCTION_ARGS)
+{
+ return generate_series_step_date(fcinfo);
+}
+
+Datum
+generate_series_step_date(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ generate_series_date_fctx *fctx;
+ DateADT result;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ DateADT start = PG_GETARG_DATEADT(0);
+ DateADT stop = PG_GETARG_DATEADT(1);
+ int32 step = 1;
+ MemoryContext oldcontext;
+
+ /* cannot iterate infinity */
+ if (DATE_NOT_FINITE(start))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("start value cannot be infinite")));
+ if (DATE_NOT_FINITE(stop))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("stop value cannot be infinite")));
+
+ /* see if we were given an explicit step size */
+ if (PG_NARGS() == 3)
+ {
+ step = PG_GETARG_INT32(2);
+ if (step == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("step size cannot equal zero")));
+ }
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* allocate memory for user context */
+ fctx = (generate_series_date_fctx *)
+ palloc(sizeof(generate_series_date_fctx));
+
+ /*
+ * Use fctx to keep state from call to call. Seed current with the
+ * original start value
+ */
+ fctx->current = start;
+ fctx->stop = stop;
+ fctx->step = step;
+
+ funcctx->user_fctx = fctx;
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * get the saved state and use current as the result for this iteration
+ */
+ fctx = funcctx->user_fctx;
+ result = fctx->current;
+
+ if ((fctx->step > 0 && fctx->current <= fctx->stop) ||
+ (fctx->step < 0 && fctx->current >= fctx->stop))
+ {
+ /* increment current in preparation for next iteration */
+ fctx->current += fctx->step;
+
+ /* do when there is more left to send */
+ SRF_RETURN_NEXT(funcctx, DateADTGetDatum(result));
+ }
+ else
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index a2248b4..a1d5fd3 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4013,6 +4013,11 @@ DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t
DESCR("non-persistent series generator");
DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ ));
DESCR("non-persistent series generator");
+DATA(insert OID = 2739 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1082 "1082 1082 23" _null_ _null_ "{start,finish,step}" _null_ _null_ generate_series_step_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+DATA(insert OID = 2740 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 2 0 1082 "1082 1082" _null_ _null_ "{start,finish}" _null_ _null_ generate_series_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+
/* boolean aggregates */
DATA(insert OID = 2515 ( booland_statefunc PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "16 16" _null_ _null_ _null_ _null_ _null_ booland_statefunc _null_ _null_ _null_ ));
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 1b962af..f427cde 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -205,4 +205,7 @@ extern Datum timetz_izone(PG_FUNCTION_ARGS);
extern Datum timetz_pl_interval(PG_FUNCTION_ARGS);
extern Datum timetz_mi_interval(PG_FUNCTION_ARGS);
+extern Datum generate_series_date(PG_FUNCTION_ARGS);
+extern Datum generate_series_step_date(PG_FUNCTION_ARGS);
+
#endif /* DATE_H */
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 56c5520..f808a0d 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1452,3 +1452,36 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SET datestyle TO iso;
+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)
+
+SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
+ERROR: start value cannot be infinite
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date) as d(date_val);
+ERROR: stop value cannot be infinite
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index e40b4c4..b790e32 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -340,3 +340,10 @@ select make_date(2013, 11, -1);
select make_date(-44, 3, 15); -- perhaps we should allow this sometime?
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+
+SET datestyle TO iso;
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date,7) as d(date_val);
+SELECT d.date_val FROM generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
+SELECT d.date_val FROM generate_series('-infinity'::date,'1999-12-29'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'infinity'::date) as d(date_val);
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
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.
Ah, understood. I rebuilt the docs thinking there was a make check failure
I missed, then viewed the html in links (I develop on an AWS box) and saw
nothing untoward.
I'll retest and update the review accordingly.
Thanks!
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, passed
Everything looks good to me. Ready for a committer to have a look.
The new status of this patch is: Ready for Committer
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2 February 2016 at 18:01, Corey Huinker <corey.huinker@gmail.com> wrote:
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.
[step] is in days, but is not documented as such.
My understanding is you want to replace this
SELECT d.dt::date as dt
FROM generate_series('2015-01-01'::date,
'2016-01-04'::date,
interval '1 day') AS d(dt);
with this
SELECT d.dt
FROM generate_series('2015-01-01'::date,
'2016-01-04'::date,
7) as d(dt);
Personally, I think writing INTERVAL '7 days' to be clearer than just
typing 7.
Other than that, the only difference is the ::date part. Is it really worth
adding extra code just for that? I would say not.
No comments on the patch itself, which seems to do the job, so apologies to
give this opinion on your work, I do hope it doesn't put you off further
contributions.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
[step] is in days, but is not documented as such.
It is in days, and is not documented as such, but since a day is the
smallest unit of time for a date, I felt there was no other interpretation.
My understanding is you want to replace this
SELECT d.dt::date as dt
FROM generate_series('2015-01-01'::date,
'2016-01-04'::date,
interval '1 day') AS d(dt);with this
SELECT d.dt
FROM generate_series('2015-01-01'::date,
'2016-01-04'::date,
7) as d(dt);
I'd also like to be able to join the values of d.dt without typecasting
them.
To me it's as awkward as (select 1::double + 1::double)::integer
Personally, I think writing INTERVAL '7 days' to be clearer than just
typing 7.
Well, nearly all my use cases involve the step being 1 (and thus omitted)
or -1.
Maybe this example will appeal to you
SELECT d.birth_date, COUNT(r.kiddo_name)
FROM generate_series('2016-01-01'::date,'2016-01-10'::date) as d(birth_date)
LEFT OUTER JOIN birth_records r ON r.birth_date = d.birth_date
GROUP BY 1
ORDER BY 1;
Other than that, the only difference is the ::date part. Is it really
worth adding extra code just for that? I would say not.
I would argue it belongs for the sake of completeness.
We added generate_series for numerics when generate_series for floats
already existed.
No comments on the patch itself, which seems to do the job, so apologies to
give this opinion on your work, I do hope it doesn't put you off further
contributions.
Thanks. I appreciate that.
On 02/21/2016 07:56 PM, Corey Huinker wrote:
Other than that, the only difference is the ::date part. Is it
really worth adding extra code just for that? I would say not.I would argue it belongs for the sake of completeness.
So would I.
+1 for adding this missing function.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: David Fetter 2016-01-26 <20160126180011.GA16903@fetter.org>
+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.
select * from generate_series(now(), 'infinity', '1 day') limit 10;
... seems pretty legit to me. If limit pushdown into SRFs happened to
work some day, it'd be a pity if the above query raised an error.
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Christoph Berg <myon@debian.org> writes:
Re: David Fetter 2016-01-26 <20160126180011.GA16903@fetter.org>
+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.
select * from generate_series(now(), 'infinity', '1 day') limit 10;
... seems pretty legit to me. If limit pushdown into SRFs happened to
work some day, it'd be a pity if the above query raised an error.
Oooh ... actually, that works today if you consider the SRF-in-targetlist
case:
regression=# select generate_series(now(), 'infinity', '1 day') limit 10;
generate_series
-------------------------------
2016-02-21 16:51:03.303064-05
2016-02-22 16:51:03.303064-05
2016-02-23 16:51:03.303064-05
2016-02-24 16:51:03.303064-05
2016-02-25 16:51:03.303064-05
2016-02-26 16:51:03.303064-05
2016-02-27 16:51:03.303064-05
2016-02-28 16:51:03.303064-05
2016-02-29 16:51:03.303064-05
2016-03-01 16:51:03.303064-05
(10 rows)
Time: 8.457 ms
Given that counterexample, I think we not only shouldn't back-patch such a
change but should reject it altogether.
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 Mon, Feb 22, 2016 at 6:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Oooh ... actually, that works today if you consider the SRF-in-targetlist
case:regression=# select generate_series(now(), 'infinity', '1 day') limit 10;
generate_series
-------------------------------
2016-02-21 16:51:03.303064-05
2016-02-22 16:51:03.303064-05
2016-02-23 16:51:03.303064-05
2016-02-24 16:51:03.303064-05
2016-02-25 16:51:03.303064-05
2016-02-26 16:51:03.303064-05
2016-02-27 16:51:03.303064-05
2016-02-28 16:51:03.303064-05
2016-02-29 16:51:03.303064-05
2016-03-01 16:51:03.303064-05
(10 rows)Time: 8.457 ms
Given that counterexample, I think we not only shouldn't back-patch such a
change but should reject it altogether.
Ouch, good point. The overflows are a different problem that we had
better address though (still on my own TODO list)...
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Given that counterexample, I think we not only shouldn't back-patch such
a
change but should reject it altogether.
Ouch, good point. The overflows are a different problem that we had
better address though (still on my own TODO list)...
So I should remove the bounds checking from
generate_series(date,date,[int]) altogether?
Hi,
On 02/22/2016 08:04 PM, Corey Huinker wrote:
Given that counterexample, I think we not only shouldn't back-patch such a
change but should reject it altogether.Ouch, good point. The overflows are a different problem that we had
better address though (still on my own TODO list)...So I should remove the bounds checking from
generate_series(date,date,[int]) altogether?
I feel rather uneasy about simply removing the 'infinity' checks. Is
there a way to differentiate those two cases, i.e. when the
generate_series is called in target list and in the FROM part? If yes,
we could do the check only in the FROM part, which is the case that does
not work (and consumes arbitrary amounts of memory).
regards
--
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 2/21/16 2:24 PM, Vik Fearing wrote:
On 02/21/2016 07:56 PM, Corey Huinker wrote:
Other than that, the only difference is the ::date part. Is it
really worth adding extra code just for that? I would say not.I would argue it belongs for the sake of completeness.
So would I.
+1 for adding this missing function.
+1. FWIW, a sample query I wrote for a customer yesterday would have
looked nicer with this function. Here's how the generate_series looked:
generate_series('2016-03-01'::date, '2016-03-31'::date, interval '1
day')::date
But it would have been cleaner to write:
generate_series('2016-03-01'::date, '2016-03-31'::date)
More importantly, though, I don't like that the timestamp version of the
function happily takes date parameters but returns timestamps. I feel
this could lead to some subtle bugs for the unwary.
--
-David
david@pgmasters.net
I feel rather uneasy about simply removing the 'infinity' checks. Is there
a way to differentiate those two cases, i.e. when the generate_series is
called in target list and in the FROM part? If yes, we could do the check
only in the FROM part, which is the case that does not work (and consumes
arbitrary amounts of memory).
It would be simple enough to remove the infinity test on the "stop" and
leave it on the "start". Or yank both. Just waiting for others to agree
which checks should remain.
On Fri, Mar 4, 2016 at 3:17 PM, Corey Huinker <corey.huinker@gmail.com> wrote:
I feel rather uneasy about simply removing the 'infinity' checks. Is there
a way to differentiate those two cases, i.e. when the generate_series is
called in target list and in the FROM part? If yes, we could do the check
only in the FROM part, which is the case that does not work (and consumes
arbitrary amounts of memory).It would be simple enough to remove the infinity test on the "stop" and
leave it on the "start". Or yank both. Just waiting for others to agree
which checks should remain.
Let's yank 'em. This is a minor issue which is distracting us from
the main point of this patch, and I don't think it's worth getting
distracted.
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>,
<parameter>stop</parameter>, <parameter>step
integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter>
to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter>I think this should be followed by the word "days" and a period.
+ else
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);I think we should drop the "else" and unindent the next two lines.
That's the style I have seen elsewhere. Plus less indentation equals
more happiness.
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
It would be simple enough to remove the infinity test on the "stop" and
leave it on the "start". Or yank both. Just waiting for others to agree
which checks should remain.Let's yank 'em. This is a minor issue which is distracting us from
the main point of this patch, and I don't think it's worth getting
distracted.
+1. It leaves this function consistent with the others, and if we want to
add checks later we can do them all at the same time.
+ <row> + <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry> + <entry><type>date</type></entry> + <entry><type>setof date</type></entry> + <entry> + Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter> + with a step size of <parameter>step</parameter>I think this should be followed by the word "days" and a period.
No objections. I just followed the pattern of the other generate_series()
docs.
+ else + /* do when there is no more left */ + SRF_RETURN_DONE(funcctx);I think we should drop the "else" and unindent the next two lines.
That's the style I have seen elsewhere. Plus less indentation equals
more happiness.
No objections here either. I just followed the pattern of generate_series()
for int there.
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.
Just David and Vik so far. The rest were either against(Simon), meh(Robert)
or +1ed/-1ed the backpatch, leaving their thoughts on the function itself
unspoken.
Happy to make the changes above if we're moving forward with it.
On Tue, Mar 8, 2016 at 3:57 PM, Corey Huinker <corey.huinker@gmail.com>
wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.Just David and Vik so far. The rest were either against(Simon),
meh(Robert) or +1ed/-1ed the backpatch, leaving their thoughts on the
function itself unspoken.Happy to make the changes above if we're moving forward with it.
I'll toss a user-land +1 for this.
David J.
Robert Haas wrote:
Let's yank 'em. This is a minor issue which is distracting us from
the main point of this patch, and I don't think it's worth getting
distracted.
+1
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.
+1
--
�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
On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
--
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 Thu, Mar 10, 2016 at 1:53 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
+1: David Steele, Vik Fearing, David Johnson, Alvaro Herrera
meh: Robert Haas, Michael Paquier
-1: Simon RiggsThat's probably marginally enough support to proceed with this, but
I'm somewhat unenthused about putting time into it myself. Alvaro,
any chance you want to handle this one?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas wrote:
That's probably marginally enough support to proceed with this, but
I'm somewhat unenthused about putting time into it myself. Alvaro,
any chance you want to handle this one?
OK, I can take it, but I have a few other things earlier in my queue so
it will be a few days.
--
�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
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Simon Riggs wrote:
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
I would say that "meh" is +0, actually. So the the tally is a small
positive number -- not great, but seems enough to press forward unless
we get more -1 votes.
--
�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
* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
Simon Riggs wrote:
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
I would say that "meh" is +0, actually. So the the tally is a small
positive number -- not great, but seems enough to press forward unless
we get more -1 votes.
I'm +1 on this also, for my part.
Thanks!
Stephen
On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 10, 2016 at 10:58 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
New patch for Alvaro's consideration.
Very minor changes since the last time, the explanations below are
literally longer than the changes:
- Rebased, though I don't think any of the files had changed in the mean
time
- Removed infinity checks/errors and the test cases to match
- Amended documentation to add 'days' after 'step' as suggested
- Did not add a period as suggested, to remain consistent with other
descriptions in the same sgml table
- Altered test case and documentation of 7 day step example so that the
generated dates do not land evenly on the end date. A reader might
incorrectly infer that the end date must be in the result set, when it
doesn't have to be.
- Removed unnecessary indentation that existed purely due to following of
other generate_series implementations
Attachments:
generate_series_date.v3.difftext/plain; charset=US-ASCII; name=generate_series_date.v3.diffDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b5ee81..0a8c280 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14700,6 +14700,26 @@ AND
</entry>
</row>
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of one day
+ </entry>
+ </row>
+
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter> days
+ </entry>
+ </row>
+
</tbody>
</tgroup>
</table>
@@ -14764,6 +14784,26 @@ SELECT * FROM generate_series('2008-03-01 00:00'::timestamp,
2008-03-03 22:00:00
2008-03-04 08:00:00
(9 rows)
+
+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-02'::date,7) as d(date_val);
+ date_val
+------------
+ 1991-09-24
+ 1991-10-01
+(2 rows)
</programlisting>
</para>
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 332db7e..af4000d 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -30,6 +30,7 @@
#include "utils/datetime.h"
#include "utils/nabstime.h"
#include "utils/sortsupport.h"
+#include "funcapi.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -2811,3 +2812,92 @@ timetz_izone(PG_FUNCTION_ARGS)
PG_RETURN_TIMETZADT_P(result);
}
+
+/* Corey BEGIN */
+typedef struct
+{
+ DateADT current;
+ DateADT stop;
+ int32 step;
+} generate_series_date_fctx;
+
+
+/* generate_series_date()
+ * Generate the set of dates from start to stop by step
+ */
+Datum
+generate_series_date(PG_FUNCTION_ARGS)
+{
+ return generate_series_step_date(fcinfo);
+}
+
+Datum
+generate_series_step_date(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ generate_series_date_fctx *fctx;
+ DateADT result;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ DateADT start = PG_GETARG_DATEADT(0);
+ DateADT stop = PG_GETARG_DATEADT(1);
+ int32 step = 1;
+ MemoryContext oldcontext;
+
+ /* see if we were given an explicit step size */
+ if (PG_NARGS() == 3)
+ {
+ step = PG_GETARG_INT32(2);
+ if (step == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("step size cannot equal zero")));
+ }
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* allocate memory for user context */
+ fctx = (generate_series_date_fctx *)
+ palloc(sizeof(generate_series_date_fctx));
+
+ /*
+ * Use fctx to keep state from call to call. Seed current with the
+ * original start value
+ */
+ fctx->current = start;
+ fctx->stop = stop;
+ fctx->step = step;
+
+ funcctx->user_fctx = fctx;
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * get the saved state and use current as the result for this iteration
+ */
+ fctx = funcctx->user_fctx;
+ result = fctx->current;
+
+ if ((fctx->step > 0 && fctx->current <= fctx->stop) ||
+ (fctx->step < 0 && fctx->current >= fctx->stop))
+ {
+ /* increment current in preparation for next iteration */
+ fctx->current += fctx->step;
+
+ /* do when there is more left to send */
+ SRF_RETURN_NEXT(funcctx, DateADTGetDatum(result));
+ }
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index a0f821a..2adb50a 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4033,6 +4033,11 @@ DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t
DESCR("non-persistent series generator");
DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ ));
DESCR("non-persistent series generator");
+DATA(insert OID = 2739 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1082 "1082 1082 23" _null_ _null_ "{start,finish,step}" _null_ _null_ generate_series_step_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+DATA(insert OID = 2740 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 2 0 1082 "1082 1082" _null_ _null_ "{start,finish}" _null_ _null_ generate_series_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+
/* boolean aggregates */
DATA(insert OID = 2515 ( booland_statefunc PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "16 16" _null_ _null_ _null_ _null_ _null_ booland_statefunc _null_ _null_ _null_ ));
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 1b962af..f427cde 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -205,4 +205,7 @@ extern Datum timetz_izone(PG_FUNCTION_ARGS);
extern Datum timetz_pl_interval(PG_FUNCTION_ARGS);
extern Datum timetz_mi_interval(PG_FUNCTION_ARGS);
+extern Datum generate_series_date(PG_FUNCTION_ARGS);
+extern Datum generate_series_step_date(PG_FUNCTION_ARGS);
+
#endif /* DATE_H */
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 56c5520..9568224 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1452,3 +1452,32 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SET datestyle TO iso;
+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-02'::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)
+
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index e40b4c4..ff9a9d2 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -340,3 +340,8 @@ select make_date(2013, 11, -1);
select make_date(-44, 3, 15); -- perhaps we should allow this sometime?
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+
+SET datestyle TO iso;
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-02'::date,7) as d(date_val);
+SELECT d.date_val FROM generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
Corey Huinker wrote:
New patch for Alvaro's consideration.
Thanks. As I said, it will be a few days before I get to this; any
further reviews in the meantime are welcome.
--
�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
On Thu, Mar 10, 2016 at 4:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.
I am of the same feeling, at -0.5. I don't feel like putting -1 for
this patch, as I don't really understand why this is worth adding more
complexity in the code for something that can be done with
generate_series using timestamps. Also, why only dates? And why not
other units like hours or seconds?
--
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 Thu, Mar 10, 2016 at 8:58 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.
I tend to think we err toward this too much. This seems like development
concerns trumping usability. Consider that anything someone took the time
to write and polish to make committable to core was obviously genuinely
useful to them - and for every person capable of actually taking things
that far there are likely many more like myself who cannot but have
encountered the, albeit minor, usability annoyance that this kind of
function seeks to remove.
I really don't care that the codebase is marginally larger or that there is
another few records in the catalog - I hope that our code organization and
performance is capable of handling it (and many more).
The overhead of adding an entirely new concept to core would give me more
pause in that situation but this function in particular simply fleshes out
what the user community sees to be a minor yet notable lack in our existing
offering of the generate_series() feature. Its threshold for meeting
"genuinely" should be considerably lower than for more invasive or complex
features such as those that require entirely new syntax (e.g., the recently
rejected ALTER ROLE patch) to implement.
If something can be done with a user-defined function on stock PostgreSQL,
especially for concepts or features we already have, the decision to commit
a c-language function that someone else has written and others have
reviewed should, IMO, be given the benefit of assumed usefulness.
My $0.02
David J.
On Thu, Mar 10, 2016 at 9:30 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Mar 10, 2016 at 4:58 PM, Robert Haas <robertmhaas@gmail.com>
wrote:On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:
On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.I am of the same feeling, at -0.5. I don't feel like putting -1 for
this patch, as I don't really understand why this is worth adding more
complexity in the code for something that can be done with
generate_series using timestamps. Also, why only dates? And why not
other units like hours or seconds?
A date is a type, hours and seconds are not types. To use hours and
seconds you need timestamp (I guess we could offer a "time" version of this
function too) which we already have. Also, not choosing to implement
something else generally shouldn't preclude something that exists and have
genuine value from being committed.
Obviously there is some user-land annoyance at having to play with
timestamp when all one really cares about is date. Given the prevalence of
date usage in user-land this is not surprising.
We're not forcing anyone to review this that doesn't see that it is worth
their time. We are asking th
at
the people that the community has placed in a position of authority spend
some a limited amount of effort reviewing a minor addition that has been
deemed desirable and that has already been reviewed and deemed something
that meets the project's technical requirements.
The expectation is that the amount of ongoing support this function
would require is similar to that of the existing generate_series functions.
This is something that can be easily added by the user as a SQL function -
its complexity cannot be so great as to be deemed a risk to the system but
including its c-language variant. As you said, we already do something
very similar for timestamps so the marginal complexity being added
shouldn't be significant.
If you are going to -1 or -0.5 for "adds too much complexity" it would be
helpful to know specifics. Scanning the thread the only real concern was
dealing with infinity - which is already a complexity the current functions
have so there is no "additional" there - but maybe I've missed something.
I understand Robert's position and while I find it to be community-hostile
this is an open source project and so I accept that this is a possible
outcome. But as soon as he asked for some +1s he got them (mostly without
reasons but the reality is that the request was for desire) and a few "I
vote -0.5 since my dislike is only half-baked". And the fact is that a
single +1 for the idea likely represents many people at large who would use
the function if present while I suspect most of those who could offer an
informed -1 are already on this list. The vast majority probably don't
care either way as long as we don't introduce bugs.
David J.
removed leftover development comment
On Thu, Mar 10, 2016 at 11:02 AM, Corey Huinker <corey.huinker@gmail.com>
wrote:
Show quoted text
On Thu, Mar 10, 2016 at 10:58 AM, Robert Haas <robertmhaas@gmail.com>
wrote:On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:On 10 March 2016 at 06:53, Michael Paquier <michael.paquier@gmail.com>
wrote:On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
I'm pretty meh about the whole idea of this function, though,
actually, and I don't see a single clear +1 vote for this
functionality upthread. (Apologies if I've missed one.) In the
absence of a few of those, I recommend we reject this.+1
I'm meh for this patch.
"meh" == +1
I thought it meant -1
In my case it meant, like, -0.5. I don't really like adding lots of
utility functions like this to the default install, because I'm not
sure how widely they get used and it gradually increases the size of
the code, system catalogs, etc. But I also don't want to block
genuinely useful things. So basically, I'm not excited about this
patch, but I don't want to fight about it either.--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL CompanyNew patch for Alvaro's consideration.
Very minor changes since the last time, the explanations below are
literally longer than the changes:
- Rebased, though I don't think any of the files had changed in the mean
time
- Removed infinity checks/errors and the test cases to match
- Amended documentation to add 'days' after 'step' as suggested
- Did not add a period as suggested, to remain consistent with other
descriptions in the same sgml table
- Altered test case and documentation of 7 day step example so that the
generated dates do not land evenly on the end date. A reader might
incorrectly infer that the end date must be in the result set, when it
doesn't have to be.
- Removed unnecessary indentation that existed purely due to following of
other generate_series implementations
Attachments:
generate_series_date.v4.difftext/plain; charset=US-ASCII; name=generate_series_date.v4.diffDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b5ee81..0a8c280 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14700,6 +14700,26 @@ AND
</entry>
</row>
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of one day
+ </entry>
+ </row>
+
+ <row>
+ <entry><literal><function>generate_series(<parameter>start</parameter>, <parameter>stop</parameter>, <parameter>step integer</parameter>)</function></literal></entry>
+ <entry><type>date</type></entry>
+ <entry><type>setof date</type></entry>
+ <entry>
+ Generate a series of values, from <parameter>start</parameter> to <parameter>stop</parameter>
+ with a step size of <parameter>step</parameter> days
+ </entry>
+ </row>
+
</tbody>
</tgroup>
</table>
@@ -14764,6 +14784,26 @@ SELECT * FROM generate_series('2008-03-01 00:00'::timestamp,
2008-03-03 22:00:00
2008-03-04 08:00:00
(9 rows)
+
+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-02'::date,7) as d(date_val);
+ date_val
+------------
+ 1991-09-24
+ 1991-10-01
+(2 rows)
</programlisting>
</para>
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 332db7e..af4000d 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -30,6 +30,7 @@
#include "utils/datetime.h"
#include "utils/nabstime.h"
#include "utils/sortsupport.h"
+#include "funcapi.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -2811,3 +2812,92 @@ timetz_izone(PG_FUNCTION_ARGS)
PG_RETURN_TIMETZADT_P(result);
}
+
+typedef struct
+{
+ DateADT current;
+ DateADT stop;
+ int32 step;
+} generate_series_date_fctx;
+
+
+/* generate_series_date()
+ * Generate the set of dates from start to stop by step
+ */
+Datum
+generate_series_date(PG_FUNCTION_ARGS)
+{
+ return generate_series_step_date(fcinfo);
+}
+
+Datum
+generate_series_step_date(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ generate_series_date_fctx *fctx;
+ DateADT result;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ DateADT start = PG_GETARG_DATEADT(0);
+ DateADT stop = PG_GETARG_DATEADT(1);
+ int32 step = 1;
+ MemoryContext oldcontext;
+
+ /* see if we were given an explicit step size */
+ if (PG_NARGS() == 3)
+ {
+ step = PG_GETARG_INT32(2);
+ if (step == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("step size cannot equal zero")));
+ }
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* allocate memory for user context */
+ fctx = (generate_series_date_fctx *)
+ palloc(sizeof(generate_series_date_fctx));
+
+ /*
+ * Use fctx to keep state from call to call. Seed current with the
+ * original start value
+ */
+ fctx->current = start;
+ fctx->stop = stop;
+ fctx->step = step;
+
+ funcctx->user_fctx = fctx;
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * get the saved state and use current as the result for this iteration
+ */
+ fctx = funcctx->user_fctx;
+ result = fctx->current;
+
+ if ((fctx->step > 0 && fctx->current <= fctx->stop) ||
+ (fctx->step < 0 && fctx->current >= fctx->stop))
+ {
+ /* increment current in preparation for next iteration */
+ fctx->current += fctx->step;
+
+ /* do when there is more left to send */
+ SRF_RETURN_NEXT(funcctx, DateADTGetDatum(result));
+ }
+ /* do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index a0f821a..2adb50a 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4033,6 +4033,11 @@ DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t
DESCR("non-persistent series generator");
DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ ));
DESCR("non-persistent series generator");
+DATA(insert OID = 2739 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 3 0 1082 "1082 1082 23" _null_ _null_ "{start,finish,step}" _null_ _null_ generate_series_step_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+DATA(insert OID = 2740 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s s 2 0 1082 "1082 1082" _null_ _null_ "{start,finish}" _null_ _null_ generate_series_date _null_ _null_ _null_ ));
+DESCR("non-persistent series generator");
+
/* boolean aggregates */
DATA(insert OID = 2515 ( booland_statefunc PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "16 16" _null_ _null_ _null_ _null_ _null_ booland_statefunc _null_ _null_ _null_ ));
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 1b962af..f427cde 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -205,4 +205,7 @@ extern Datum timetz_izone(PG_FUNCTION_ARGS);
extern Datum timetz_pl_interval(PG_FUNCTION_ARGS);
extern Datum timetz_mi_interval(PG_FUNCTION_ARGS);
+extern Datum generate_series_date(PG_FUNCTION_ARGS);
+extern Datum generate_series_step_date(PG_FUNCTION_ARGS);
+
#endif /* DATE_H */
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index 56c5520..9568224 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1452,3 +1452,32 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SET datestyle TO iso;
+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-02'::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)
+
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index e40b4c4..ff9a9d2 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -340,3 +340,8 @@ select make_date(2013, 11, -1);
select make_date(-44, 3, 15); -- perhaps we should allow this sometime?
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+
+SET datestyle TO iso;
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-01'::date) as d(date_val);
+SELECT d.date_val FROM generate_series('1991-09-24'::date,'1991-10-02'::date,7) as d(date_val);
+SELECT d.date_val FROM generate_series('1999-12-31'::date,'1999-12-29'::date,-1) as d(date_val);
On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:
I tend to think we err toward this too much. This seems like development
concerns trumping usability. Consider that anything someone took the time
to write and polish to make committable to core was obviously genuinely
useful to them - and for every person capable of actually taking things that
far there are likely many more like myself who cannot but have encountered
the, albeit minor, usability annoyance that this kind of function seeks to
remove.
Sure, an individual function like this has almost no negative impact.
On the other hand, working around its absence is also trivial. You
can create a wrapper function that does exactly this in a couple of
lines of SQL. In my opinion, saying that people should do that in
they need it has some advantages over shipping it to everyone. If you
don't have it and you want it, you can easily get it. But what if you
have it and you don't want it, for example because what you really
want is a minimal postgres installation? You can't take anything in
core back out again, or at least not easily. Everything about core is
expanding very randomly - code size, shared memory footprint, all of
it. If you think that has no downside for users, I respectfully
disagree.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10 March 2016 at 18:45, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:I tend to think we err toward this too much. This seems like development
concerns trumping usability. Consider that anything someone took thetime
to write and polish to make committable to core was obviously genuinely
useful to them - and for every person capable of actually taking thingsthat
far there are likely many more like myself who cannot but have
encountered
the, albeit minor, usability annoyance that this kind of function seeks
to
remove.
Sure, an individual function like this has almost no negative impact.
On the other hand, working around its absence is also trivial. You
can create a wrapper function that does exactly this in a couple of
lines of SQL. In my opinion, saying that people should do that in
they need it has some advantages over shipping it to everyone. If you
don't have it and you want it, you can easily get it. But what if you
have it and you don't want it, for example because what you really
want is a minimal postgres installation? You can't take anything in
core back out again, or at least not easily. Everything about core is
expanding very randomly - code size, shared memory footprint, all of
it. If you think that has no downside for users, I respectfully
disagree.
Not sure what y'all are discussing, but I should add that I would have
committed this based solely upon Vik's +1.
My objection was made, then overruled; that works because the objection
wasn't "it won't work", only a preference so I'm happy.
But I still don't know "meh" means.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 10, 2016 at 2:35 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
But I still don't know "meh" means.
Maybe this helps?
https://en.wikipedia.org/wiki/Meh
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/10/2016 11:44 AM, Robert Haas wrote:
On Thu, Mar 10, 2016 at 2:35 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
But I still don't know "meh" means.
Maybe this helps?
LOL
https://en.wikipedia.org/wiki/LOL
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 10, 2016 at 11:45 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:I tend to think we err toward this too much. This seems like development
concerns trumping usability. Consider that anything someone took thetime
to write and polish to make committable to core was obviously genuinely
useful to them - and for every person capable of actually taking thingsthat
far there are likely many more like myself who cannot but have
encountered
the, albeit minor, usability annoyance that this kind of function seeks
to
remove.
Sure, an individual function like this has almost no negative impact.
On the other hand, working around its absence is also trivial. You
can create a wrapper function that does exactly this in a couple of
lines of SQL. In my opinion, saying that people should do that in
they need it has some advantages over shipping it to everyone. If you
don't have it and you want it, you can easily get it. But what if you
have it and you don't want it, for example because what you really
want is a minimal postgres installation?
I'd rather cater to the people who are content that everything in
PostgreSQL is of high quality and ignore those things that they have no
immediate need for - and then are happy to find out that when they have a
new need PostgreSQL already provides them well thought-out and tested tool
that they can use.
We purport to be highly extensible but that doesn't preclude us from being
well-stocked at the same time.
And by not including these kinds of things in core the broader ecosystem is
harmed since not every provider or PostgreSQL services is willing or
capable of allowing random third-party extensions; nor is adding 20
"important and generally useful to me but an annoyance to the project"
functions to every database I create a trivial exercise. But it is trivial
to ignore something that exists but that I have no need for.
You can't take anything in
core back out again, or at least not easily. Everything about core is
expanding very randomly - code size, shared memory footprint, all of
it. If you think that has no downside for users, I respectfully
disagree.
Attempts to limit that expansion seemingly happen "very randomly" as well.
If there is a goal and demand for a "minimalist" installation then we don't
seem to have anything going on that is actively trying to make it a
reality. I'm likely being a bit myopic here but at the same time the
increased footprint from JSON, parallel queries, and the like dwarf the
contribution a handful of C-language functions would add.
I do understand why more trivial features are scrutinized the way they are
but I still hold the opinion that features such as this should generally be
given the benefit of inclusion unless there are concrete technical concerns.
David J.
On 11/03/16 08:48, Igal @ Lucee.org wrote:
On 3/10/2016 11:44 AM, Robert Haas wrote:
On Thu, Mar 10, 2016 at 2:35 PM, Simon Riggs <simon@2ndquadrant.com>
wrote:But I still don't know "meh" means.
Maybe this helps?
I'm pretending to work, so will you and Robert stop making that pretence
more difficult!!! :-)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/10/16 1:24 PM, Corey Huinker wrote:
New patch for Alvaro's consideration.
Very minor changes since the last time, the explanations below are
literally longer than the changes:
- Rebased, though I don't think any of the files had changed in the
mean time
- Removed infinity checks/errors and the test cases to match
- Amended documentation to add 'days' after 'step' as suggested
- Did not add a period as suggested, to remain consistent with other
descriptions in the same sgml table
- Altered test case and documentation of 7 day step example so that
the generated dates do not land evenly on the end date. A reader
might incorrectly infer that the end date must be in the result set,
when it doesn't have to be.
- Removed unnecessary indentation that existed purely due to
following of other generate_series implementations
As far as I can see overall support is in favor of this patch although
it is not overwhelming by any means.
I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to �lvaro.
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net> wrote:
On 3/10/16 1:24 PM, Corey Huinker wrote:
New patch for Alvaro's consideration.
Very minor changes since the last time, the explanations below are
literally longer than the changes:
- Rebased, though I don't think any of the files had changed in the
mean time
- Removed infinity checks/errors and the test cases to match
- Amended documentation to add 'days' after 'step' as suggested
- Did not add a period as suggested, to remain consistent with other
descriptions in the same sgml table
- Altered test case and documentation of 7 day step example so that
the generated dates do not land evenly on the end date. A reader
might incorrectly infer that the end date must be in the result set,
when it doesn't have to be.
- Removed unnecessary indentation that existed purely due to
following of other generate_series implementationsAs far as I can see overall support is in favor of this patch although it is
not overwhelming by any means.I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to Álvaro.
So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.
However, I now realise that it introduces a backwards-compatibility
breakage. Today it is possible to type
SELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7 days');
and it works with just that one cast. However, this patch breaks that.
Now I'm not saying that I have used the above construct. Probably not
in fact, but maybe my work colleagues have. I honestly can't say, but
the thought of having to grep through thousands of lines of
application code to check isn't particularly appealing.
So I'm afraid it's -1 from me.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/17/16 4:49 AM, Dean Rasheed wrote:
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net> wrote:
I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to Álvaro.So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.However, I now realise that it introduces a backwards-compatibility
breakage. Today it is possible to typeSELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7 days');
It can also be broken as below and this is even scarier to me:
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days');
ERROR: invalid input syntax for integer: "7 days"
LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7 days');
And only works when:
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days'::interval);
generate_series
------------------------
2000-01-01 00:00:00+00
(1 row)
One might argue that string constants for dates in actual code might be
rare but I think it's safe to say that string constants for intervals
are pretty common. I also think it unlikely that they are all
explicitly cast.
I marked this "waiting for author" so Corey can respond. I actually
tested with the v3 patch since the v4 patch seems to be broken with git
apply or patch:
$ patch -p1 < ../other/generate_series_date.v4.diff
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 14787 (offset 87 lines).
Hunk #2 succeeded at 14871 (offset 87 lines).
patching file src/backend/utils/adt/date.c
patch: **** malformed patch at line 163: diff --git
a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
$ git apply -3 ../other/generate_series_date.v4.diff
fatal: corrupt patch at line 163
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 17, 2016 at 10:00 AM, David Steele <david@pgmasters.net> wrote:
On 3/17/16 4:49 AM, Dean Rasheed wrote:
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net> wrote:
I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to Álvaro.So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.However, I now realise that it introduces a backwards-compatibility
breakage. Today it is possible to typeSELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7
days');
It can also be broken as below and this is even scarier to me:
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days');
ERROR: invalid input syntax for integer: "7 days"
LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7 days');And only works when:
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days'::interval);
generate_series
------------------------
2000-01-01 00:00:00+00
(1 row)One might argue that string constants for dates in actual code might be
rare but I think it's safe to say that string constants for intervals
are pretty common. I also think it unlikely that they are all
explicitly cast.I marked this "waiting for author" so Corey can respond. I actually
tested with the v3 patch since the v4 patch seems to be broken with git
apply or patch:$ patch -p1 < ../other/generate_series_date.v4.diff
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 14787 (offset 87 lines).
Hunk #2 succeeded at 14871 (offset 87 lines).
patching file src/backend/utils/adt/date.c
patch: **** malformed patch at line 163: diff --git
a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h$ git apply -3 ../other/generate_series_date.v4.diff
fatal: corrupt patch at line 163--
-David
david@pgmasters.net
Not sure what's wrong with the patch, but I get a clean one to you pending
the outcome of the design discussion. v4 just ripped out the infinity
tests, so v3 is valid for the issue you found..
So the function clobbers the situation where the user meant a timestamp
range but instead gave two dates, and meant an interval but gave a string.
I'm curious how generate_series() for numeric didn't have the same issue
with floats.
I see two ways around this:
1. Drop the step parameter entirely. My own use cases only ever require the
step values 1 and -1, and which one the user wants can be inferred from
(start < end). This would still change the output type where a person
wanted timestamps, but instead input two dates.
2. Rename the function date_series() or generate_series_date()
I still think this is an important function because at the last several
places I've worked, I've found myself manufacturing a query where some
event data is likely to have date gaps, but we want to see the date gaps or
at least have the 0 values contribute to a later average aggregate.
Like this:
select d.dt, sum(s.v1), sum(s2.v2), sum(t.v1), sum(t2.v2)
from date_series_function(input1,input2) as d(dt),
some_dimensional_characteristic c
left join sparse_table s on s.event_date = d.dt and s.c1 = c.cval
left join sparse_table s2 on s2.event_date = d.dt and s2.c2 = c.cval
left join other_sparse t on t.event_date = d.dt and t.c1 = c.cval
left join other_sparse t2 on t2.event_date = d.dt and t2.c2 = c.cval
group by d.dt
order by d.dt
This gets cumbersome when you have to cast every usage of your date column.
Furthermore, this is 90%+ of my usage of generate_series(), the remaining
10% being the integer case to populate sample data for tests.
Any thoughts on how to proceed?
On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker <corey.huinker@gmail.com>
wrote:
On Thu, Mar 17, 2016 at 10:00 AM, David Steele <david@pgmasters.net>
wrote:On 3/17/16 4:49 AM, Dean Rasheed wrote:
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net> wrote:
I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to Álvaro.So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.However, I now realise that it introduces a backwards-compatibility
breakage. Today it is possible to typeSELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7
days');
It can also be broken as below and this is even scarier to me:
Above and below are the same query...
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days');
ERROR: invalid input syntax for integer: "7 days"
LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7 days');And only works when:
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days'::interval);
generate_series
------------------------
2000-01-01 00:00:00+00
(1 row)
--
-David
david@pgmasters.netNot sure what's wrong with the patch, but I get a clean one to you pending
the outcome of the design discussion. v4 just ripped out the infinity
tests, so v3 is valid for the issue you found..So the function clobbers the situation where the user meant a timestamp
range but instead gave two dates, and meant an interval but gave a string.
I'm curious how generate_series() for numeric didn't have the same issue
with floats.
The numeric forms use anyelement for all three arguments but the timestamp
version uses an explicit interval for the third.
I see two ways around this:
1. Drop the step parameter entirely. My own use cases only ever require
the step values 1 and -1, and which one the user wants can be inferred from
(start < end). This would still change the output type where a person
wanted timestamps, but instead input two dates.
Undesirable.
2. Rename the function date_series() or generate_series_date()
I still think this is an important function because at the last several
places I've worked, I've found myself manufacturing a query where some
event data is likely to have date gaps, but we want to see the date gaps or
at least have the 0 values contribute to a later average aggregate.
I'd call it "generate_dates(...)" and be done with it.
We would then have:
generate_series()
generate_subscripts()
generate_dates()
David J.
On Thu, Mar 17, 2016 at 11:30 AM, David G. Johnston <
david.g.johnston@gmail.com> wrote:
I'd call it "generate_dates(...)" and be done with it.
Sold. Hope to have a revised patch for you today or tomorrow.
On 3/17/16 11:30 AM, David G. Johnston wrote:
On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker <corey.huinker@gmail.com
<mailto:corey.huinker@gmail.com>>wrote:On Thu, Mar 17, 2016 at 10:00 AM, David Steele <david@pgmasters.net
<mailto:david@pgmasters.net>> wrote:On 3/17/16 4:49 AM, Dean Rasheed wrote:
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net <mailto:david@pgmasters.net>> wrote:
I think in this case it comes down to a committer's judgement so I have
marked this "ready for committer" and passed the buck on to Álvaro.So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.However, I now realise that it introduces a backwards-compatibility
breakage. Today it is possible to typeSELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7 days');
It can also be broken as below and this is even scarier to me:
Above and below are the same query...
Not sure I agree. My point was that even if developers were pretty
careful with their casting (or are using two actual dates) then there's
still possibility for breakage.
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days');
ERROR: invalid input syntax for integer: "7 days"
LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7
days');
<...>I see two ways around this:
1. Drop the step parameter entirely. My own use cases only ever
require the step values 1 and -1, and which one the user wants can
be inferred from (start < end). This would still change the output
type where a person wanted timestamps, but instead input two dates.Undesirable.
Very undesirable. Week intervals are a very valid use case and I don't
like the automagic interval idea.
2. Rename the function date_series() or generate_series_date()
I still think this is an important function because at the last
several places I've worked, I've found myself manufacturing a query
where some event data is likely to have date gaps, but we want to
see the date gaps or at least have the 0 values contribute to a
later average aggregate.I'd call it "generate_dates(...)" and be done with it.
We would then have:
generate_series()
generate_subscripts()
generate_dates()
To me this completely negates the idea of this "just working" which is
why it got a +1 from me in the first place. If I have to remember to
use a different function name then I'd prefer to just cast on the
timestamp version of generate_series().
Sorry, but this is now -1 from me, at least for this commitfest. While
I like the idea I think it is far too late to be redesigning such a
minor feature.
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 17, 2016 at 8:41 AM, David Steele <david@pgmasters.net> wrote:
On 3/17/16 11:30 AM, David G. Johnston wrote:
On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker <corey.huinker@gmail.com
<mailto:corey.huinker@gmail.com>>wrote:On Thu, Mar 17, 2016 at 10:00 AM, David Steele <david@pgmasters.net
<mailto:david@pgmasters.net>> wrote:On 3/17/16 4:49 AM, Dean Rasheed wrote:
On 16 March 2016 at 23:32, David Steele <david@pgmasters.net
<mailto:david@pgmasters.net>> wrote:
I think in this case it comes down to a committer's judgement
so I have
marked this "ready for committer" and passed the buck on to
Álvaro.
So I was pretty much "meh" on this patch too, because I'm not
convinced it actually saves much typing, if any.However, I now realise that it introduces a
backwards-compatibility
breakage. Today it is possible to type
SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000', '7 days');
It can also be broken as below and this is even scarier to me:
Above and below are the same query...
Not sure I agree. My point was that even if developers were pretty
careful with their casting (or are using two actual dates) then there's
still possibility for breakage.
With the first argument casted to date it doesn't matter whether you cast
the second argument as the pseudo-type anyelement will take its value from
the first argument and force the second to date.
The main problem is that the system tries to cast unknown to integer based
upon finding gs(date, date, integer) and fails without ever considering
that gs(ts, ts, interval) would succeed.
postgres=# SELECT * FROM generate_series('01-01-2000'::date,
'01-04-2000'::date, '7 days');
ERROR: invalid input syntax for integer: "7 days"
LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7
days');
<...>I see two ways around this:
1. Drop the step parameter entirely. My own use cases only ever
require the step values 1 and -1, and which one the user wants can
be inferred from (start < end). This would still change the output
type where a person wanted timestamps, but instead input two dates.Undesirable.
Very undesirable. Week intervals are a very valid use case and I don't
like the automagic interval idea.2. Rename the function date_series() or generate_series_date()
I still think this is an important function because at the last
several places I've worked, I've found myself manufacturing a query
where some event data is likely to have date gaps, but we want to
see the date gaps or at least have the 0 values contribute to a
later average aggregate.I'd call it "generate_dates(...)" and be done with it.
We would then have:
generate_series()
generate_subscripts()
generate_dates()To me this completely negates the idea of this "just working" which is
why it got a +1 from me in the first place. If I have to remember to
use a different function name then I'd prefer to just cast on the
timestamp version of generate_series().
So let the user decide whether to trade-off learning a new function name
but getting dates instead of timestamps or going through the hassle of
casting.
For me, it would have been a nice bonus if the generate_series() could be
used directly but I feel that the desired functionality is desirable and
the name itself is of only minor consideration.
I can see that newbies might ponder why two functions exist but they should
understand "backward compatibility".
I suspect that most people will simply think: "use generate_series for
numbers and use generate_dates for, well, dates". The ones left out in the
cold are those wondering why they use "generate_series" for timestamp
series with a finer precision than date. I'd be willing to offer a
"generate_timestamps" to placate them. And might as well toss in
"generate_numbers" for completeness - though now I likely doom the
proposal...so I'll stick with just add generate_dates so the behavior is
possible without superfluous casting or the need to write a custom
function. Dates are too common a unit of measure to leave working with
them this cumbersome.
David J.
On 3/17/16 11:55 AM, David G. Johnston wrote:
On Thu, Mar 17, 2016 at 8:41 AM, David Steele <david@pgmasters.net
<mailto:david@pgmasters.net>>wrote:Not sure I agree. My point was that even if developers were pretty
careful with their casting (or are using two actual dates) then there's
still possibility for breakage.With the first argument casted to date it doesn't matter whether you
cast the second argument as the pseudo-type anyelement will take its
value from the first argument and force the second to date.
Ah, I see.
The main problem is that the system tries to cast unknown to integer
based upon finding gs(date, date, integer) and fails without ever
considering that gs(ts, ts, interval) would succeed.
I'm tempted to say, "why don't we just fix that?" but I'm sure any
changes in that area would introduce a variety of new and interesting
behaviors.
So let the user decide whether to trade-off learning a new function
name but getting dates instead of timestamps or going through the hassle
of casting.For me, it would have been a nice bonus if the generate_series() could
be used directly but I feel that the desired functionality is desirable
and the name itself is of only minor consideration.I can see that newbies might ponder why two functions exist but they
should understand "backward compatibility".I suspect that most people will simply think: "use generate_series for
numbers and use generate_dates for, well, dates". The ones left out in
the cold are those wondering why they use "generate_series" for
timestamp series with a finer precision than date. I'd be willing to
offer a "generate_timestamps" to placate them. And might as well toss
in "generate_numbers" for completeness - though now I likely doom the
proposal...so I'll stick with just add generate_dates so the behavior is
possible without superfluous casting or the need to write a custom
function. Dates are too common a unit of measure to leave working with
them this cumbersome.
I'm not finding this argument persuasive.
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
David Steele <david@pgmasters.net> writes:
On 3/17/16 11:30 AM, David G. Johnston wrote:
I'd call it "generate_dates(...)" and be done with it.
We would then have:
generate_series()
generate_subscripts()
generate_dates()
To me this completely negates the idea of this "just working" which is
why it got a +1 from me in the first place. If I have to remember to
use a different function name then I'd prefer to just cast on the
timestamp version of generate_series().
Yeah, this point greatly weakens the desirability of this function IMO.
I've also gone from "don't care" to "-1".
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 Thu, Mar 17, 2016 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Steele <david@pgmasters.net> writes:
On 3/17/16 11:30 AM, David G. Johnston wrote:
I'd call it "generate_dates(...)" and be done with it.
We would then have:
generate_series()
generate_subscripts()
generate_dates()To me this completely negates the idea of this "just working" which is
why it got a +1 from me in the first place. If I have to remember to
use a different function name then I'd prefer to just cast on the
timestamp version of generate_series().Yeah, this point greatly weakens the desirability of this function IMO.
I've also gone from "don't care" to "-1".regards, tom lane
Since that diminishes the already moderate support for this patch, I'll
withdraw it.
David Steele <david@pgmasters.net> writes:
On 3/17/16 11:55 AM, David G. Johnston wrote:
With the first argument casted to date it doesn't matter whether you
cast the second argument as the pseudo-type anyelement will take its
value from the first argument and force the second to date.
Ah, I see.
FWIW, there isn't any "anyelement" involved here, AFAICT. The set
of functions we have today is:
regression=# \df generate*
List of functions
Schema | Name | Result data type | Argument data types | Type
------------+---------------------+-----------------------------------+--------------------------------------------------------------------+--------
pg_catalog | generate_series | SETOF bigint | bigint, bigint | normal
pg_catalog | generate_series | SETOF bigint | bigint, bigint, bigint | normal
pg_catalog | generate_series | SETOF integer | integer, integer | normal
pg_catalog | generate_series | SETOF integer | integer, integer, integer | normal
pg_catalog | generate_series | SETOF numeric | numeric, numeric | normal
pg_catalog | generate_series | SETOF numeric | numeric, numeric, numeric | normal
pg_catalog | generate_series | SETOF timestamp with time zone | timestamp with time zone, timestamp with time zone, interval | normal
pg_catalog | generate_series | SETOF timestamp without time zone | timestamp without time zone, timestamp without time zone, interval | normal
pg_catalog | generate_subscripts | SETOF integer | anyarray, integer | normal
pg_catalog | generate_subscripts | SETOF integer | anyarray, integer, boolean | normal
(10 rows)
Now, generate_subscripts is a totally different function that *should*
have a separate name; it does not take a couple of endpoints. That's
not much of an argument for inventing different names for some of
the functions that do work with a pair of endpoints.
The real problem is that if we invent generate_series(date,date,integer)
then, given the problem "generate_series(date,unknown,unknown)"
the parser's ambiguous-function rules[1]http://www.postgresql.org/docs/devel/static/typeconv-func.html will resolve that as
generate_series(date,date,integer), on the grounds that that provides
more exact type matches (i.e. 1) than any of the other alternatives.
Previously, the same call would have been resolved as
generate_series(timestamptz,timestamptz,interval) on the grounds that
that's reachable by casting and timestamptz is a preferred type.
So if you had written such a call with an interval literal and didn't
bother to cast the literal, your code would break.
We could avoid that problem if the new function were defined as
generate_series(date,date,interval), but then I'm not certain
what the code ought to do if the given interval is not a multiple
of a day.
One idea that might be worth considering is to define the function
as generate_series(date,date,interval) returns timestamp (without
time zone). The point here would be only to move the behavior for
date inputs as far as getting timestamp without tz rather than
timestamp with tz; which would at least save some timezone rotations
in typical use, as well as rather debatable semantics. (The fact
that timestamptz is the preferred type in this hierarchy isn't
really doing us any favors here.)
regards, tom lane
[1]: http://www.postgresql.org/docs/devel/static/typeconv-func.html
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 17, 2016 at 12:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
One idea that might be worth considering is to define the function
as generate_series(date,date,interval) returns timestamp (without
time zone). The point here would be only to move the behavior for
date inputs as far as getting timestamp without tz rather than
timestamp with tz; which would at least save some timezone rotations
in typical use, as well as rather debatable semantics. (The fact
that timestamptz is the preferred type in this hierarchy isn't
really doing us any favors here.)
That's a fairly tenuous benefit, though, and a substantially different
patch. I think it's time to give up here and move on. We can revisit
this for another release after we've had more time to think about it,
if that seems like a smart thing to do when the time comes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/17/16 1:17 PM, Robert Haas wrote:
On Thu, Mar 17, 2016 at 12:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
One idea that might be worth considering is to define the function
as generate_series(date,date,interval) returns timestamp (without
time zone). The point here would be only to move the behavior for
date inputs as far as getting timestamp without tz rather than
timestamp with tz; which would at least save some timezone rotations
in typical use, as well as rather debatable semantics. (The fact
that timestamptz is the preferred type in this hierarchy isn't
really doing us any favors here.)That's a fairly tenuous benefit, though, and a substantially different
patch. I think it's time to give up here and move on. We can revisit
this for another release after we've had more time to think about it,
if that seems like a smart thing to do when the time comes.
This has been marked "returned with feedback".
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Mar 17, 2016 at 12:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
One idea that might be worth considering is to define the function
as generate_series(date,date,interval) returns timestamp (without
time zone). The point here would be only to move the behavior for
date inputs as far as getting timestamp without tz rather than
timestamp with tz; which would at least save some timezone rotations
in typical use, as well as rather debatable semantics.
That's a fairly tenuous benefit, though, and a substantially different
patch.
Well, some folks were already of the opinion that the patch's benefits
were tenuous ;-)
I think it's time to give up here and move on.
Agreed, letting this go for now seems like the right move. Maybe
someone will have a bright idea later.
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