Re: [PATCHES] 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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/func.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.267 diff -c -c -r1.267 func.sgml ***
Re: [PATCHES] Interval-day patch
Bruce Momjian pgman@candle.pha.pa.us writes: 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? That's a completely separate TODO item. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Interval-day patch
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: 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? That's a completely separate TODO item. OK. Is it clear enough? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Interval-day patch
I am close to completing work on this patch and will post an updated version in a few days. --- 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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] Interval-day patch
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 interval_day.diff Description: Binary data ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly