Re: Interval->day patch

I have applied this patch with significant adjustments. I changed your
"simplify" function into two new functions, justify_hours() and
justify_days(), to handle the adjustment of interval values to hours <
24 and days < 30. Do we want to separate functions?

I used date2j and j2date to add days to the interval value (you used a
comment as a place-holder). I also went through all the Interval
mentions and made sure everything was handling the new 'day' field
properly.

SELECT '2005-04-03 00:00:00'::timestamp WITH TIME ZONE + '1 day';
?column?
------------------------
2005-04-04 00:00:00-04

SELECT '2005-04-03 00:00:00'::timestamp WITH TIME ZONE + '24 hours';
?column?
------------------------
2005-04-04 01:00:00-04

This looks a little strange:

SELECT '2005-04-04 00:00:00'::timestamp with time zone - '2005-04-03 00:00:00'::timestamp with time zone;
----------
23:00:00
(1 row)

SELECT '2005-04-04 01:00:00'::timestamp with time zone - '2005-04-03 00:00:00'::timestamp with time zone;
?column?
----------
1 day

When you subtract two timestamps, do we return the hours or days of
difference? What happens now is the difference is in hours/time, and
hours are rolled up into days. Is this what we want?

We have this TODO item:

o Allow TIMESTAMP WITH TIME ZONE to store the original timezone
information, either zone name or offset from UTC [timezone]

If the TIMESTAMP value is stored with a time zone name, interval
computations should adjust based on the time zone rules.

It was originally added so we could distinguish 24 hours from 1 day. Do
we still need this TODO?

---------------------------------------------------------------------------

Michael Glaesemann wrote:
> Please find attached a patch which adds a day field to the interval
> struct so that we can treat INTERVAL '1 day' differently from
> INTERVAL '24 hours' in DST-aware situations. It also includes a
> function called interval_simplify() which takes an interval argument
> and returns an interval where hours over 24 are promoted to days, e.g.,
>
> template1=# select interval_simplify('3 months -11 days 79 hours 2
> minutes'::interval);
> interval_simplify
> --------------------------
> 3 mons -7 days -16:58:00
> (1 row)
>
> If anyone has better ideas for the name of this function, please let
> me know.
>
> I've modified the regression tests, but still need to add additional
> tests for the interval_simplify function, and I want to add a few
> more tests for the new interval behavior. Also, the docs will need to
> be updated to mention the new behavior. I plan on doing this in over
> the next couple of days.
>
> This is some of the first C I've hacked, and the first patch I've
> submitted that's more than a documentation or a simple one-liner (and
> even that one got worked over pretty good :) ), so I fully expect
> some mistakes to be found. Please let me know and I'll do my best to
> fix them.
>
> In timestamp.c, I suspect that AdjustIntervalForTypmod,
> interval_scale will need some modifications, though I'm not quite
> sure what this code is doing. I've left them as-is. I've made some
> changes to interval2tm, but believe that the changes I've made may
> not be adequate. Given sufficient instruction, I'll be happy to make
> the necessary changes to these functions.
>
> A few things I noticed while I was working:
>
> In interval_mul and interval_div, I'm wondering whether 30.0 and 24.0
> shouldn't be substituted for 30 and 24 in the non-integer-timestamp
> code path, as these are floats. Perhaps it doesn't make a difference
> for multiplication, but I see similar usage in interval_cmp_interval.
> I've left the code as-is.
>
> In the deconstruct_array calls in interval_accum and interval_avg,
> the size of interval is passed as a magic number (16). I think this
> could be abstracted out, such as #define SIZEOF_INTERVAL 16 to make
> the code a bit more robust (albeit just a little). Is this a
> reasonable change?
>
> Michael Glaesemann
> grzm myrealbox com
>

[ Attachment, skipping... ]

>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073