On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker <corey.huin...@gmail.com> wrote:
> On Thu, Mar 17, 2016 at 10:00 AM, David Steele <da...@pgmasters.net> > wrote: > >> On 3/17/16 4:49 AM, Dean Rasheed wrote: >> >> > On 16 March 2016 at 23:32, David Steele <da...@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... > >> 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 >> da...@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. > > 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.