Dead code with short varlenas in toast_save_datum()

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Nikhil Kumar Veldanda <veldanda(dot)nikhilkumar17(at)gmail(dot)com>
Subject: Dead code with short varlenas in toast_save_datum()
Date: 2025-08-04 03:15:59
Message-ID: aJAl7-NvIk0kZByz@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

Nikhil (in CC.), has noticed while looking at the 8B-value TOAST patch
that this code block used for short varlenas in toast_save_datum() is
dead based on the current coverage:
if (VARATT_IS_SHORT(dval))
{
data_p = VARDATA_SHORT(dval);
data_todo = VARSIZE_SHORT(dval) - VARHDRSZ_SHORT;
toast_pointer.va_rawsize = data_todo + VARHDRSZ; /* as if not short */
toast_pointer.va_extinfo = data_todo;
}

Coverage link:
https://coverage.postgresql.org/src/backend/access/common/toast_internals.c.gcov.html

toast_save_datum() is taken only through toast_tuple_externalize(),
and I was assuming first that an EXTERNAL attribute with a minimal
toast_tuple_target of 128B would have been enough to trigger this,
still even a minimal bound is not enough to trigger
heap_toast_insert_or_update(), which heap_prepare_insert() would call
if:
- the hardcoded check on TOAST_TUPLE_THRESHOLD is reached, which of
course would not happen.
- tuple is marked with HEAP_HASEXTERNAL, something that happens only
if fill_val() finds an external varlena (aka VARATT_IS_EXTERNAL),
and that does not happen for SHORT varlenas.

The insertion of short varlenas in TOAST is old enough to vote and it
assumed as supported when reading external on-disk TOAST datums, still
it's amazing to see that there no in-core coverage for it.

Anyway, There is an action item here. I don't see a SQL pattern that
would trigger this code path on HEAD (perhaps I lack the imagination
to do so), so the only alternative I can think of is the introduction
of a C function in regress.c to force our way through, calling
directly toast_save_datum() or something equivalent, so as
toast_save_datum() is tested with more varlena patterns. That would
be something similar to what we do for indirect TOAST tuples.

Another possibility is to assume that this code is dead. But I doubt
that it is possible to claim that as the TOAST code is assumed as
being usable by out-of-core code, so depending on how fancy things are
being done a TOAST relation could use a short varatt for an insert.

Thoughts?
--
Michael

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2025-08-04 03:30:32 Re: Conflict detection for update_deleted in logical replication
Previous Message Chao Li 2025-08-04 03:10:53 Re: A little cosmetic to convert_VALUES_to_ANY()