pgman wrote:
> Marc G. Fournier wrote:
> > On Thu, 21 Jul 2005, Tom Lane wrote:
> > 
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > >> Tom Lane wrote:
> > >>>> #define DAYS_PER_YEAR   365.25
> > >>>> #define MONTHS_PER_YEAR 12
> > >>>> #define DAYS_PER_MONTH  30
> > >>>> #define HOURS_PER_DAY   24
> > >>>
> > >>> Considering that only one of these four is actually an accurate
> > >>> constant, I have to question the usefulness of this.
> > >
> > >> Yea, the problem is that these non-absolute constants are littered all
> > >> through the code, so it is best to have them at least in one place.  I
> > >> will add a comment marking the non-accurate ones more clearly.
> > >
> > > Unless you comment every single use of the macros, you won't have
> > > addressed my point.  No one will ever read "DAYS_PER_YEAR" in the midst
> > > of some code and not stop to wonder "hmm, is that 365, or 365.25, or
> > > 365.2425"?  And in most places where this might be used, that's not
> > > an ignorable issue.  I think it is actually better to write out the
> > > literal number, because then you can see right away what is happening.
> > >
> > > In short, I don't think this is an improvement.
> > 
> > 'k, I have to be missing something here, but other then, what, 5 months of 
> > the year (not even half), DAYS_PER_MONTH isn't 30 either ... this can't be 
> > good, especially not for a database ... that's like saying "oh, pi is 
> > around 3.2" (assuming .05 rounds up to 2 instead of down to 1) ... the 
> > conversions would only be right 5/12ths of the time ...
> > 
> > Or am I missing something?
> 
> No, you are not.  Our using 30 is pretty wacky, but when we need to
> convert from months to days/time, that's the only way we can do it.
> 
> At least with the macro, we can now know every place we make that
> assumption.

I have added this comment above the DAYS_PER_MONTH macro:

        + /*
        +  *    DAYS_PER_MONTH is very imprecise.  The more accurate value is
        +  *    365.25/12 = 30.4375, or '30 days 10:30:00'.  Right now we only
        +  *    return an integral number of days, but someday perhaps we should
        +  *    also return a 'time' value to be used as well.
        +  */
          #define DAYS_PER_MONTH        30              /* assumes exactly 30 
days per month */

Let me add that we could actually do this in many places now because we
are already converting to 'time' in those places.  Is this a TODO?

-- 
  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 9: In versions below 8.0, the planner will ignore your desire to
       choose an index scan if your joining column's datatypes do not
       match

Reply via email to