On 2020-06-30 06:34, John Naylor wrote:
> In v9, I've simplified the patch somewhat to make it easier for future
> work to build on.
>
> - When truncating on month-or-greater intervals, require the origin to
> align on month. This removes the need to handle weird corner cases
> that have no straightforward behavior.
> - Remove hackish and possibly broken code to allow origin to be after
> the input timestamp. The default origin is Jan 1, 1 AD, so only AD
> dates will behave correctly by default. This is not enforced for now,
> since it may be desirable to find a way to get this to work in a nicer
> way.
> - Rebase docs over PG13 formatting changes.
This looks pretty solid now. Are there any more corner cases and other
areas with unclear behavior that you are aware of?
A couple of thoughts:
- After reading the discussion a few times, I'm not so sure anymore
whether making this a cousin of date_trunc is the right way to go. As
you mentioned, there are some behaviors specific to date_trunc that
don't really make sense in date_trunc_interval, and maybe we'll have
more of those. Also, date_trunc_interval isn't exactly a handy name.
Maybe something to think about. It's obviously fairly straightforward
to change it.
- There were various issues with the stride interval having months and
years. I'm not sure we even need that. It could be omitted unless you
are confident that your implementation is now sufficient.
- Also, negative intervals could be prohibited, but I suppose that
matters less.
- I'm curious about the origin being set to 0001-01-01. This seems to
work correctly in that it sets the origin to a Monday, which is what we
wanted, but according to Google that day was a Saturday. Something to
do with Julian vs. Gregorian calendar? Maybe we should choose a date
that is a bit more recent and easier to reason with.
- Then again, I'm thinking that maybe we should make the origin
mandatory. Otherwise, the default answers when having strides larger
than a day are entirely arbitrary, e.g.,
=> select date_trunc_interval('10 year', '0196-05-20 BC'::timestamp);
0190-01-01 00:00:00 BC
=> select date_trunc_interval('10 year', '0196-05-20 AD'::timestamp);
0191-01-01 00:00:00
Perhaps the origin could be defaulted if the interval is less than a day
or something like that.
- I'm wondering whether we need the date_trunc_interval(interval,
timestamptz, timezone) variant. Isn't that the same as
date_trunc_interval(foo AT ZONE xyz, value)?
