Re: Reduce build times of pg_trgm GIN indexes

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, David Geier <geidav(dot)pg(at)gmail(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Reduce build times of pg_trgm GIN indexes
Date: 2026-04-17 19:21:41
Message-ID: b25bd9ff-3d18-4d23-a480-17434096fab1@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 16/04/2026 20:47, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
>> On 16/04/2026 17:37, Tom Lane wrote:
>>> Not excited about making massive changes for this.
>
>> Having all three would be a very localized change in postgres.h.
>
> Sure, but *using* them in a consistent way would be invasive.
>
>>> 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?
>
> WFM.

Grepping for PointerGetDatum(), there are a bunch of wrappers of it for
specific types, like:

static inline Datum CStringGetDatum(const char *X)
static inline Datum NumericGetDatum(Numeric X)

Most are marked "const". These all potentially have the same problem,
but I think for these it is a good assumption that resulting Datum will
not be used to modify *X, so we can leave them alone. I guess we didn't
do that for NumericGetDatum just because the Numeric typedef doesn't
allow that.

There's also:

static inline Datum fetch_att(const void *T, bool attbyval, int attlen)

The "const" seems reasonable on that too.

This is an interesting case:

static inline Datum
EOHPGetRWDatum(const struct ExpandedObjectHeader *eohptr)
{
return PointerGetDatum(eohptr->eoh_rw_ptr);
}

That RW stands for read/write, which sounds alarming. But the returned
datum points to eohptr->eoh_rw_ptr rather than *eohptr itself, so I
think the 'const' is correct here after all.

So, pushed a commit that changes just PointerGetDatum() itself, leaving
all those others alone.

- Heikki

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2026-04-17 19:30:37 small cleanup patches for collation code
Previous Message Robert Haas 2026-04-17 19:00:05 Re: pg_plan_advice