Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Gavin Flower

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  
wrote:

But I still don't know "meh" means.

Maybe this helps?

https://en.wikipedia.org/wiki/Meh


LOL
https://en.wikipedia.org/wiki/LOL



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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread David G. Johnston
On Thu, Mar 10, 2016 at 11:45 AM, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
>  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?


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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Igal @ Lucee.org

On 3/10/2016 11:44 AM, Robert Haas wrote:

On Thu, Mar 10, 2016 at 2:35 PM, Simon Riggs  wrote:

But I still don't know "meh" means.

Maybe this helps?

https://en.wikipedia.org/wiki/Meh


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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 2:35 PM, Simon Riggs  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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Simon Riggs
On 10 March 2016 at 18:45, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
>  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.


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 Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
 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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Corey Huinker
removed leftover development comment

On Thu, Mar 10, 2016 at 11:02 AM, Corey Huinker 
wrote:

> On Thu, Mar 10, 2016 at 10:58 AM, Robert Haas 
> wrote:
>
>> On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
>> wrote:
>> > On 10 March 2016 at 06:53, Michael Paquier 
>> > wrote:
>> >>
>> >> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
>> >>  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
>
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
   
  
 
+ 
+  generate_series(start, 
stop)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of one day
+  
+ 
+
+ 
+  generate_series(start, 
stop, step 
integer)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of step days
+  
+ 
+
 

   
@@ -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)
 
   
 
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,
+   

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread David G. Johnston
On Thu, Mar 10, 2016 at 9:30 AM, Michael Paquier 
wrote:

> On Thu, Mar 10, 2016 at 4:58 PM, Robert Haas 
> wrote:
> > On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
> wrote:
> >> On 10 March 2016 at 06:53, Michael Paquier 
> >> wrote:
> >>>
> >>> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >>>  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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread David G. Johnston
On Thu, Mar 10, 2016 at 8:58 AM, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
> wrote:
> > On 10 March 2016 at 06:53, Michael Paquier 
> > wrote:
> >>
> >> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >>  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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Michael Paquier
On Thu, Mar 10, 2016 at 4:58 PM, Robert Haas  wrote:
> On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs  wrote:
>> On 10 March 2016 at 06:53, Michael Paquier 
>> wrote:
>>>
>>> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
>>>  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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Alvaro Herrera
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 Herrerahttp://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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Corey Huinker
On Thu, Mar 10, 2016 at 10:58 AM, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
> wrote:
> > On 10 March 2016 at 06:53, Michael Paquier 
> > wrote:
> >>
> >> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >>  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
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
   
  
 
+ 
+  generate_series(start, 
stop)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of one day
+  
+ 
+
+ 
+  generate_series(start, 
stop, step 
integer)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of step days
+  
+ 
+
 

   
@@ -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)
 
   
 
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")));
+   }
+
+   /* 

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs  wrote:
> On 10 March 2016 at 06:53, Michael Paquier 
> wrote:
>>
>> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
>>  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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Simon Riggs wrote:
> > On 10 March 2016 at 06:53, Michael Paquier 
> > wrote:
> > 
> > > On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> > >  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


signature.asc
Description: Digital signature


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Alvaro Herrera
Simon Riggs wrote:
> On 10 March 2016 at 06:53, Michael Paquier 
> wrote:
> 
> > On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >  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 Herrerahttp://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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Simon Riggs
On 10 March 2016 at 06:53, Michael Paquier 
wrote:

> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
>  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 Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Alvaro Herrera
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 Herrerahttp://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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 1:53 AM, Michael Paquier
 wrote:
> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
>  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 Riggs

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?

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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-09 Thread Michael Paquier
On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
 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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-08 Thread Alvaro Herrera
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 Herrerahttp://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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-08 Thread David G. Johnston
On Tue, Mar 8, 2016 at 3:57 PM, Corey Huinker 
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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-08 Thread Corey Huinker
>
> > 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.


>
> + 
> +
> generate_series(start,
> stop, step
> integer)
> +  date
> +  setof date
> +  
> +   Generate a series of values, from start
> to stop
> +   with a step size of step
>
> 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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-08 Thread Robert Haas
On Fri, Mar 4, 2016 at 3:17 PM, Corey Huinker  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.

+ 
+  generate_series(start,
stop, step
integer)
+  date
+  setof date
+  
+   Generate a series of values, from start
to stop
+   with a step size of step

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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-03-04 Thread Corey Huinker
>
>
> 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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-02-23 Thread Tomas Vondra

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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-02-22 Thread Corey Huinker
>
> >
> > 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?


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-02-21 Thread Michael Paquier
On Mon, Feb 22, 2016 at 6:52 AM, Tom Lane  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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-02-21 Thread Tom Lane
Christoph Berg  writes:
> Re: David Fetter 2016-01-26 <20160126180011.ga16...@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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-02-21 Thread Christoph Berg
Re: David Fetter 2016-01-26 <20160126180011.ga16...@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
-- 
c...@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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-27 Thread Torsten Zühlsdorff

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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Torsten Zuehlsdorff


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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Corey Huinker
On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier 
wrote:

> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>  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
>
>
> --
> 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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread David Fetter
On Tue, Jan 26, 2016 at 09:53:26PM +0900, Michael Paquier wrote:
> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>  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  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Euler Taveira
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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Corey Huinker
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 
wrote:

> On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier <
> michael.paqu...@gmail.com> wrote:
>
>> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>>  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
>>
>>
>> --
>> 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.
>
>


v2.generate_series_date.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Alvaro Herrera
Simon Riggs wrote:
> On 25 January 2016 at 09:55, Tomas Vondra 
> 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 Herrerahttp://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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Michael Paquier
On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
 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
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');

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-25 Thread Simon Riggs
On 25 January 2016 at 09:55, Tomas Vondra 
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 Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-25 Thread Corey Huinker
On Mon, Jan 25, 2016 at 2:07 AM, Tom Lane  wrote:

> Corey Huinker  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!


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-25 Thread Michael Paquier
On Mon, Jan 25, 2016 at 6:55 PM, Tomas Vondra
 wrote:
> On 01/25/2016 07:22 AM, Tom Lane wrote:
>>
>> Michael Paquier  writes:
>>>
>>> On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker 
>>> wrote:

 One thing I discovered in doing this patch is that if you do a timestamp
 generate_series involving infinityit 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 100? 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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-25 Thread Tomas Vondra

On 01/25/2016 07:22 AM, Tom Lane wrote:

Michael Paquier  writes:

On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker  wrote:

One thing I discovered in doing this patch is that if you do a timestamp
generate_series involving infinityit 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 100? 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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-24 Thread Corey Huinker
>
>
> 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.


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-24 Thread Tom Lane
Corey Huinker  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


[HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-24 Thread Corey Huinker
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 infinityit 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.
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
   
  
 
+ 
+  generate_series(start, 
stop)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of one day
+  
+ 
+
+ 
+  generate_series(start, 
stop, step 
integer)
+  date
+  setof date
+  
+   Generate a series of values, from start to 
stop
+   with a step size of step
+  
+ 
+
 

   
@@ -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)
 
   
 
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);
+
+

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-24 Thread Michael Paquier
On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker  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 infinityit 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


Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-24 Thread Tom Lane
Michael Paquier  writes:
> On Mon, Jan 25, 2016 at 3:00 PM, Corey Huinker  
> wrote:
>> One thing I discovered in doing this patch is that if you do a timestamp
>> generate_series involving infinityit 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 100?  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