Michael Paquier <michael.paquier@gmail.com> writes:
> So, looking at 0001 now... Shouldn't there be a DatumGetBool8(), with
> the existing DatumGetBool() which should depend on the size of bool? I
> can see that all the catalogs are correctly updated with bool8 in the
> patch.
Surely bool and bool8 should have identical Datum representations,
so I'm not sure they need different DatumGet/GetDatum macros.
Although this opens up another point: just above those macros,
postgres.h says
* 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.
regards, tom lane
