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
signature.asc
Description: PGP signature
