> On Mar 14, 2016, at 6:23 AM, David Steele <da...@pgmasters.net> wrote: > > On 2/25/16 4:44 PM, Vitaly Burovoy wrote: > >> Added to the commitfest 2016-03. >> >> [CF] https://commitfest.postgresql.org/9/540/ > > This looks like a fairly straight-forward bug fix (the size of the patch is > deceptive because there a lot of new tests included). It applies cleanly. > > Anastasia, I see you have signed up to review. Do you have an idea when you > will get the chance to do that?
The first thing I notice about this patch is that src/include/datatype/timestamp.h has some #defines that are brittle. The #defines have comments explaining their logic, but I'd rather embed that in the #define directly: This: +#ifdef HAVE_INT64_TIMESTAMP +#define MIN_TIMESTAMP INT64CONST(-211813488000000000) +/* == (0 - POSTGRES_EPOCH_JDATE) * 86400 * USECS_PER_SEC */ +#define MAX_TIMESTAMP INT64CONST(9223371331200000000) +/* == (JULIAN_MAX4STAMPS - POSTGRES_EPOCH_JDATE) * 86400 * USECS_PER_SEC */ +#else +#define MIN_TIMESTAMP -211813488000.0 +/* == (0 - POSTGRES_EPOCH_JDATE) * 86400 */ +#define MAX_TIMESTAMP 9223371331200.0 +/* == (JULIAN_MAX4STAMPS - POSTGRES_EPOCH_JDATE) * 86400 */ +#endif Could be written as: #ifdef HAVE_INT64_TIMESTAMP #define MIN_TIMESTAMP ((INT64CONST(0) - POSTGRES_EPOCH_JDATE) * USECS_PER_DAY) #define MAX_TIMESTAMP ((JULIAN_MAX4STAMPS - POSTGRES_EPOCH_JDATE) * USECS_PER_DAY) #else #define MIN_TIMESTAMP ((INT64CONST(0) - POSTGRES_EPOCH_JDATE) * SECS_PER_DAY) #define MAX_TIMESTAMP ((JULIAN_MAX4STAMPS - POSTGRES_EPOCH_JDATE) * SECS_PER_DAY) #endif I assume modern compilers would convert these to the same constants at compile-time, rather than impose a runtime penalty. The #defines would be less brittle in the event, for example, that the postgres epoch were ever changed. Mark Dilger -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
