Re: [PATCHES] Interval-day patch

2005-07-20 Thread Bruce Momjian

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

2005-07-20 Thread Tom Lane
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

2005-07-20 Thread Bruce Momjian
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

2005-07-16 Thread Bruce Momjian

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

2005-06-27 Thread Michael Glaesemann
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