Interval->day proposal
There has been discussion in the past of including number of days as
part of the Interval structure in addition to the current months and
time components. Here are some mailing list threads where the topic
has arisen.
[Re: [GENERAL] '1 year' = '360 days' ????](http://
archives.postgresql.org/pgsql-general/2004-10/msg01104.php)
[Re: [HACKERS] timestamp with time zone a la sql99](http://
archives.postgresql.org/pgsql-hackers/2004-10/msg00843.php)
One advantage of this is that it would allow '1 day' to have a
different meaning that '24 hours', which would be meaningful when
crossing daylight saving time changes. For example, PostgreSQL
returns the following results:
test=# set time zone 'CST7CDT';
SET
test=# select '2005-04-01 12:00-07'::timestamptz + '1 day'::interval;
?column?
------------------------
2005-04-02 12:00:00-07
(1 row)
test=# select '2005-04-02 12:00-07'::timestamptz + '1 day'::interval;
?column?
------------------------
2005-04-03 13:00:00-06
(1 row)
A daylight saving time change occurred at 2005-04-03 02:00.
Internally, the '1 day' interval is converted to 24 hours, which is
then added to '2005-04-02 12:00-07', returning '2005-04-03 13:00-06'.
Distinguishing between '1 day' and '24 hours' would allow '2005-04-02
12:00-07'::timestamptz + '1 day'::interval to return '2005-04-03
12:00-06', while '2005-04-02 12:00-07'::timestamptz + '1
day'::interval would return '2005-04-03 13:00-06' as it does now.
I'm interested in implementing at least part of this. In my mind
there are two steps: the first is to modify the Interval data type
implementation to include days as well as months and time; the second
is to modify the functions used to add interval to timestamptz to
take into account the difference between '1 day' and '24 hours' when
working across daylight saving time changes. I'm thinking of it in
two steps primarily because this is my first venture into the backend
code and I'm breaking it into pieces that I hopefully deal with.
Arguably, implementing only the first step isn't useful in and of
itself, but I have to start somewhere, and that seems like the
logical place to start.
I've been looking at the current interval implementation, and am
beginning to form a plan for my first cut at the code. Before
starting out, I'd appreciate any feedback on my understanding of the
code or my plan.
In interval_in, (backend/utils/adt/timestamp.c) strings representing
intervals are parsed using DecodeInterval() which is defined in
backend/utils/adt/datetime.c. DecodeInterval() assigns values to the
appropriate components of a pg_tm structure, which includes tm_year,
tm_mon, tm_mday, tm_hour, tm_min, and tm_sec. This pg_tm data is
passed to tm2interval, where the tm_year and tm_mon components are
used to determine the interval->month value, and the tm_mday,
tm_hour, tm_min, and tm_sec values are used to determine the interval-
time value.
When the string is read by DecodeInterval, the "days" component is
assigned to tm_mday. It seems relatively straightforward to use this
input to provide the interval->day value. However, I'm wondering what
range of days this the interval->day component can be expected to
handle. tm_mday is an int value, which is only guaranteed to be 2
bytes (though it may be larger), if I understand correctly. This
means that tm_mday isn't able to hold the full span of days, which is
nearly 2.15 billion over the range of valid timestamps (from BC 4713
to AD 5,874,897).
However, this range of days may not be necessary in practice. The
(minimum) 2 bytes guaranteed by int is enough to hold +/- 44 years of
days. Is it likely that DST issues are going to be important over
anything larger? I'm not sure, and I welcome others' thoughts on
this. If a wider range of days is deemed required, I'm not sure how
this would be accomplished outside of changing the pg_tm datatype.
If interval->day can be stored in an int16, then the interval struct
would be widened from 12 to 14 bytes. Are there concerns about
widening the interval datatype? This is perhaps a naive approach, so
if anyone has other ideas, I'd love to hear them.
Is my understanding correct? Any corrections or pointers appreciated.
Michael Glaesemann
grzm myrealbox com
Michael Glaesemann <grzm@myrealbox.com> writes:
When the string is read by DecodeInterval, the "days" component is
assigned to tm_mday. It seems relatively straightforward to use this
input to provide the interval->day value. However, I'm wondering what
range of days this the interval->day component can be expected to
handle. tm_mday is an int value, which is only guaranteed to be 2
bytes (though it may be larger), if I understand correctly.
Actually, practically all of the Postgres code assumes int is at least
32 bits. Feel free to change pg_tm's field to be declared int32 instead
of just int if that bothers you, but it is really quite academic.
If interval->day can be stored in an int16, then the interval struct
would be widened from 12 to 14 bytes. Are there concerns about
widening the interval datatype?
I'd make the on-disk field an int32, taking the struct to 16 bytes.
Given that it already requires double alignment for the embedded
double-or-int8 field, it's likely that it effectively takes 16 bytes
anyway after you count padding effects. So I don't see any percentage
in trying to shave a couple bytes.
regards, tom lane
Michael,
One advantage of this is that it would allow '1 day' to have a
different meaning that '24 hours', which would be meaningful when
crossing daylight saving time changes. For example, PostgreSQL
returns the following results:
I've been stumping for this for years. See my arguments with Thomas Lockhart
in 2000. A "calendar day" is not the same as 24 hours, and DST behavior has
forced me to use TIMESTAMP WITHOUT TIME ZONE on many a calendaring
application.
Unfortunately, it appears that tri-partitioning INTERVAL ( year/month ;
week/day ; hour/minute/second ) is a violation of the SQL spec which has only
the two partitions ( year/month ; week/day/hour/minute/second ). Have they
changed this in SQL 2003? If not, do we want to do it anyway, perhaps
using a 2nd interval type?
--
Josh Berkus
Aglio Database Solutions
San Francisco
Josh Berkus <josh@agliodbs.com> writes:
Unfortunately, it appears that tri-partitioning INTERVAL ( year/month ;
week/day ; hour/minute/second ) is a violation of the SQL spec which has only
the two partitions ( year/month ; week/day/hour/minute/second ).
I think it's an extension of the spec, not a violation. In
particular, if you were working in a daylight-savings-less timezone,
you could not tell the difference (could you?) The spec's worldview
essentially corresponds to daylight-savings-less all the time, and
so they are already a subset of what we do.
regards, tom lane
On May 31, 2005, at 12:48 AM, Tom Lane wrote:
Michael Glaesemann <grzm@myrealbox.com> writes:
tm_mday is an int value, which is only guaranteed to be 2
bytes (though it may be larger), if I understand correctly.Actually, practically all of the Postgres code assumes int is at least
32 bits. Feel free to change pg_tm's field to be declared int32
instead
of just int if that bothers you, but it is really quite academic.
Thanks for the clarification. My instinct would be to change so that
it's no longer just an assumption. Is there any benefit to changing
the other pg_tm int fields to int32? I imagine int is used quite a
bit throughout the code, and I'd think assuming 32-bit ints would
have bitten people in the past if it were invalid, so perhaps
changing them is unnecessary.
I'd make the on-disk field an int32, taking the struct to 16 bytes.
Will do.
Thanks for you comments.
Michael Glaesemann
grzm myrealbox com
On May 31, 2005, at 1:40 AM, Tom Lane wrote:
Josh Berkus <josh@agliodbs.com> writes:
Unfortunately, it appears that tri-partitioning INTERVAL ( year/
month ;
week/day ; hour/minute/second ) is a violation of the SQL spec
which has only
the two partitions ( year/month ; week/day/hour/minute/second ).I think it's an extension of the spec, not a violation. In
particular, if you were working in a daylight-savings-less timezone,
you could not tell the difference (could you?)
I've started working on this change, and one difference has shown up
immediately in the regression tests. v8.0.3 currently returns:
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
9 years...
----------------------------------
! 9 years 1 mon -11 days -10:46:00
(1 row)
With my first round of changes,
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
9 years...
----------------------------------
! 9 years 1 mon -12 days +13:14:00
(1 row)
These are equivalent in both CVS and my branch, as '1 day'::interval
= '24 hours'::interval. I haven't checked the SQL spec yet (and
intend to do so), but is there a canonical form for intervals that we
need to return? I can imagine there might be, and if the spec
considers only months and time, I could see where they might want
days and time to have the same sign, and put results in "simplest
form". Even if the spec doesn't require it, the behavior is
definitely changed even outside of DST-aware code.
-- v8.0.3
test=# select '9 years 1 mon -11 days -10:46:00'::interval;
interval
----------------------------------
9 years 1 mon -11 days -10:46:00
(1 row)
test=# select '9 years 1 mon -12 days +13:14:00'::interval;
interval
----------------------------------
9 years 1 mon -11 days -10:46:00
(1 row)
test=# select '25 hours'::interval;
interval
----------------
1 day 01:00:00
(1 row)
-- new interval code
test=# select ' 9 years 1 mon -11 days -10:46:00'::interval;
interval
----------------------------------
9 years 1 mon -11 days -10:46:00
(1 row)
test=# select '9 years 1 mon -12 days +13:14:00'::interval;
interval
----------------------------------
9 years 1 mon -12 days +13:14:00
(1 row)
test=# select '25 hours'::interval;
interval
----------
25:00:00
(1 row)
I'll be digging into the spec later and post what I find. Thoughts?
Michael Glaesemann
grzm myrealbox com
Michael Glaesemann <grzm@myrealbox.com> writes:
On May 31, 2005, at 12:48 AM, Tom Lane wrote:
Actually, practically all of the Postgres code assumes int is at least
32 bits. Feel free to change pg_tm's field to be declared int32
instead
of just int if that bothers you, but it is really quite academic.
Thanks for the clarification. My instinct would be to change so that
it's no longer just an assumption. Is there any benefit to changing
the other pg_tm int fields to int32? I imagine int is used quite a
bit throughout the code, and I'd think assuming 32-bit ints would
have bitten people in the past if it were invalid, so perhaps
changing them is unnecessary.
As I understand it, the received wisdom of the C community is that
"int" means the machine's natural, most efficient word width. The
C specification was written at a time when a fair percentage of hardware
thought that meant int16 (and I do remember programming such hardware).
But there are no longer any machines ... or at least none on which you'd
want to run Postgres ... for which int means int16; today I'd assume
that int means "probably int32, maybe int64 if that's really faster
on this machine".
regards, tom lane
On Jun 1, 2005, at 1:42 PM, Michael Glaesemann wrote:
-- v8.0.3
test=# select '25 hours'::interval;
interval
----------------
1 day 01:00:00
(1 row)-- new interval code
test=# select '25 hours'::interval;
interval
----------
25:00:00
(1 row)I'll be digging into the spec later and post what I find. Thoughts?
I've dug a bit, and this is definitely not spec compliant, as
interval hours must be in the range 0-23. From Section 4.6.3 of SQL/
Foundation (2003 draft)
Table 6 — Valid values for fields in INTERVAL values
Keyword Valid values of INTERVAL fields
YEAR Unconstrained except by <interval leading field precision>
MONTH Months (within years) (0-11)
DAY Unconstrained except by <interval leading field precision>
HOUR Hours (within days) (0-23)
MINUTE Minutes (within hours) (0-59)
SECOND Seconds (within minutes) (0-59.999...)
I'm not quite sure what the parenthetical "within days" adds to the
definition. However, the point of keeping a separate day field is so
we can keep the number of days separate from the number of 24-hour
units, and limiting the range of HOUR between 0 and 23 makes that
impossible, it appears.
I'd be happy if someone can let me know I'm reading the spec wrong!
Michael Glaesemann
grzm myrealbox com
Michael Glaesemann <grzm@myrealbox.com> writes:
On Jun 1, 2005, at 1:42 PM, Michael Glaesemann wrote:
-- v8.0.3
test=# select '25 hours'::interval;
interval
----------------
1 day 01:00:00
(1 row)-- new interval code
test=# select '25 hours'::interval;
interval
----------
25:00:00
(1 row)I'll be digging into the spec later and post what I find. Thoughts?
I've dug a bit, and this is definitely not spec compliant, as
interval hours must be in the range 0-23.
Doesn't bother me. The spec says what results you must get from
spec-compliant input; I don't think it says we may take only
spec-compliant input. (If we were to read it that way, we'd have
to rip out every PG extension, not only the interval-related ones.)
The entire *point* of this change is to be able to distinguish
"25 hours" from "1 day 1 hour", so you can hardly argue that being
able to do that is not what we want it to do...
regards, tom lane
Michael Glaesemann <grzm@myrealbox.com> writes:
I've started working on this change, and one difference has shown up
immediately in the regression tests. v8.0.3 currently returns:
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
9 years...
----------------------------------
! 9 years 1 mon -11 days -10:46:00
(1 row)
With my first round of changes,
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
9 years...
----------------------------------
! 9 years 1 mon -12 days +13:14:00
(1 row)
Right, because it is now possible for day and hour to have different signs.
Comes with the territory I think.
These are equivalent in both CVS and my branch, as '1 day'::interval
= '24 hours'::interval. I haven't checked the SQL spec yet (and
intend to do so), but is there a canonical form for intervals that we
need to return?
AFAICS, the spec only allows one sign for the entire interval value,
so this behavior is already outside the spec. As long as we don't
generate different field signs in cases where that was not already
present in the input, I don't think it's a spec violation.
regards, tom lane