Re: Reduce build times of pg_trgm GIN indexes - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Reduce build times of pg_trgm GIN indexes
Date
Msg-id 3038C8A0-6C5F-45B2-ACB2-329DABB4A1E1@anarazel.de
Whole thread
In response to Re: Reduce build times of pg_trgm GIN indexes  (Peter Eisentraut <peter@eisentraut.org>)
List pgsql-hackers
Hi,

On April 16, 2026 4:45:55 AM EDT, Peter Eisentraut <peter@eisentraut.org> wrote:
>On 15.04.26 23:25, Tom Lane wrote:
>> Peter Eisentraut <peter@eisentraut.org> writes:
>>> On 15.04.26 13:06, Heikki Linnakangas wrote:
>>>> This was briefly discussed when PointerGetDatum() was changed from a
>>>> macro to a static inline function [1]. On that email, Peter pointed out
>>>> that the compiler was doing the same deduction that Coverity did now,
>>>> i.e. that if you pass the Datum returned by PointerGetDatum(&foo) to a
>>>> function, it cannot change *foo. I'm surprised we dismissed that worry
>>>> so quickly. If the compiler optimizes based on that assumption, you can
>>>> get incorrect code.
>>
>>> I don't think this is in evidence.  AFAICT, it's just Coverity that is
>>> complaining here, which is its right, but the code is not incorrect.
>>
>> Are you sure?  This seems like the sort of thing that will bite us on
>> the rear sometime in the future, as the compiler geeks put in more and
>> more aggressive optimizations.
>>
>> I think we should at least test the theory that changing
>> PointerGetDatum to remove the const cast would silence Coverity's
>> complaint.  If it does not then we're attributing too much
>> intelligence to Coverity.  But if it does, then we've correctly
>> identified why it's complaining, and we should take seriously the
>> idea that they aren't the only ones making this sort of deduction
>> (or won't be for long).
>
>I think it's quite clear to me that Coverity is complaining about this correctly, in its view of the world.  Compilers
sometimescomplain about this, too, although in this case they apparently don't look quite as deeply to do this
analysis.
>
>What I'm missing here is, essentially where the previous thread stopped: What is the overall message that we want to
communicatewith the API? 
>
>If the default assumption is that what pointers converted to Datums point to should not be modified on the other side
(wherethe Datum is converted back to a pointer), then the current declaration of PointerGetDatum() is suitable, and the
GINcode can be considered an exception and we make a special API for that.  The previous thread proposed
NonconstPointerGetDatum().

To me it seems way way way to dangerous to just redefine what PointerGetDatum() means for all existing callers, without
doingan exhaustive verification of all the callers. 

Separately from that, it doesn't seem defensible to take a const pointer and return a non const one. Why is that sane?

Greetings,

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: [doc] pg_ctl: fix wrong description for -l
Next
From: "Zhijie Hou (Fujitsu)"
Date:
Subject: RE: Parallel Apply