On 16/04/2026 17:37, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> On 16/04/2026 11:45, Peter Eisentraut wrote:
>>> What I'm missing here is, essentially where the previous thread stopped:
>>> What is the overall message that we want to communicate with the API?
>
> Good point.
>
>>> If the default assumption is that what pointers converted to Datums
>>> point to should not be modified on the other side (where the Datum is
>>> converted back to a pointer), then the current declaration of
>>> PointerGetDatum() is suitable, and the GIN code can be considered an
>>> exception and we make a special API for that. The previous thread
>>> proposed NonconstPointerGetDatum().
>
> I think there can be no doubt that most functions receiving a
> pass-by-ref Datum are not supposed to scribble on the pointed-to
> data. So it makes sense to me that PointerGetDatum should carry
> an implication of const-ness, and then we need to invent a new
> notation to use in the small number of places where that's not
> appropriate. I'd capitalize it as NonConstPointerGetDatum,
> but other than that nit that naming suggestion seems fine to me.
That makes sense. My worry is that we're changing the rules in a very
subtle way: It used to be OK to use PointerGetDatum(), pass the
resulting datum to something that modifies it. Now we say it's not OK,
and you must use NonConstPointerGetDatum(). You don't get any compiler
warnings if you use it wrong, except for this one coverity warning
apparently, but it doesn't catch this reliably either.
> Of course, then the *real* question is why DatumGetPointer
> doesn't deliver a const pointer. But I don't see how to get
> there without extremely invasive changes.
Good point.
>> We could have all three:
>
> Not excited about making massive changes for this.
Having all three would be a very localized change in postgres.h.
> I remain far less certain than Peter is that this discussion has
> anything to do with why Coverity is complaining about
> ginExtractEntries. I still think we should make some minimum-effort
> change to see if the complaint goes away before expending a lot of
> brain cells on choosing a final fix.
I think I'm going to commit my proposal to turn PointerGetDatum() back
into a macro, and see if that makes Coverity happy. Then we'll know, and
we can decide on the next steps. Any objections?
One open question is whether we should backpatch any of this. I guess
compilers don't misoptimize this in practice, or we would've gotten more
reports, but I really can't rationalize why not and a new compiler
version might well start hitting this.
- Heikki
