Re: Potential ABI breakage in upcoming minor releases

Aleksander Alekseev <aleksander@timescale.com>

From: Aleksander Alekseev <aleksander@timescale.com>

To: pgsql-hackers <pgsql-hackers@postgresql.org>

Cc: Noah Misch <noah@leadboat.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tom Lane <tgl@sss.pgh.pa.us>, Christoph Berg <myon@debian.org>, Pavan Deolasee <pavan.deolasee@gmail.com>

Date: 2024-11-15T08:56:45Z

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 →

Undo unintentional ABI break in struct ResultRelInfo.
- ee33d58471de 16.6 landed
- 6bfacd368bb4 17.2 landed
- 17db248f318f 15.10 landed
- 099e711b77b8 14.15 landed
For inplace update durability, make heap_update() callers wait.
- 51ff46de29f6 16.5 cited
Fix btmarkpos/btrestrpos array key wraparound bug.
- 3fa81b62e09b 16.1 cited
Fix calculation of which GENERATED columns need to be updated.
- 8cd190e13a22 14.7 cited

Hi,

> > src/ts_catalog/catalog.c-extern TSDLLEXPORT ResultRelInfo *
> > src/ts_catalog/catalog.c-ts_catalog_open_indexes(Relation heapRel)
> > src/ts_catalog/catalog.c-{
> > src/ts_catalog/catalog.c-   ResultRelInfo *resultRelInfo;
> > src/ts_catalog/catalog.c-
> > src/ts_catalog/catalog.c:   resultRelInfo = makeNode(ResultRelInfo);
> > src/ts_catalog/catalog.c-   resultRelInfo->ri_RangeTableIndex = 0; /* dummy */
> > src/ts_catalog/catalog.c-   resultRelInfo->ri_RelationDesc = heapRel;
> > src/ts_catalog/catalog.c-   resultRelInfo->ri_TrigDesc = NULL; /* we don't fire triggers */
> > src/ts_catalog/catalog.c-
> > src/ts_catalog/catalog.c-   ExecOpenIndices(resultRelInfo, false);
>
> This is a copy of PostgreSQL's CatalogOpenIndexes().  Assuming timescaledb
> uses it like PostgreSQL uses CatalogOpenIndexes(), it's fine.  Specifically,
> CatalogOpenIndexes() is fine since PostgreSQL does not pass the ResultRelInfo
> to ModifyTable.

Yes. Initially I thought this was done for compatibility reasons with
different PG versions, but this doesn't seem to be the case since
CatalogOpenIndexes() was in the core for a long time.

> > Oh, hmm, there's this bit which uses struct assignment into a stack
> > element:
>
> Yes, stack allocation of a ResultRelInfo sounds relevant to the crash.  It
> qualifies as a "very unusual code pattern" in the postgr.es/c/e54a42a sense.

Yes, this is another issue.

I'm working with the TimescaleDB dev team to fix these issues on the
TimescaleDB side.

-- 
Best regards,
Aleksander Alekseev