Re: Dead code with short varlenas in toast_save_datum()

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