Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements - Mailing list pgsql-hackers

Hi,

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