Re: Making type Datum be 8 bytes everywhere

Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
> These patches look mechanically correct to me.

Thanks for reviewing!

> The various additional uses of *GetDatum and DatumGet* could be applied
> as a separate patch. That would make the remaining patch clearer as
> mostly just removing dead code.

I looked at doing this, but couldn't convince myself that it would be
anything but make-work. It's not like 0002 and 0003 could be pure
deletions in any case -- at least there would be comment updates to
make.

> In tupmacs.h, I think the two sites with
> case sizeof(Datum):
> should be rewritten using
> case sizeof(int64):
> to match the other cases. Otherwise, this code ends up looking
> mysteriously asymmetric. (And the related code in pg_type.c as well.)

Fair point, and I also realized that these were another place to
insert Datum conversions :-(. The existing code would fail badly
if we redefined Datum as a struct with an extra field in it.

> The remaining mentions of "SIZEOF_DATUM" in gistproc.c and network.c
> could be replaced by "sizeof(Datum)". Then we could eventually remove
> SIZEOF_DATUM altogether.

Agreed with s/SIZEOF_DATUM/sizeof(Datum)/g, so I've done that. I do
not think removing the #define altogether will be sensible, however.
It will accomplish little except to break extensions that we didn't
need to break. It'd be particularly painful for extensions that
want to compile the same source code against multiple PG versions.
However, it does make sense to put in a comment explaining that
SIZEOF_DATUM is now vestigial, so I've done that.

> (Maybe DatumBigEndianToNative() would better
> live in a different header file at that point, not sure.)

Not sure either. I did realize that #ifdef'ing it with SIZEOF_DATUM
was pretty pointless, so I undid that aspect.

> I would also remove most of the remaining uses of FLOAT8PASSBYVAL,
> especially where it is used in relation with INT8OID, which is just
> endlessly confusing.

Fair enough, also done below.

> We should also get rid of these things in the control file and the ABI
> magic structs, but that could be done as separate patches. It would
> probably require a separate round of thinking about compatibility,
> upgrading, etc.

Meh. The savings is not enough to justify expending any brain cells
working through the implications. There would be implications, too,
for example with programs that look at the output of pg_controldata.
Again, just labeling those fields as vestigial seems like enough.

PFA v2 with these changes. I feel like this is pretty close to
committable now.

regards, tom lane