Re: BUG #19636: heap_fill_tuple misses short varlena conversion after EOH_flatten_into, causing 3-byte waste per row

The following bug has been logged on the website:

Bug reference:      19636
Logged by:          anliuan
Email address:      17801022106@163.com
PostgreSQL version: 19beta1
Operating system:   centos
Description:       

When a PL/pgSQL function/procedure inserts an array value that arrives as an
expanded object (via EOH_flatten_into), the resulting tuple stores the
varlena with a 4-byte header instead of converting it to a
  1-byte short varlena header, wasting 3 bytes per row for arrays <= 127
bytes total size.

  Plain INSERT correctly uses the short varlena format; only the PL/pgSQL
expanded-object path is affected.

 Steps to reproduce:

  CREATE TABLE tt2(a text[]);
  INSERT INTO tt2 VALUES('{aaaaa}');  -- plain INSERT

  CREATE OR REPLACE FUNCTION call_insert_acl(racl text[]) RETURNS int AS $$
  BEGIN
      INSERT INTO tt2 VALUES(racl);
      RETURN 1;
  END;
  $$ LANGUAGE plpgsql;

  SELECT call_insert_acl('{aaaaa}');  -- PL/pgSQL INSERT

  SELECT a, pg_column_size(a) FROM tt2;

 Expected result: Both rows should have the same pg_column_size (33 bytes —
short varlena with 1-byte header).

Actual result:

  ┌─────────────────┬────────────────┬───────────────────────────────┐
  │       row       │ pg_column_size │         header format         │
  ├─────────────────┼────────────────┼───────────────────────────────┤
  │ plain INSERT    │ 33             │ short varlena (1-byte header) │
  ├─────────────────┼────────────────┼───────────────────────────────┤
  │ PL/pgSQL INSERT │ 36             │ 4-byte header                 │
  └─────────────────┴────────────────┴───────────────────────────────┘
  Root cause:

  In src/backend/access/common/heaptuple.c, heap_fill_tuple has four varlena
paths. The EOH_flatten_into path (for expanded objects) produces a
4-byte-header varlena and writes it directly, without checking
  VARATT_CAN_MAKE_SHORT:

  /* Current code — line ~229 */
  if (VARATT_IS_EXTERNAL_EXPANDED(val))
  {
      ExpandedObjectHeader *eoh = DatumGetEOHP(values[i]);
      data = (char *) ATT_ALIGN_NOMINAL(data, att[i]->attalign);
      data_length = EOH_get_flat_size(eoh);
      EOH_flatten_into(eoh, data, data_length);
      /* BUG: no VARATT_CAN_MAKE_SHORT check here */
  }

  The normal 4-byte-header path correctly performs this check:

  /* Normal path — line ~240 */
  else if (VARLENA_ATT_IS_PACKABLE(att[i]) && VARATT_CAN_MAKE_SHORT(val))
  {
      data_length = VARATT_CONVERTED_SHORT_SIZE(val);
      SET_VARSIZE_SHORT(data, data_length);
  }

  heap_compute_data_size has the same omission — it uses EOH_get_flat_size()
without considering the short-header size reduction, so the size calculation
and fill logic are consistently wrong (no memory
  overrun, just wasted space).

History
-------
Short headers date from 3e23b68dac0 (2007, "Support varlena fields with
single-byte headers and unaligned storage").  heap_form_tuple has packed
inline 4-byte-header varlenas via VARATT_CAN_MAKE_SHORT ever since the
modern heap_form_tuple path.

Expanded objects arrived in 1dc5ebc9077 (2015, "Support expanded
objects, particularly arrays, for better performance").  That commit
added an EXTERNAL_EXPANDED branch in heap_compute_data_size / fill_val
that flattens with EOH_flatten_into and writes the result as-is.
Flatteners are required to produce an inline 4-byte-header varlena
(see expandeddatum.h).  The new branch never applied the existing
short-header conversion afterward.  So this looks like an omission from
day one of expanded objects, not a later regression.

Proposal Fix
---
In those EXTERNAL_EXPANDED arms, if the attribute is packable and the
flat size from EOH_get_flat_size() would fit a short header, flatten
into a temporary palloc buffer and emit the short form into the tuple.
Otherwise keep the previous path (align and flatten straight into the
tuple).

VARATT_CAN_MAKE_SHORT cannot be used on the expanded toast pointer
itself.  It requires a 4B_U varlena.  Before flattening we only have
the flat size, so the patch uses a size-only helper matching that
macro's length rule.

The temporary buffer is needed because EOH_flatten_into expects a
maxaligned destination (same constraint as datumSerialize), while short
packing writes at an unaligned data pointer.  The temp is not pfree'd.
fill_val can run with CurrentMemoryContext as a BumpContext.  On
master / REL_19, RecursiveUnion UNION DISTINCT stores hashed tuples in
a BumpContext tuplescxt (c106ef08071, "Use BumpContext contexts in
TupleHashTables").  LookupTupleHashEntry switches to that context
before ExecCopySlotMinimalTupleExtra → heap_form_minimal_tuple.  Bump
does not support pfree.  A variant that pfree'd the temp failed the
"with" regress test on 19/master with:

ERROR:  pfree is not supported by the bump memory allocator

REL_18 still used AllocSet for that context, so the same pfree passed
there.  The short-packable flat size is at most about 130 bytes.  The
chunk is reclaimed when the context is reset.

Alternatives considered: Stack buffer was considered (fits the size bound), but palloc with EOH_flatten_into seemed better to me.