Feedback-ID: ia2694551:Fastmail
Date: Thu, 14 Nov 2024 21:33:32 +0100
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>
Subject: Re: Potential ABI breakage in upcoming minor releases
Message-ID: <202411142033.u6za5a6ylj2k@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1976928.1731614974@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/202411142033.u6za5a6ylj2k%40alvherre.pgsql>
Precedence: bulk

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)