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

At Mon, 04 Nov 2019 12:53:48 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> 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))

# I didn't noticed the existing bit above.

Agreed. it is equivalent to the trick AFAICS thus no need to add
another one to warry with.

> (or s/double/float8/ ?)

Maybe.

> c.h is probably a reasonable place, seeing that we define the constants
> there.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center