Dead code with short varlenas in toast_save_datum()

From 22bcce5c8c7fd8cc866044cd6819e98db9bc91e3 Mon Sep 17 00:00:00 2001
From: Nikhil Kumar Veldanda <veldanda.nikhilkumar17@gmail.com>
Date: Sun, 10 Aug 2025 09:16:22 +0000
Subject: [PATCH v1] Add tests coverage for TOAST value reuse and OID
 allocation in toast_save_datum

Add isolation test for TOAST value reuse during table rewrite operations with
concurrent transactions, and regression tests for TOAST OID allocation
code paths in GetNewOidWithIndex(). These tests exercise edge cases in
toast_save_datum and TOAST chunk ID assignment during table rewrites.
---
 .../expected/cluster-toast-value-reuse.out    | 15 +++++
 src/test/isolation/isolation_schedule         |  1 +
 .../specs/cluster-toast-value-reuse.spec      | 58 +++++++++++++++++++
 src/test/regress/expected/strings.out         | 49 ++++++++++++++++
 src/test/regress/sql/strings.sql              | 27 +++++++++
 5 files changed, 150 insertions(+)
 create mode 100644 src/test/isolation/expected/cluster-toast-value-reuse.out
 create mode 100644 src/test/isolation/specs/cluster-toast-value-reuse.spec

diff --git a/src/test/isolation/expected/cluster-toast-value-reuse.out b/src/test/isolation/expected/cluster-toast-value-reuse.out
new file mode 100644
index 00000000000..fe1af5c5753
--- /dev/null
+++ b/src/test/isolation/expected/cluster-toast-value-reuse.out
@@ -0,0 +1,15 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_begin s1_update s2_cluster s1_commit
+step s1_begin: BEGIN;
+step s1_update: 
+  UPDATE cluster_toast_value_reuse
+     SET flag = (random()*1000)::int
+   WHERE id IN (
+     SELECT 1 + (random()*999)::int
+     FROM generate_series(1,30)
+   );
+
+step s2_cluster: CLUSTER cluster_toast_value_reuse; <waiting ...>
+step s1_commit: COMMIT;
+step s2_cluster: <... completed>
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index e3c669a29c7..67c67c83855 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -116,3 +116,4 @@ test: serializable-parallel-2
 test: serializable-parallel-3
 test: matview-write-skew
 test: lock-nowait
+test: cluster-toast-value-reuse
diff --git a/src/test/isolation/specs/cluster-toast-value-reuse.spec b/src/test/isolation/specs/cluster-toast-value-reuse.spec
new file mode 100644
index 00000000000..4549c05f690
--- /dev/null
+++ b/src/test/isolation/specs/cluster-toast-value-reuse.spec
@@ -0,0 +1,58 @@
+# Hold an UPDATE open, run CLUSTER in another session, then COMMIT. Which triggers data_todo = 0; code path in toast_save_datum
+
+# ---------- global setup ----------
+setup
+{
+  DROP TABLE IF EXISTS cluster_toast_value_reuse CASCADE;
+
+  CREATE TABLE cluster_toast_value_reuse
+  (
+    id    serial PRIMARY KEY,
+    flag  integer,
+    value text
+  );
+
+  -- Make sure 'value' is large enough to be TOASTed.
+  ALTER TABLE cluster_toast_value_reuse ALTER COLUMN value SET STORAGE EXTERNAL;
+
+  -- Define the clustering index.
+  CLUSTER "cluster_toast_value_reuse_pkey" ON cluster_toast_value_reuse;
+
+  -- Seed data: enough rows with big strings to force TOAST tuples.
+  INSERT INTO cluster_toast_value_reuse(flag, value)
+  SELECT 0,
+         repeat(md5(gs::text), 120) || repeat('x', 8000)  -- ~11KB
+  FROM generate_series(1, 1000) AS gs;
+
+  ANALYZE cluster_toast_value_reuse;
+
+  -- Establish clustered state so plain "CLUSTER cluster_toast_value_reuse;" uses the pkey.
+  CLUSTER cluster_toast_value_reuse;
+}
+
+teardown
+{
+  DROP TABLE IF EXISTS cluster_toast_value_reuse;
+}
+
+# ---------- sessions ----------
+# Session 1: starts a txn and updates some rows, then commits later.
+session s1
+step s1_begin  { BEGIN; }
+step s1_update {
+  UPDATE cluster_toast_value_reuse
+     SET flag = (random()*1000)::int
+   WHERE id IN (
+     SELECT 1 + (random()*999)::int
+     FROM generate_series(1,30)
+   );
+}
+step s1_commit { COMMIT; }
+
+# Session 2: runs CLUSTER while s1 holds locks.
+session s2
+step s2_cluster { CLUSTER cluster_toast_value_reuse; }
+
+# ---------- single interleaving ----------
+# Do the update in s1, then attempt CLUSTER in s2 (will wait), then commit s1.
+permutation s1_begin s1_update s2_cluster s1_commit
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index ba302da51e7..afc0cd6977e 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -2124,6 +2124,55 @@ SELECT pg_column_compression(f1) AS f1_comp, pg_column_compression(f2) AS f2_com
          | 
 (1 row)
 
+DROP TABLE toasttest;
+-- Test to trigger TOAST OID allocation code path in GetNewOidWithIndex()
+-- This exercises the toast_pointer.va_valueid assignment when creating new TOAST entries
+-- Create table with plain storage (forces inline storage initially)
+CREATE TABLE toasttest (f1 TEXT STORAGE plain);
+-- Insert large value to trigger TOAST storage
+INSERT INTO toasttest values (repeat('a', 7000));
+-- Switch to external storage to force TOAST table usage
+ALTER TABLE toasttest ALTER COLUMN f1 SET STORAGE EXTERNAL;
+-- Check initial TOAST chunk ID (should return NULL)
+SELECT pg_column_toast_chunk_id(f1) FROM toasttest;
+ pg_column_toast_chunk_id 
+--------------------------
+                         
+(1 row)
+
+SELECT substr(f1, 5, 10) AS f1_data FROM toasttest;
+  f1_data   
+------------
+ aaaaaaaaaa
+(1 row)
+
+SELECT pg_column_compression(f1) AS f1_comp FROM toasttest;
+ f1_comp 
+---------
+ 
+(1 row)
+
+-- VACUUM FULL forces TOAST data rewrite
+vacuum full toasttest;
+-- Verify new TOAST chunk ID was assigned via GetNewOidWithIndex()
+SELECT pg_column_toast_chunk_id(f1) IS NOT NULL FROM toasttest;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT substr(f1, 5, 10) AS f1_data FROM toasttest;
+  f1_data   
+------------
+ aaaaaaaaaa
+(1 row)
+
+SELECT pg_column_compression(f1) AS f1_comp FROM toasttest;
+ f1_comp 
+---------
+ 
+(1 row)
+
 DROP TABLE toasttest;
 --
 -- test length
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index b94004cc08c..ed0feaa0302 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -670,6 +670,33 @@ SELECT pg_column_compression(f1) AS f1_comp, pg_column_compression(f2) AS f2_com
   FROM toasttest;
 DROP TABLE toasttest;
 
+-- Test to trigger TOAST OID allocation code path in GetNewOidWithIndex()
+-- This exercises the toast_pointer.va_valueid assignment when creating new TOAST entries
+
+-- Create table with plain storage (forces inline storage initially)
+CREATE TABLE toasttest (f1 TEXT STORAGE plain);
+
+-- Insert large value to trigger TOAST storage
+INSERT INTO toasttest values (repeat('a', 7000));
+
+-- Switch to external storage to force TOAST table usage
+ALTER TABLE toasttest ALTER COLUMN f1 SET STORAGE EXTERNAL;
+
+-- Check initial TOAST chunk ID (should return NULL)
+SELECT pg_column_toast_chunk_id(f1) FROM toasttest;
+SELECT substr(f1, 5, 10) AS f1_data FROM toasttest;
+SELECT pg_column_compression(f1) AS f1_comp FROM toasttest;
+
+-- VACUUM FULL forces TOAST data rewrite
+vacuum full toasttest;
+
+-- Verify new TOAST chunk ID was assigned via GetNewOidWithIndex()
+SELECT pg_column_toast_chunk_id(f1) IS NOT NULL FROM toasttest;
+SELECT substr(f1, 5, 10) AS f1_data FROM toasttest;
+SELECT pg_column_compression(f1) AS f1_comp FROM toasttest;
+
+DROP TABLE toasttest;
+
 --
 -- test length
 --
-- 
2.47.3

From d434b39c83db5c3804aab4de657946324c083a2a Mon Sep 17 00:00:00 2001
From: Nikhil Kumar Veldanda <veldanda.nikhilkumar17@gmail.com>
Date: Sat, 16 Aug 2025 09:12:32 +0000
Subject: [PATCH v1] Add isolation test for TOAST value deduplication during
 CLUSTER

This test exercises the corner case in toast_save_datum() where CLUSTER operations encounter duplicate TOAST references and correctly reuse existing TOAST data instead of creating redundant copies.

During table rewrites like CLUSTER, both live and recently-dead versions of a row may reference the same TOAST value. When copying the second or later version of such a row, the system checks if the TOAST OID already exists in the new toast table using toastrel_valueid_exists(). If found, it sets data_todo = 0 to skip redundant data storage, ensuring only one copy of the TOAST value exists in the new table.

The test creates a scenario where:
- Session 1 updates rows while holding a transaction lock
- Session 2 attempts CLUSTER, which waits for the lock
- When CLUSTER proceeds, it encounters the duplicate TOAST references
- The test verifies TOAST chunk IDs are preserved via deduplication
---
 .../expected/cluster-toast-value-reuse.out    | 29 +++++++++
 src/test/isolation/isolation_schedule         |  1 +
 .../specs/cluster-toast-value-reuse.spec      | 64 +++++++++++++++++++
 3 files changed, 94 insertions(+)
 create mode 100644 src/test/isolation/expected/cluster-toast-value-reuse.out
 create mode 100644 src/test/isolation/specs/cluster-toast-value-reuse.spec

diff --git a/src/test/isolation/expected/cluster-toast-value-reuse.out b/src/test/isolation/expected/cluster-toast-value-reuse.out
new file mode 100644
index 00000000000..84cfc00c84e
--- /dev/null
+++ b/src/test/isolation/expected/cluster-toast-value-reuse.out
@@ -0,0 +1,29 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_begin s1_update s2_store_chunk_ids s2_cluster s1_commit s2_verify_chunk_ids
+step s1_begin: BEGIN;
+step s1_update: 
+  UPDATE cluster_toast_value_reuse
+     SET flag = 1 WHERE TRUE;
+
+step s2_store_chunk_ids: 
+  -- Store the primary keys and their associated chunk IDs before CLUSTER
+  CREATE TABLE chunk_id_comparison AS
+  SELECT c.id, pg_column_toast_chunk_id(c.value) as chunk_id
+  FROM cluster_toast_value_reuse c;
+
+step s2_cluster: CLUSTER cluster_toast_value_reuse; <waiting ...>
+step s1_commit: COMMIT;
+step s2_cluster: <... completed>
+step s2_verify_chunk_ids: 
+  -- Verify that chunk IDs are the same before and after CLUSTER (indicating reuse)
+  SELECT COUNT(*) = 0 AS chunk_ids_preserved
+  FROM chunk_id_comparison orig
+  JOIN cluster_toast_value_reuse c ON orig.id = c.id
+  WHERE orig.chunk_id != pg_column_toast_chunk_id(c.value);
+
+chunk_ids_preserved
+-------------------
+t                  
+(1 row)
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 4411d3c86dd..cb8a3bfbcbf 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -117,3 +117,4 @@ test: serializable-parallel-2
 test: serializable-parallel-3
 test: matview-write-skew
 test: lock-nowait
+test: cluster-toast-value-reuse
diff --git a/src/test/isolation/specs/cluster-toast-value-reuse.spec b/src/test/isolation/specs/cluster-toast-value-reuse.spec
new file mode 100644
index 00000000000..c123da9f720
--- /dev/null
+++ b/src/test/isolation/specs/cluster-toast-value-reuse.spec
@@ -0,0 +1,64 @@
+# Hold an UPDATE open, run CLUSTER in another session, then COMMIT. Which triggers data_todo = 0; code path in toast_save_datum
+
+# ---------- global setup ----------
+setup
+{
+  DROP TABLE IF EXISTS cluster_toast_value_reuse CASCADE;
+  DROP TABLE IF EXISTS chunk_id_comparison CASCADE;
+
+  CREATE TABLE cluster_toast_value_reuse
+  (
+    id    serial PRIMARY KEY,
+    flag  integer,
+    value text
+  );
+
+  -- Make sure 'value' is large enough to be TOASTed.
+  ALTER TABLE cluster_toast_value_reuse ALTER COLUMN value SET STORAGE EXTERNAL;
+
+  -- Define the clustering index.
+  CLUSTER "cluster_toast_value_reuse_pkey" ON cluster_toast_value_reuse;
+
+  -- Seed data: one row with big string to force TOAST tuple and trigger the todo=0 code path.
+  INSERT INTO cluster_toast_value_reuse(flag, value) 
+  VALUES (0, repeat(md5('1'), 120) || repeat('x', 8000));
+
+  CLUSTER cluster_toast_value_reuse;
+}
+
+teardown
+{
+  DROP TABLE IF EXISTS cluster_toast_value_reuse;
+  DROP TABLE IF EXISTS chunk_id_comparison;
+}
+
+# ---------- sessions ----------
+# Session 1: starts a txn and updates some rows, then commits later.
+session s1
+step s1_begin  { BEGIN; }
+step s1_update {
+  UPDATE cluster_toast_value_reuse
+     SET flag = 1 WHERE TRUE;
+}
+step s1_commit { COMMIT; }
+
+# Session 2: runs CLUSTER while s1 holds locks.
+session s2
+step s2_store_chunk_ids {
+  -- Store the primary keys and their associated chunk IDs before CLUSTER
+  CREATE TABLE chunk_id_comparison AS
+  SELECT c.id, pg_column_toast_chunk_id(c.value) as chunk_id
+  FROM cluster_toast_value_reuse c;
+}
+step s2_cluster { CLUSTER cluster_toast_value_reuse; }
+step s2_verify_chunk_ids {
+  -- Verify that chunk IDs are the same before and after CLUSTER (indicating reuse)
+  SELECT COUNT(*) = 0 AS chunk_ids_preserved
+  FROM chunk_id_comparison orig
+  JOIN cluster_toast_value_reuse c ON orig.id = c.id
+  WHERE orig.chunk_id != pg_column_toast_chunk_id(c.value);
+}
+
+# ---------- single interleaving ----------
+# Do the update in s1, store chunk IDs, then attempt CLUSTER in s2 (will wait), then commit s1, then verify chunk IDs.
+permutation s1_begin s1_update s2_store_chunk_ids s2_cluster s1_commit s2_verify_chunk_ids
-- 
2.47.3