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