On 16/04/2026 17:37, Tom Lane wrote:
Heikki Linnakangas <[email protected]> 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
