Thread
Commits
-
Fix inconsistency with HeapTuple freeing in extended_stats_funcs.c
- d5ea206728de 19 (unreleased) landed
-
[PATCH] Fix improper tuple deallocation in import_pg_statist()
yonghao_lee@qq.com <yonghao_lee@qq.com> — 2026-03-05T00:40:10Z
Hi hackers, I found a tuple deallocation issue in the import_pg_statistic() function in src/backend/statistics/extended_stats_funcs.c. The code uses pfree() to release a HeapTuple, which doesn't properly free the underlying tuple data. Bug Description: ================ In import_pg_statistic(), after heap_form_tuple() creates a HeapTuple and heap_copy_tuple_as_datum() copies it to a Datum, the code attempts to free the temporary HeapTuple using pfree(): pgstup = heap_form_tuple(RelationGetDescr(pgsd), values, nulls); pgstdat = heap_copy_tuple_as_datum(pgstup, RelationGetDescr(pgsd)); pfree(pgstup); /* <-- BUG: Improper tuple release */ The Problem: ============ HeapTuple is a pointer to HeapTupleData structure, which contains a nested pointer t_data pointing to the actual tuple header and data: typedef struct HeapTupleData { uint32 t_len; ItemPointerData t_self; Oid t_tableOid; HeapTupleHeader t_data; } HeapTupleData; Using pfree(pgstup) only frees the HeapTupleData structure itself , but leaves the underlying tuple data unfreed. The Fix: ======== Replace pfree(pgstup) with heap_freetuple(pgstup), which properly frees both the HeapTupleData structure and the underlying t_data: - pfree(pgstup); + heap_freetuple(pgstup); Patch: ====== --- src/backend/statistics/extended_stats_funcs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c index 0ec77a6..9279904 100644 --- a/src/backend/statistics/extended_stats_funcs.c +++ b/src/backend/statistics/extended_stats_funcs.c @@ -1509,7 +1509,7 @@ import_pg_statistic(Relation pgsd, JsonbContainer *cont, pgstup = heap_form_tuple(RelationGetDescr(pgsd), values, nulls); pgstdat = heap_copy_tuple_as_datum(pgstup, RelationGetDescr(pgsd)); - pfree(pgstup); + heap_freetuple(pgstup); *pg_statistic_ok = true; -- Regards, Yonghao Lee -
Re: [PATCH] Fix improper tuple deallocation in import_pg_statist()
Tomas Vondra <tomas@vondra.me> — 2026-03-05T01:01:32Z
On 3/5/26 01:40, yonghao_lee wrote: > Hi hackers, > > I found a tuple deallocation issue in the import_pg_statistic() function > in src/backend/statistics/extended_stats_funcs.c. The code uses pfree() > to release a > HeapTuple, which doesn't properly free the underlying tuple data. > > Bug Description: > ================ > > In import_pg_statistic(), after heap_form_tuple() creates a HeapTuple and > heap_copy_tuple_as_datum() copies it to a Datum, the code attempts to free > the temporary HeapTuple using pfree(): > > pgstup = heap_form_tuple(RelationGetDescr(pgsd), values, nulls); > pgstdat = heap_copy_tuple_as_datum(pgstup, RelationGetDescr(pgsd)); > pfree(pgstup); /* <-- BUG: Improper tuple release */ > > The Problem: > ============ > > HeapTuple is a pointer to HeapTupleData structure, which contains a nested > pointer t_data pointing to the actual tuple header and data: > > typedef struct HeapTupleData { > uint32 t_len; > ItemPointerData t_self; > Oid t_tableOid; > HeapTupleHeader t_data; > } HeapTupleData; > > Using pfree(pgstup) only frees the HeapTupleData structure itself > , but leaves the underlying tuple data unfreed. > > The Fix: > ======== > > Replace pfree(pgstup) with heap_freetuple(pgstup), which properly frees both > the HeapTupleData structure and the underlying t_data: > > - pfree(pgstup); > + heap_freetuple(pgstup); > Except that heap_freetuple is defined like this: void heap_freetuple(HeapTuple htup) { pfree(htup); } so this does not change anything. This report seems to miss how tuples are allocated in heap_form_tuple. -- Tomas Vondra -
Re: [PATCH] Fix improper tuple deallocation in import_pg_statist()
Michael Paquier <michael@paquier.xyz> — 2026-03-06T06:06:05Z
On Thu, Mar 05, 2026 at 02:01:32AM +0100, Tomas Vondra wrote: > Except that heap_freetuple is defined like this: > > void > heap_freetuple(HeapTuple htup) > { > pfree(htup); > } > > so this does not change anything. This report seems to miss how tuples > are allocated in heap_form_tuple. Right, what is obviously an LLM analysis forgot the context of how HeapTuple allocations happen when these are formed. Saying that, this is new code, that I had the idea to commit like this, and I don't like fresh inconsistencies. We also use heap_freetuple() in upsert_pg_statistic_ext_data(). Hence, at the end, done. It was harmless as-is, still.. -- Michael