Обсуждение: pgsql: Add time/date macros for code clarity: #define DAYS_PER_YEAR
pgsql: Add time/date macros for code clarity: #define DAYS_PER_YEAR
От
momjian@svr1.postgresql.org (Bruce Momjian)
Дата:
Log Message:
-----------
Add time/date macros for code clarity:
#define DAYS_PER_YEAR 365.25
#define MONTHS_PER_YEAR 12
#define DAYS_PER_MONTH 30
#define HOURS_PER_DAY 24
Modified Files:
--------------
pgsql/src/backend/commands:
variable.c (r1.110 -> r1.111)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/variable.c.diff?r1=1.110&r2=1.111)
pgsql/src/backend/postmaster:
postmaster.c (r1.459 -> r1.460)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/postmaster.c.diff?r1=1.459&r2=1.460)
syslogger.c (r1.16 -> r1.17)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/syslogger.c.diff?r1=1.16&r2=1.17)
pgsql/src/backend/tcop:
postgres.c (r1.454 -> r1.455)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c.diff?r1=1.454&r2=1.455)
pgsql/src/backend/utils/adt:
date.c (r1.113 -> r1.114)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/date.c.diff?r1=1.113&r2=1.114)
datetime.c (r1.152 -> r1.153)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/datetime.c.diff?r1=1.152&r2=1.153)
formatting.c (r1.91 -> r1.92)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/formatting.c.diff?r1=1.91&r2=1.92)
nabstime.c (r1.136 -> r1.137)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/nabstime.c.diff?r1=1.136&r2=1.137)
selfuncs.c (r1.185 -> r1.186)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/selfuncs.c.diff?r1=1.185&r2=1.186)
timestamp.c (r1.134 -> r1.135)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/timestamp.c.diff?r1=1.134&r2=1.135)
pgsql/src/backend/utils/misc:
guc.c (r1.274 -> r1.275)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/guc.c.diff?r1=1.274&r2=1.275)
pgsql/src/include/utils:
timestamp.h (r1.47 -> r1.48)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/timestamp.h.diff?r1=1.47&r2=1.48)
pgsql/src/interfaces/ecpg/pgtypeslib:
datetime.c (r1.22 -> r1.23)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/pgtypeslib/datetime.c.diff?r1=1.22&r2=1.23)
dt.h (r1.23 -> r1.24)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/pgtypeslib/dt.h.diff?r1=1.23&r2=1.24)
dt_common.c (r1.25 -> r1.26)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/pgtypeslib/dt_common.c.diff?r1=1.25&r2=1.26)
interval.c (r1.24 -> r1.25)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/pgtypeslib/interval.c.diff?r1=1.24&r2=1.25)
timestamp.c (r1.27 -> r1.28)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/pgtypeslib/timestamp.c.diff?r1=1.27&r2=1.28)
momjian@svr1.postgresql.org (Bruce Momjian) writes:
> Log Message:
> -----------
> Add time/date macros for code clarity:
> #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.
regards, tom lane
Tom Lane wrote: > momjian@svr1.postgresql.org (Bruce Momjian) writes: > > Log Message: > > ----------- > > Add time/date macros for code clarity: > > > #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. -- 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
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.
regards, tom lane
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. The problem is that 24 or 30 or 60 doesn't really say what it is, while the macros are self-documenting. The 30 is aparticular problem because I am like, hey, what does that 30 mean, then I have to think about it. What we can do is to rename them to AVG_* macros so it is clear it is approximate. -- 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
BTW, the initial returns (see kookaburra buildfarm log) say that you
broke things completely.
cc -O2 -qmaxmem=16384 -qsrcmsg -qlonglong -g -I../../../../src/include -I/usr/local/include -c -o timestamp.o
timestamp.c
1211 | span->time = (((((tm->tm_hour * ((int64) SECS_PER_MINUTELL)) +
.................................................a....................
a - 1506-045 (S) Undeclared identifier SECS_PER_MINUTELL.
1611 | span1 += interval1->month * ((int64) DAYS_PER_MONTHLL) * ((int64) 86400000000LL);
.............................................a..................................
a - 1506-045 (S) Undeclared identifier DAYS_PER_MONTHLL.
1612 | span1 += interval1->day * ((int64) HOURS_PER_DAYLL) * ((int64) 3600000000LL);
...........................................a....................................
a - 1506-045 (S) Undeclared identifier HOURS_PER_DAYLL.
2267 | result->time += (months - result->month) * ((int64) DAYS_PER_MONTHLL) * ((int64) 86400000000LL);
............................................................a...................
a - 1506-045 (S) Undeclared identifier DAYS_PER_MONTHLL.
2268 | result->time += (days - result->day) * ((int64) HOURS_PER_DAYLL) * ((int64) 3600000000LL);
........................................................a.......................
a - 1506-045 (S) Undeclared identifier HOURS_PER_DAYLL.
gmake[4]: *** [timestamp.o] Error 1
regards, tom lane
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? ---- Marc G. Fournier Hub.Org Networking Services (http://www.hub.org) Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> In short, I don't think this is an improvement.
> The problem is that 24 or 30 or 60 doesn't really say what it is, while
> the macros are self-documenting.
Except that they're NOT.
Anyone who is reading datetime code will be entirely familiar with the
Gregorian calendar (and if they aren't, the macro names you propose are
not going to help them). You cannot honestly sit there and say that
"365" or "24" isn't going to convey anything to anyone who could
usefully read the code in the first place.
> What we can do is to rename them to AVG_* macros so it is clear it is
> approximate.
But still not clear which approximation is being used. And in most
places where this might be used, that matters.
regards, tom lane
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. -- 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
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> In short, I don't think this is an improvement. > > > The problem is that 24 or 30 or 60 doesn't really say what it is, while > > the macros are self-documenting. > > Except that they're NOT. > > Anyone who is reading datetime code will be entirely familiar with the > Gregorian calendar (and if they aren't, the macro names you propose are > not going to help them). You cannot honestly sit there and say that > "365" or "24" isn't going to convey anything to anyone who could > usefully read the code in the first place. > > > What we can do is to rename them to AVG_* macros so it is clear it is > > approximate. > > But still not clear which approximation is being used. And in most > places where this might be used, that matters. Well, if you want to see the approximation, look at the macro value. At least with AVG we are documenting it is an approximation, and are doing it consistently. -- 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
Fixed. --------------------------------------------------------------------------- Tom Lane wrote: > BTW, the initial returns (see kookaburra buildfarm log) say that you > broke things completely. > > cc -O2 -qmaxmem=16384 -qsrcmsg -qlonglong -g -I../../../../src/include -I/usr/local/include -c -o timestamp.o timestamp.c > 1211 | span->time = (((((tm->tm_hour * ((int64) SECS_PER_MINUTELL)) + > .................................................a.................... > a - 1506-045 (S) Undeclared identifier SECS_PER_MINUTELL. > 1611 | span1 += interval1->month * ((int64) DAYS_PER_MONTHLL) * ((int64) 86400000000LL); > .............................................a.................................. > a - 1506-045 (S) Undeclared identifier DAYS_PER_MONTHLL. > 1612 | span1 += interval1->day * ((int64) HOURS_PER_DAYLL) * ((int64) 3600000000LL); > ...........................................a.................................... > a - 1506-045 (S) Undeclared identifier HOURS_PER_DAYLL. > 2267 | result->time += (months - result->month) * ((int64) DAYS_PER_MONTHLL) * ((int64) 86400000000LL); > ............................................................a................... > a - 1506-045 (S) Undeclared identifier DAYS_PER_MONTHLL. > 2268 | result->time += (days - result->day) * ((int64) HOURS_PER_DAYLL) * ((int64) 3600000000LL); > ........................................................a....................... > a - 1506-045 (S) Undeclared identifier HOURS_PER_DAYLL. > gmake[4]: *** [timestamp.o] Error 1 > > > regards, tom lane > -- 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
On Thu, 21 Jul 2005, Bruce Momjian wrote: > Tom Lane wrote: >> Bruce Momjian <pgman@candle.pha.pa.us> writes: >>> Tom Lane wrote: >>>> In short, I don't think this is an improvement. >> >>> The problem is that 24 or 30 or 60 doesn't really say what it is, while >>> the macros are self-documenting. >> >> Except that they're NOT. >> >> Anyone who is reading datetime code will be entirely familiar with the >> Gregorian calendar (and if they aren't, the macro names you propose are >> not going to help them). You cannot honestly sit there and say that >> "365" or "24" isn't going to convey anything to anyone who could >> usefully read the code in the first place. >> >>> What we can do is to rename them to AVG_* macros so it is clear it is >>> approximate. >> >> But still not clear which approximation is being used. And in most >> places where this might be used, that matters. > > Well, if you want to see the approximation, look at the macro value. At > least with AVG we are documenting it is an approximation, and are doing > it consistently. Make it APPROX_ vs AVG_ then ... ---- Marc G. Fournier Hub.Org Networking Services (http://www.hub.org) Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664