Date: Sat, 21 Oct 2023 21:59:04 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>, suchithjn22@gmail.com,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: The documentation for storage type 'plain' actually allows
 single byte header
Message-ID: <ZTSB6JcPcpta3AGc@momjian.us>
References: 
 <167336599095.2667301.15497893107226841625@wrigleys.postgresql.org>
 <d99b3e0beebdde0741d28035aaea7f4af56fa050.camel@cybertec.at>
 <ZRdNjrI6lUQbD90Y@momjian.us>
 <4977c12e127250fe9777ae57f6f5884b8f5c84ae.camel@cybertec.at>
 <1421365.1696027552@sss.pgh.pa.us>
 <ZTMt1XxPmZekFXBL@momjian.us>
 <ZTSBPankJQ9_kfXF@momjian.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="u2LQugKEmWTJtBfn"
Content-Disposition: inline
In-Reply-To: <ZTSBPankJQ9_kfXF@momjian.us>
Archived-At: 
 <https://www.postgresql.org/message-id/ZTSB6JcPcpta3AGc%40momjian.us>
Precedence: bulk


--u2LQugKEmWTJtBfn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Oct 21, 2023 at 09:56:13PM -0400, Bruce Momjian wrote:
> I did some more research.  It turns out that the source slot/planSlot is
> populating its pg_attribute information via makeTargetEntry() and it
> has no concept of a storage type.
> 
> Digging further, I found that we cannot get rid of the the use of
> att->attstorage != TYPSTORAGE_PLAIN in macros ATT_IS_PACKABLE and
> VARLENA_ATT_IS_PACKABLE macros in src/backend/access/common/heaptuple.c
> because there are internal uses of fill_val() that can't handle packed
> varlena headers.
> 
> I ended up with a doc patch that adds a C comment about this odd
> behavior and removes doc text about PLAIN storage not using packed
> headers.

Oops, patch attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

--u2LQugKEmWTJtBfn
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="varlena_short.diff"

diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 148fb1b49d..3ea4e5526d 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -456,9 +456,7 @@ for storing <acronym>TOAST</acronym>-able columns on disk:
     <listitem>
      <para>
       <literal>PLAIN</literal> prevents either compression or
-      out-of-line storage; furthermore it disables use of single-byte headers
-      for varlena types.
-      This is the only possible strategy for
+      out-of-line storage.  This is the only possible strategy for
       columns of non-<acronym>TOAST</acronym>-able data types.
      </para>
     </listitem>
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index d6a4ddfd51..c52d40dce0 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -68,7 +68,16 @@
 #include "utils/memutils.h"
 
 
-/* Does att's datatype allow packing into the 1-byte-header varlena format? */
+/*
+ * Does att's datatype allow packing into the 1-byte-header varlena format?
+ * While functions that use TupleDescAttr() and assign attstorage =
+ * TYPSTORAGE_PLAIN cannot use packed varlena headers, functions that call
+ * TupleDescInitEntry() use typeForm->typstorage (TYPSTORAGE_EXTENDED) and
+ * can use packed varlena headers, e.g.:
+ *     CREATE TABLE test(a VARCHAR(10000) STORAGE PLAIN);
+ *     INSERT INTO test VALUES (repeat('A',10));
+ * This can be verified with pageinspect.
+ */
 #define ATT_IS_PACKABLE(att) \
 	((att)->attlen == -1 && (att)->attstorage != TYPSTORAGE_PLAIN)
 /* Use this if it's already known varlena */
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index d65e5625c7..24bad52923 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -2608,7 +2608,8 @@ range_contains_elem_internal(TypeCacheEntry *typcache, const RangeType *r, Datum
  * values into a range object.  They are modeled after heaptuple.c's
  * heap_compute_data_size() and heap_fill_tuple(), but we need not handle
  * null values here.  TYPE_IS_PACKABLE must test the same conditions as
- * heaptuple.c's ATT_IS_PACKABLE macro.
+ * heaptuple.c's ATT_IS_PACKABLE macro.  See the comments thare for more
+ * details.
  */
 
 /* Does datatype allow packing into the 1-byte-header varlena format? */

--u2LQugKEmWTJtBfn--