Re: [HACKERS] taking stdbool.h into use
Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Michael Paquier <michael.paquier@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
pgsql-hackers <pgsql-hackers@postgresql.org>,
Thomas Munro <thomas.munro@enterprisedb.com>
Date: 2018-01-11T22:01:55Z
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
Commits
-
Use stdbool.h if suitable
- 9a95a77d9d5d 11.0 landed
-
Add configure tests for stdbool.h and sizeof bool
- f20b3285340c 11.0 landed
-
Remove useless use of bit-masking macros
- d91da5ecedc8 11.0 landed
-
Change various Gin*Is* macros to return 0/1.
- af4472bcb88a 9.6.0 cited
-
Fix several possibly non-portable gaffs in record_image_ops.
- 4cbb646334b3 9.4.0 cited