Horton

Re: Potential ABI breakage in upcoming minor releases

Noah Misch <noah@leadboat.com>

View thread
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Aleksander Alekseev <aleksander@timescale.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Marco Slot <marco.slot@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Christoph Berg <myon@debian.org>, Pavan Deolasee <pavan.deolasee@gmail.com>
Date: 2024-11-15T17:52:23Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Undo unintentional ABI break in struct ResultRelInfo.

  2. For inplace update durability, make heap_update() callers wait.

  3. Fix btmarkpos/btrestrpos array key wraparound bug.

  4. Fix calculation of which GENERATED columns need to be updated.

On Fri, Nov 15, 2024 at 10:09:54AM -0500, Tom Lane wrote:
> Aleksander Alekseev <aleksander@timescale.com> writes:
> > Hi Macro,
> >> The problem here is that because TimescaleDB compiled against 17.0
> >> assumes a struct size of 376 (on my laptop) while PostgreSQL allocated
> >> the array with a struct size of 384, so the pointer math no longer
> >> holds and the whichrel value becomes nonsense. (1736263376 for
> >> whatever reason)
> 
> > Thanks for reporting. Yes, the code assumed fixed
> > sizeof(ResultRelInfo) within a given PG major release branch which
> > turned out not to be the case. We will investigate whether it can be
> > easily fixed on TimescaleDB side.
> 
> Yeah, the array-stride problem seems extremely hard to work around,
> because whichever size it is, you can't get code compiled with the
> other size to work.  I believe ResultRelInfo is the only node type
> we use arrays of, so this was a particularly unfortunate place
> to break ABI, but there it is.

I see ModifyTableState.resultRelInfo; are there other known ResultRelInfo
arrays?  That does add firebird_fdw to the list of PGXN modules requiring
rebuilds:

$ grep -rE 'resultRelInfo(\[|.* \+ )' | tee /tmp/3 | sed 's/-[^:]*/:/'|sort -u
firebird_fdw::                  resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex;
firebird_fdw::          resultRelInfo > mtstate->resultRelInfo + mtstate->mt_whichplan)

> I'm starting to lean to the opinion that we need a re-wrap.

Perhaps.  Even if we do rewrap for some reason, it's not a given that
restoring the old struct size is net beneficial.  If we restore the old struct
size in v16.6, those who rebuild for v16.5 would need to rebuild again.
Hearing about other ResultRelInfo arrays will help clarify that decision.

Either way, users of timescaledb should rebuild timescaledb for every future
PostgreSQL minor release.