Re: pgsql: Further code review for pg_lsn data type.

On 2014-02-20 10:22:30 +0200, Heikki Linnakangas wrote:
> On 02/20/2014 09:47 AM, Andres Freund wrote:
> >On 2014-02-20 08:25:01 +0200, Heikki Linnakangas wrote:
> >>On 02/20/2014 02:56 AM, Andres Freund wrote:
> >>>Hm, won't
> >>>#define DatumGetLSN(X) ((XLogRecPtr) DatumGetInt64(X))
> >>>#define LSNGetDatum(X) (Int64GetDatum((int64) (X)))
> >>>possibly truncate the value if it's larger than 2^(63-1) as int is
> >>>signed but XLogRecPtr is unsigned?
> >>
> >>No. Casting between unsigned and signed integers of same width doesn't lose
> >>information. For example with 16-bit integers, casting unsigned 40000 to
> >>signed gives -25536. Casting signed -25536 back to unsigned gives back
> >>40000.
> >
> >Are you sure?
> >
> >6.3.1.3 Signed and unsigned integers, paragraph 3:
> >"Otherwise, the new type is signed and the value cannot be represented
> >in it; either the result is implementation-defined or an
> >implementation-defined signal is raised."
> >
> >Afaik unsigned to signed always safe, but not the other way round?

Bollocks, as visible in the quoted paragraph above, it's other way
round: signed to unsigned is well defined, unsigned to signed is
not. Don't type while waiting for coffee. Sorry for that.

> And in fact, the SET_X_BYTES macros also work by casting the value to an
> unsigned integer. So if signed -> unsigned is undefined, then the behavior
> of IntXGetDatum macros is also undefined.

Our whole usage of Datum is a pretty damned big mess of undefined
behaviour. One day I want to see how big the penalty of making Datum a
proper union with all the basetypes is. That's well defined.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services