Yuya Watari <watari.y...@gmail.com> writes: > I attached the modified patch. In the patch, I placed the macro in > "src/include/c.h", but this may not be a good choice because c.h is > widely included from a lot of files. Do you have any good ideas about > its placement?
I agree that there's an actual bug here; it can be demonstrated with # select extract(epoch from '256 microseconds'::interval * (2^55)::float8); date_part -------------------- -9223372036854.775 (1 row) which clearly is a wrong answer. I do not however like any of the proposed patches. We already have one place that deals with this problem correctly, in int8.c's dtoi8(): /* * Range check. We must be careful here that the boundary values are * expressed exactly in the float domain. We expect PG_INT64_MIN to be an * exact power of 2, so it will be represented exactly; but PG_INT64_MAX * isn't, and might get rounded off, so avoid using it. */ if (unlikely(num < (float8) PG_INT64_MIN || num >= -((float8) PG_INT64_MIN) || isnan(num))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); We should adopt that coding technique not invent new ones. I do concur with creating a macro that encapsulates a correct version of this test, maybe like #define DOUBLE_FITS_IN_INT64(num) \ ((num) >= (double) PG_INT64_MIN && \ (num) < -((double) PG_INT64_MIN)) (or s/double/float8/ ?) c.h is probably a reasonable place, seeing that we define the constants there. regards, tom lane