Re: Potential ABI breakage in upcoming minor releases

From: Noah Misch <noah(at)leadboat(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Aleksander Alekseev <aleksander(at)timescale(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Marco Slot <marco(dot)slot(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Christoph Berg <myon(at)debian(dot)org>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
Subject: Re: Potential ABI breakage in upcoming minor releases
Date: 2024-11-15 17:52:23
Message-ID: 20241115175223.68.nmisch@google.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 15, 2024 at 10:09:54AM -0500, Tom Lane wrote:
> Aleksander Alekseev <aleksander(at)timescale(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2024-11-15 18:01:45 Re: Skip collecting decoded changes of already-aborted transactions
Previous Message Tomas Vondra 2024-11-15 17:48:05 Re: logical replication: restart_lsn can go backwards (and more), seems broken since 9.4