Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements

Hi,

Today I encountered a segmentation fault caused by the patch
v20-0007-Add-Datum-storage-support-to-tuplestore.patch. During the merge
phase, I inserted some tuples into the table so that STIR would have data
for the validation phase. The segfault occurred during a call to
tuplestore_end().

The root cause is that a few functions in the tuplestore code still assume
that all stored data is a pointer and thus attempt to pfree it. This
assumption breaks when datumByVal is used, as the data is stored directly
and not as a pointer. In particular, tuplestore_end(), tuplestore_trim(),
and tuplestore_clear() incorrectly try to free such values.

When addressing this, please also ensure that context memory accounting is
handled properly: we should not increment or decrement the remaining
context memory size when cleaning or trimming datumByVal entries, since no
actual memory was allocated for them.

Interestingly, I’m surprised you haven’t hit this segfault yourself. Are
you perhaps testing on an older system where INT8OID is passed by
reference? Or is your STIR always empty during the validation phase?

One more point: I noticed you modified the index_create() function
signature. You added the relpersistence parameter, which seems
unnecessary—this can be determined internally by checking if it’s an
auxiliary index, in which case the index should be marked as unlogged. You
also added an auxiliaryIndexOfOid argument (do not remember exact naming,
but was used for dependency). It might be cleaner to pass this via the
IndexInfo structure instead. index_create() already has dozens of mouthful
arguments, and external extensions (like pg_squeeze) still rely on the old
signature, so minimizing changes to the function interface would improve
compatibility.

Best regards,
Sergey

On Wed, Jun 18, 2025, 1:50 PM Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>
wrote:

> Hello, Sergey!
>
> > In patch
> v20-0006-Add-STIR-access-method-and-flags-related-to-auxi.patch, within the
> "StirMarkAsSkipInserts" function, a critical section appears to be left
> unclosed. This resulted in an assertion failure during ANALYZE of a table
> containing a leftover STIR index.
> Thanks, good catch. I'll add it to batch fix with the other things.
>
> Best regards,
> Mikhail.
>