Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

Yuya Watari <watari.yuya@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