Re: [HACKERS] taking stdbool.h into use

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Subject: Re: [HACKERS] taking stdbool.h into use
Date: 2018-01-11 22:01:55
Message-ID: 5d51721a-69ef-2053-9172-599b539f0628@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/9/18 00:17, Michael Paquier wrote:
>> * When a type narrower than Datum is stored in a Datum, we place it in the
>> * low-order bits and are careful that the DatumGetXXX macro for it discards
>> * the unused high-order bits (as opposed to, say, assuming they are zero).
>> * This is needed to support old-style user-defined functions, since depending
>> * on architecture and compiler, the return value of a function returning char
>> * or short may contain garbage when called as if it returned Datum.
>>
>> Since we flushed support for V0 functions, the stated rationale doesn't
>> apply anymore. I wonder whether there is anything to be gained by
>> changing DatumGetXXX and XXXGetDatum to be simple casts (as, if memory
>> serves, they once were until we noticed the stated hazard). Or, if
>> there's still a reason to keep the masking steps in place, we'd better
>> update this comment.

Yeah, we should remove those bit-masking calls if they are no longer
necessary.

Looking around where else they are used, the uses in numeric.c sure seem
like noops:

#if SIZEOF_DATUM == 8
#define NumericAbbrevGetDatum(X) ((Datum) SET_8_BYTES(X))
#define DatumGetNumericAbbrev(X) ((int64) GET_8_BYTES(X))
#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT64_MIN)
#else
#define NumericAbbrevGetDatum(X) ((Datum) SET_4_BYTES(X))
#define DatumGetNumericAbbrev(X) ((int32) GET_4_BYTES(X))
#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT32_MIN)
#endif

We can just replace these by straight casts, too.

That leaves the uses in rowtypes.c. Those were introduced as a
portability fix by commit 4cbb646334b. I'm curious why these are
necessary. The Datums they operate come from heap_deform_tuple(), which
gets them from fetchatt(), which does run all pass-by-value values
through the very same GET_X_BYTES() macros (until now). I don't see
where those dirty upper bits would be coming from.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2018-01-11 22:18:30 Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
Previous Message Robert Haas 2018-01-11 21:44:52 Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)