Re: Potential ABI breakage in upcoming minor releases

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Christoph Berg <myon@debian.org>, Noah Misch <noah@leadboat.com>, Pavan Deolasee <pavan.deolasee@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-11-14T20:33:32Z
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 2024-Nov-14, Tom Lane wrote:

> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > On 2024-Nov-14, Christoph Berg wrote:
> >> It didn't actually crash, true.
> 
> > But timescale did crash:
> 
> So what is timescale doing differently?

No idea.  Maybe a stacktrace from a core would tell us more; but the
jenkins task doesn't seem to have server logs or anything else to gives
us more clues.

There are three makeNode(ResultRelInfo), but they don't look anything
out of the ordinary:

C perhan: timescaledb 0$ git grep -C5 makeNode.ResultRelInfo
src/copy.c-  *
src/copy.c-  * WARNING. The dummy rangetable index is decremented by 1 (unchecked)
src/copy.c-  * inside `ExecConstraints` so unless you want to have a overflow, keep it
src/copy.c-  * above zero. See `rt_fetch` in parsetree.h.
src/copy.c-  */
src/copy.c: resultRelInfo = makeNode(ResultRelInfo);
src/copy.c-
src/copy.c-#if PG16_LT
src/copy.c- ExecInitRangeTable(estate, pstate->p_rtable);
src/copy.c-#else
src/copy.c- Assert(pstate->p_rteperminfos != NULL);
--
src/nodes/chunk_dispatch/chunk_insert_state.c-create_chunk_result_relation_info(const ChunkDispatch *dispatch, Relation rel)
src/nodes/chunk_dispatch/chunk_insert_state.c-{
src/nodes/chunk_dispatch/chunk_insert_state.c-  ResultRelInfo *rri;
src/nodes/chunk_dispatch/chunk_insert_state.c-  ResultRelInfo *rri_orig = dispatch->hypertable_result_rel_info;
src/nodes/chunk_dispatch/chunk_insert_state.c-  Index hyper_rti = rri_orig->ri_RangeTableIndex;
src/nodes/chunk_dispatch/chunk_insert_state.c:  rri = makeNode(ResultRelInfo);
src/nodes/chunk_dispatch/chunk_insert_state.c-
src/nodes/chunk_dispatch/chunk_insert_state.c-  InitResultRelInfo(rri, rel, hyper_rti, NULL, dispatch->estate->es_instrument);
src/nodes/chunk_dispatch/chunk_insert_state.c-
src/nodes/chunk_dispatch/chunk_insert_state.c-  /* Copy options from the main table's (hypertable's) result relation info */
src/nodes/chunk_dispatch/chunk_insert_state.c-  rri->ri_WithCheckOptions = rri_orig->ri_WithCheckOptions;
--
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);


I didn't catch any other allocations that would depend on the size of
ResultRelInfo in obvious ways.



Oh, hmm, there's this bit which uses struct assignment into a stack
element:

tsl/src/compression/compression.c-1559- if (decompressor->indexstate->ri_NumIndices > 0)
tsl/src/compression/compression.c-1560- {
tsl/src/compression/compression.c:1561:     ResultRelInfo indexstate_copy = *decompressor->indexstate;
tsl/src/compression/compression.c-1562-     Relation single_index_relation;

I find this rather suspect.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La espina, desde que nace, ya pincha" (Proverbio africano)