Andres Freund <and...@anarazel.de> writes:
> On 2025-07-17 20:09:57 -0400, Tom Lane wrote:
>> I made it just as a proof-of-concept that this can work. It compiled
>> cleanly and passed check-world for me on a 32-bit FreeBSD image.
> Interestingly it generates a *lot* of warnings here when building for 32 bit
> with gcc.
Oh, that's annoying. I tested it with
$ cc --version
FreeBSD clang version 13.0.0 (g...@github.com:llvm/llvm-project.git
llvmorg-13.0.0-0-gd7b669b3a303)
Target: i386-unknown-freebsd13.1
Thread model: posix
InstalledDir: /usr/bin
which is slightly back-rev but not that old. Which gcc did you use?
> One class of complaints is about DatumGetPointer() and
> PointerGetDatum() casting between different sizes:
> ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function
> 'DatumGetPointer':
> ../../../../../home/andres/src/postgresql/src/include/postgres.h:320:16:
> warning: cast to pointer from integer of different size
> [-Wint-to-pointer-cast]
> 320 | return (Pointer) X;
> | ^
> ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function
> 'PointerGetDatum':
> ../../../../../home/andres/src/postgresql/src/include/postgres.h:330:16:
> warning: cast from pointer to integer of different size
> [-Wpointer-to-int-cast]
> 330 | return (Datum) X;
> | ^
We might be able to silence those with intermediate casts to uintptr_t,
perhaps?
> And then there's a set of complains due to code converting from NULL to Datum
> without going through PointerGetDatum():
> 887 | return (Datum) NULL;
This code just strikes me as tin-eared. Our normal locution for
that is "(Datum) 0", and I see no reason to deviate.
> A third, not quite as verbose set is random code in .c files missing uses of
> DatumGetPointer(). There are lot of those.
Yeah, that's stuff we ought to fix anyway. Same with VAR* macros.
> One of these days I should again try the experiment of making Datum into a
> struct, to automatically catch omissions of datum <-> native type. Having them
> be silent most of the time really sucks.
Perhaps. I'd be a little sad if the "(Datum) 0" notation stops
working, because there are sure a lot of those.
>> I've not looked into the performance consequences. We probably
>> should at least try to measure that, though I'm not sure what
>> our threshold of pain would be for deciding not to do this.
> From my POV the threshold would have to be rather high for backend code. Less
> so in libpq, but that's not affected here.
I don't know if it's "rather high" or not, but that seems like
the gating factor that ought to be checked before putting in
more work.
regards, tom lane
