0001-Truncate-trailing-null-attributes-from-heap-rows-to-.patch

From 319a81beafad444cf352040942123f0aed1dc392 Mon Sep 17 00:00:00 2001
From: Jamie Martin <jameison.martin@yahoo.com>
Date: Wed, 11 Apr 2012 18:12:42 -0700
Subject: [PATCH] Truncate trailing null attributes from heap rows to reduce the size of the row bitmap.

---
 src/backend/access/common/heaptuple.c        |   64 ++++++---
 src/backend/access/common/indextuple.c       |   10 +-
 src/backend/access/heap/tuptoaster.c         |   20 ++-
 src/include/access/htup.h                    |    2 +-
 src/test/regress/expected/trailing_nulls.out |  210 ++++++++++++++++++++++++++
 src/test/regress/parallel_schedule           |    2 +-
 src/test/regress/sql/trailing_nulls.sql      |  166 ++++++++++++++++++++
 7 files changed, 445 insertions(+), 29 deletions(-)
 create mode 100644 src/test/regress/expected/trailing_nulls.out
 create mode 100644 src/test/regress/sql/trailing_nulls.sql

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 034dfe5..8975a12 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -133,22 +133,24 @@ void
 heap_fill_tuple(TupleDesc tupleDesc,
 				Datum *values, bool *isnull,
 				char *data, Size data_size,
-				uint16 *infomask, bits8 *bit)
+				uint16 *infomask, bits8 *bit,
+				int numberOfAttributes)
 {
 	bits8	   *bitP;
 	int			bitmask;
 	int			i;
-	int			numberOfAttributes = tupleDesc->natts;
 	Form_pg_attribute *att = tupleDesc->attrs;
 
 #ifdef USE_ASSERT_CHECKING
 	char	   *start = data;
 #endif
 
+	*infomask &= ~(HEAP_HASNULL | HEAP_HASVARWIDTH | HEAP_HASEXTERNAL);
 	if (bit != NULL)
 	{
 		bitP = &bit[-1];
 		bitmask = HIGHBIT;
+		*infomask |= HEAP_HASNULL;
 	}
 	else
 	{
@@ -157,7 +159,6 @@ heap_fill_tuple(TupleDesc tupleDesc,
 		bitmask = 0;
 	}
 
-	*infomask &= ~(HEAP_HASNULL | HEAP_HASVARWIDTH | HEAP_HASEXTERNAL);
 
 	for (i = 0; i < numberOfAttributes; i++)
 	{
@@ -176,7 +177,6 @@ heap_fill_tuple(TupleDesc tupleDesc,
 
 			if (isnull[i])
 			{
-				*infomask |= HEAP_HASNULL;
 				continue;
 			}
 
@@ -636,14 +636,20 @@ heap_form_tuple(TupleDesc tupleDescriptor,
 	int			hoff;
 	bool		hasnull = false;
 	Form_pg_attribute *att = tupleDescriptor->attrs;
-	int			numberOfAttributes = tupleDescriptor->natts;
+	int			maxNumberOfAttributes = tupleDescriptor->natts;
 	int			i;
+	int 		lastNonNullAttribute = -1;
+	int 		numberOfAttributes; /* The actual number of attributes for this particular row.
+	 	 	 	 	 	 	 	 	 * trailing nulls aren't stored so this may be smaller than
+	 	 	 	 	 	 	 	 	 * maxNumberOfAttributes
+	 	 	 	 	 	 	 	 	 */
+
 
-	if (numberOfAttributes > MaxTupleAttributeNumber)
+	if (maxNumberOfAttributes > MaxTupleAttributeNumber)
 		ereport(ERROR,
 				(errcode(ERRCODE_TOO_MANY_COLUMNS),
 				 errmsg("number of columns (%d) exceeds limit (%d)",
-						numberOfAttributes, MaxTupleAttributeNumber)));
+						maxNumberOfAttributes, MaxTupleAttributeNumber)));
 
 	/*
 	 * Check for nulls and embedded tuples; expand any toasted attributes in
@@ -656,20 +662,26 @@ heap_form_tuple(TupleDesc tupleDescriptor,
 	 * if an attribute is already toasted, it must have been sent to disk
 	 * already and so cannot contain toasted attributes.
 	 */
-	for (i = 0; i < numberOfAttributes; i++)
+	for (i = 0; i < maxNumberOfAttributes; i++)
 	{
 		if (isnull[i])
 			hasnull = true;
-		else if (att[i]->attlen == -1 &&
+		else
+		{
+			lastNonNullAttribute = i;
+
+			if (att[i]->attlen == -1 &&
 				 att[i]->attalign == 'd' &&
 				 att[i]->attndims == 0 &&
 				 !VARATT_IS_EXTENDED(DatumGetPointer(values[i])))
-		{
-			values[i] = toast_flatten_tuple_attribute(values[i],
+			{
+				values[i] = toast_flatten_tuple_attribute(values[i],
 													  att[i]->atttypid,
 													  att[i]->atttypmod);
+			}
 		}
 	}
+	numberOfAttributes = lastNonNullAttribute + 1;
 
 	/*
 	 * Determine total space needed
@@ -719,7 +731,8 @@ heap_form_tuple(TupleDesc tupleDescriptor,
 					(char *) td + hoff,
 					data_len,
 					&td->t_infomask,
-					(hasnull ? td->t_bits : NULL));
+					(hasnull ? td->t_bits : NULL),
+					numberOfAttributes);
 
 	return tuple;
 }
@@ -1390,14 +1403,19 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
 	int			hoff;
 	bool		hasnull = false;
 	Form_pg_attribute *att = tupleDescriptor->attrs;
-	int			numberOfAttributes = tupleDescriptor->natts;
+	int			maxNumberOfAttributes = tupleDescriptor->natts;
 	int			i;
+	int 		lastNonNullAttribute = -1;
+	int 		numberOfAttributes; /* The actual number of attributes for this particular row.
+	 	 	 	 	 	 	 	 	 * trailing nulls aren't stored so this may be smaller than
+	 	 	 	 	 	 	 	 	 * maxNumberOfAttributes
+	 	 	 	 	 	 	 	 	 */
 
-	if (numberOfAttributes > MaxTupleAttributeNumber)
+	if (maxNumberOfAttributes > MaxTupleAttributeNumber)
 		ereport(ERROR,
 				(errcode(ERRCODE_TOO_MANY_COLUMNS),
 				 errmsg("number of columns (%d) exceeds limit (%d)",
-						numberOfAttributes, MaxTupleAttributeNumber)));
+						maxNumberOfAttributes, MaxTupleAttributeNumber)));
 
 	/*
 	 * Check for nulls and embedded tuples; expand any toasted attributes in
@@ -1410,20 +1428,25 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
 	 * if an attribute is already toasted, it must have been sent to disk
 	 * already and so cannot contain toasted attributes.
 	 */
-	for (i = 0; i < numberOfAttributes; i++)
+	for (i = 0; i < maxNumberOfAttributes; i++)
 	{
 		if (isnull[i])
 			hasnull = true;
-		else if (att[i]->attlen == -1 &&
+		else
+		{
+			lastNonNullAttribute = i;
+			if (att[i]->attlen == -1 &&
 				 att[i]->attalign == 'd' &&
 				 att[i]->attndims == 0 &&
 				 !VARATT_IS_EXTENDED(values[i]))
-		{
-			values[i] = toast_flatten_tuple_attribute(values[i],
+			{
+				values[i] = toast_flatten_tuple_attribute(values[i],
 													  att[i]->atttypid,
 													  att[i]->atttypmod);
+			}
 		}
 	}
+	numberOfAttributes = lastNonNullAttribute + 1;
 
 	/*
 	 * Determine total space needed
@@ -1463,7 +1486,8 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
 					(char *) tuple + hoff,
 					data_len,
 					&tuple->t_infomask,
-					(hasnull ? tuple->t_bits : NULL));
+					(hasnull ? tuple->t_bits : NULL),
+					numberOfAttributes);
 
 	return tuple;
 }
diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 76c76e9..a924863 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -29,6 +29,13 @@
 /* ----------------
  *		index_form_tuple
  * ----------------
+ *
+ * Note that as opposed to heap rows we don't bother truncating off trailing
+ * null attributes. This is because the maximum number of attributes in an
+ * index (32) is well bounded and there isn't much to be gained by trying to
+ * reduce the bitmap that tracks null attributes since we don't expect index
+ * keys have a lot of trailing nulls. Also, the logic for tracking the null
+ * bitmap is hard-wired at 4 bytes (see IndexAttributeBitMapData).
  */
 IndexTuple
 index_form_tuple(TupleDesc tupleDescriptor,
@@ -139,7 +146,8 @@ index_form_tuple(TupleDesc tupleDescriptor,
 					(char *) tp + hoff,
 					data_size,
 					&tupmask,
-					(hasnull ? (bits8 *) tp + sizeof(IndexTupleData) : NULL));
+					(hasnull ? (bits8 *) tp + sizeof(IndexTupleData) : NULL),
+					numberOfAttributes);
 
 #ifdef TOAST_INDEX_HACK
 	for (i = 0; i < numberOfAttributes; i++)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 28b5a20..6935a3c 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -425,6 +425,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 	bool		need_free = false;
 	bool		need_delold = false;
 	bool		has_nulls = false;
+	int			lastNonNullValOffset = -1;
 
 	Size		maxDataLen;
 	Size		hoff;
@@ -534,6 +535,10 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 			has_nulls = true;
 			continue;
 		}
+		else
+		{
+			lastNonNullValOffset = i;
+		}
 
 		/*
 		 * Now look at varlena attributes
@@ -593,8 +598,8 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 
 	/* compute header overhead --- this should match heap_form_tuple() */
 	hoff = offsetof(HeapTupleHeaderData, t_bits);
-	if (has_nulls)
-		hoff += BITMAPLEN(numAttrs);
+	if (has_nulls && lastNonNullValOffset > -1)
+		hoff += BITMAPLEN(lastNonNullValOffset+1);
 	if (newtup->t_data->t_infomask & HEAP_HASOID)
 		hoff += sizeof(Oid);
 	hoff = MAXALIGN(hoff);
@@ -867,6 +872,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 		int32		new_header_len;
 		int32		new_data_len;
 		int32		new_tuple_len;
+		numAttrs = lastNonNullValOffset + 1; /* only store up to the last non-null attribute */
 
 		/*
 		 * Calculate the new size of the tuple.
@@ -879,7 +885,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 		 * whether there needs to be one at all.
 		 */
 		new_header_len = offsetof(HeapTupleHeaderData, t_bits);
-		if (has_nulls)
+		if (has_nulls && numAttrs > 0)
 			new_header_len += BITMAPLEN(numAttrs);
 		if (olddata->t_infomask & HEAP_HASOID)
 			new_header_len += sizeof(Oid);
@@ -914,7 +920,8 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 						(char *) new_data + new_header_len,
 						new_data_len,
 						&(new_data->t_infomask),
-						has_nulls ? new_data->t_bits : NULL);
+						has_nulls ? new_data->t_bits : NULL,
+						numAttrs);
 	}
 	else
 		result_tuple = newtup;
@@ -1060,12 +1067,12 @@ toast_flatten_tuple_attribute(Datum value,
 		return value;			/* not a composite type */
 
 	att = tupleDesc->attrs;
-	numAttrs = tupleDesc->natts;
 
 	/*
 	 * Break down the tuple into fields.
 	 */
 	olddata = DatumGetHeapTupleHeader(value);
+	numAttrs = HeapTupleHeaderGetNatts(olddata);
 	Assert(typeId == HeapTupleHeaderGetTypeId(olddata));
 	Assert(typeMod == HeapTupleHeaderGetTypMod(olddata));
 	/* Build a temporary HeapTuple control structure */
@@ -1147,7 +1154,8 @@ toast_flatten_tuple_attribute(Datum value,
 					(char *) new_data + new_header_len,
 					new_data_len,
 					&(new_data->t_infomask),
-					has_nulls ? new_data->t_bits : NULL);
+					has_nulls ? new_data->t_bits : NULL,
+			        numAttrs);
 
 	/*
 	 * Free allocated temp values
diff --git a/src/include/access/htup.h b/src/include/access/htup.h
index 6a3778d..565b32b 100644
--- a/src/include/access/htup.h
+++ b/src/include/access/htup.h
@@ -891,7 +891,7 @@ extern Size heap_compute_data_size(TupleDesc tupleDesc,
 extern void heap_fill_tuple(TupleDesc tupleDesc,
 				Datum *values, bool *isnull,
 				char *data, Size data_size,
-				uint16 *infomask, bits8 *bit);
+				uint16 *infomask, bits8 *bit, int lastNonNullAttributeOffset);
 extern bool heap_attisnull(HeapTuple tup, int attnum);
 extern Datum nocachegetattr(HeapTuple tup, int attnum,
 			   TupleDesc att);
diff --git a/src/test/regress/expected/trailing_nulls.out b/src/test/regress/expected/trailing_nulls.out
new file mode 100644
index 0000000..197e729
--- /dev/null
+++ b/src/test/regress/expected/trailing_nulls.out
@@ -0,0 +1,210 @@
+--
+-- The row bitmap and row length don't track trailing nulls. Verify that this works correctly.
+--
+-- simple case
+CREATE TABLE trailing_null (
+	c1 varchar(10), 
+	c2 varchar(10), 
+	c3 varchar(10), 
+	c4 varchar(10), 
+	c5 varchar(10) 
+	);
+	
+INSERT INTO trailing_null VALUES (null, null, null, null, null);
+INSERT INTO trailing_null VALUES (null, null, null, null, '5');
+INSERT INTO trailing_null VALUES (null, null, null, '4', null);
+INSERT INTO trailing_null VALUES (null, null, '3', null, null);
+INSERT INTO trailing_null VALUES (null, '2', null, null, null);
+INSERT INTO trailing_null VALUES ('1', null, null, null, null);
+SELECT * FROM trailing_null ORDER BY c1, c2, c3, c4, c5;
+ c1 | c2 | c3 | c4 | c5 
+----+----+----+----+----
+ 1  |    |    |    | 
+    | 2  |    |    | 
+    |    | 3  |    | 
+    |    |    | 4  | 
+    |    |    |    | 5
+    |    |    |    | 
+(6 rows)
+
+CREATE INDEX trailing_nulli ON trailing_null (c1, c2, c3, c4, c5);
+INSERT INTO trailing_null VALUES ('1', null, null, null, null);
+SELECT * FROM trailing_null ORDER BY c1, c2, c3, c4, c5;
+ c1 | c2 | c3 | c4 | c5 
+----+----+----+----+----
+ 1  |    |    |    | 
+ 1  |    |    |    | 
+    | 2  |    |    | 
+    |    | 3  |    | 
+    |    |    | 4  | 
+    |    |    |    | 5
+    |    |    |    | 
+(7 rows)
+
+DROP TABLE trailing_null;
+-- corner case, no columns
+CREATE TABLE trailing_null ();
+DROP TABLE trailing_null;
+-- toastable
+CREATE TABLE trailing_null (
+	c1 varchar(500), 
+	c2 varchar(500), 
+	c3 varchar(500), 
+	c4 varchar(500), 
+	c5 varchar(500), 
+	c6 varchar(500), 
+	c7 varchar(500), 
+	c8 varchar(500), 
+	c9 varchar(500), 
+	c10 varchar(500), 
+	c11 varchar(500), 
+	c12 varchar(500), 
+	c13 varchar(500), 
+	c14 varchar(500), 
+	c15 varchar(500), 
+	c16 varchar(500), 
+	c17 varchar(500)
+	);
+-- toast: easily compressed
+INSERT INTO trailing_null VALUES (
+	rpad('c1', 450, 'x'),
+	rpad('c2', 450, 'x'),
+	rpad('c3', 450, 'x'),
+	rpad('c4', 450, 'x'),
+	rpad('c5', 450, 'x'),
+	rpad('c6', 450, 'x'),
+	rpad('c7', 450, 'x'),
+	rpad('c8', 450, 'x'),
+	rpad('c9', 450, 'x'),
+	rpad('c10', 450, 'x'),
+	rpad('c11', 450, 'x'),
+	rpad('c12', 450, 'x'),
+	rpad('c13', 450, 'x'),
+	rpad('c14', 450, 'x'),
+	rpad('c15', 450, 'x'),
+	rpad('c16', 450, 'x'),
+	rpad('c17', 450, 'x')
+	);
+-- toast: off row storage
+INSERT INTO trailing_null VALUES (
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3)
+);
+-- verify that we have rows in our toast storage
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+ reltuples 
+-----------
+         4
+(1 row)
+
+	
+-- non-toast, separate codepath
+UPDATE trailing_null SET c17 = null;
+SELECT length(c1), length(c16), c17 FROM trailing_null;
+ length | length | c17 
+--------+--------+-----
+    450 |    450 | 
+    336 |    336 | 
+(2 rows)
+
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+ reltuples 
+-----------
+         4
+(1 row)
+
+UPDATE trailing_null SET c16 = null;
+SELECT length(c1), c16, c17 FROM trailing_null;
+ length | c16 | c17 
+--------+-----+-----
+    450 |     | 
+    336 |     | 
+(2 rows)
+
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+ reltuples 
+-----------
+         4
+(1 row)
+
+UPDATE trailing_null SET
+	c1 = null,
+	c2 = null,
+	c3 = null,
+	c4 = null,
+	c5 = null,
+	c6 = null,
+	c7 = null,
+	c8 = null,
+	c9 = null,
+	c10 = null,
+	c11 = null,
+	c12 = null,
+	c13 = null,
+	c14 = null,
+	c15 = null,
+	c16 = null,
+	c17 = null;
+-- verify that we got rid of our toasted data
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+ reltuples 
+-----------
+         4
+(1 row)
+
+	
+DROP TABLE trailing_null;
+-- Let's hit the detoasting logic related to composite objects. Borrowed
+-- from rowtypes.sql
+-- The object here is to ensure that toasted references inside
+-- composite values don't cause problems.  The large f1 value will
+-- be toasted inside pp, it must still work after being copied to people.
+CREATE TYPE fullname as (first text, last text, extra text);
+CREATE temp TABLE pp (f1 text, c1 varchar(10) null);
+INSERT INTO pp VALUES (repeat('abcdefghijkl', 100000), null);
+CREATE temp TABLE people (fn fullname, c1 varchar(10), c2 varchar(10));
+-- trailing null in our nested object 
+INSERT into people SELECT('Jim', f1, null)::fullname, c1, null from pp;
+SELECT (fn).first, length((fn).last), c1, c2 from people;
+ first | length  | c1 | c2 
+-------+---------+----+----
+ Jim   | 1200000 |    | 
+(1 row)
+
+ALTER TABLE people ADD COLUMN c3 varchar(10);
+SELECT (fn).first, length((fn).last), c1, c2, c3 from people;
+ first | length  | c1 | c2 | c3 
+-------+---------+----+----+----
+ Jim   | 1200000 |    |    | 
+(1 row)
+
+DROP TABLE pp;
+DROP TABLE people;
+DROP TYPE fullname;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 8852e0a..9ab329d 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -92,7 +92,7 @@ test: rules
 # ----------
 # Another group of parallel tests
 # ----------
-test: select_views portals_p2 foreign_key cluster dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock json
+test: select_views portals_p2 foreign_key cluster dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock json trailing_nulls 
 
 # ----------
 # Another group of parallel tests
diff --git a/src/test/regress/sql/trailing_nulls.sql b/src/test/regress/sql/trailing_nulls.sql
new file mode 100644
index 0000000..f617257
--- /dev/null
+++ b/src/test/regress/sql/trailing_nulls.sql
@@ -0,0 +1,166 @@
+--
+-- The row bitmap and row length don't track trailing nulls. Verify that this works correctly.
+--
+
+-- simple case
+CREATE TABLE trailing_null (
+	c1 varchar(10), 
+	c2 varchar(10), 
+	c3 varchar(10), 
+	c4 varchar(10), 
+	c5 varchar(10) 
+	);
+	
+INSERT INTO trailing_null VALUES (null, null, null, null, null);
+INSERT INTO trailing_null VALUES (null, null, null, null, '5');
+INSERT INTO trailing_null VALUES (null, null, null, '4', null);
+INSERT INTO trailing_null VALUES (null, null, '3', null, null);
+INSERT INTO trailing_null VALUES (null, '2', null, null, null);
+INSERT INTO trailing_null VALUES ('1', null, null, null, null);
+
+SELECT * FROM trailing_null ORDER BY c1, c2, c3, c4, c5;
+
+CREATE INDEX trailing_nulli ON trailing_null (c1, c2, c3, c4, c5);
+
+INSERT INTO trailing_null VALUES ('1', null, null, null, null);
+
+SELECT * FROM trailing_null ORDER BY c1, c2, c3, c4, c5;
+
+DROP TABLE trailing_null;
+
+-- corner case, no columns
+CREATE TABLE trailing_null ();
+DROP TABLE trailing_null;
+
+
+-- toastable
+CREATE TABLE trailing_null (
+	c1 varchar(500), 
+	c2 varchar(500), 
+	c3 varchar(500), 
+	c4 varchar(500), 
+	c5 varchar(500), 
+	c6 varchar(500), 
+	c7 varchar(500), 
+	c8 varchar(500), 
+	c9 varchar(500), 
+	c10 varchar(500), 
+	c11 varchar(500), 
+	c12 varchar(500), 
+	c13 varchar(500), 
+	c14 varchar(500), 
+	c15 varchar(500), 
+	c16 varchar(500), 
+	c17 varchar(500)
+	);
+
+-- toast: easily compressed
+INSERT INTO trailing_null VALUES (
+	rpad('c1', 450, 'x'),
+	rpad('c2', 450, 'x'),
+	rpad('c3', 450, 'x'),
+	rpad('c4', 450, 'x'),
+	rpad('c5', 450, 'x'),
+	rpad('c6', 450, 'x'),
+	rpad('c7', 450, 'x'),
+	rpad('c8', 450, 'x'),
+	rpad('c9', 450, 'x'),
+	rpad('c10', 450, 'x'),
+	rpad('c11', 450, 'x'),
+	rpad('c12', 450, 'x'),
+	rpad('c13', 450, 'x'),
+	rpad('c14', 450, 'x'),
+	rpad('c15', 450, 'x'),
+	rpad('c16', 450, 'x'),
+	rpad('c17', 450, 'x')
+	);
+
+-- toast: off row storage
+INSERT INTO trailing_null VALUES (
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3),
+ repeat('1234567890abcdefghijklmnopqrstuvwzyzABCDEFGHIJKLMNOPQRSTUVWZYZ0987654321zywvutsrqponmlkjihgfedcba1b2c3e5f6g7h9i0', 3)
+);
+
+-- verify that we have rows in our toast storage
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+	
+-- non-toast, separate codepath
+UPDATE trailing_null SET c17 = null;
+SELECT length(c1), length(c16), c17 FROM trailing_null;
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+
+UPDATE trailing_null SET c16 = null;
+SELECT length(c1), c16, c17 FROM trailing_null;
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+
+UPDATE trailing_null SET
+	c1 = null,
+	c2 = null,
+	c3 = null,
+	c4 = null,
+	c5 = null,
+	c6 = null,
+	c7 = null,
+	c8 = null,
+	c9 = null,
+	c10 = null,
+	c11 = null,
+	c12 = null,
+	c13 = null,
+	c14 = null,
+	c15 = null,
+	c16 = null,
+	c17 = null;
+
+-- verify that we got rid of our toasted data
+VACUUM FULL trailing_null;
+VACUUM ANALYZE trailing_null;
+SELECT CAST(reltuples AS INTEGER) FROM pg_class
+	WHERE oid = (SELECT reltoastrelid FROM pg_class where relname = 'trailing_null');
+	
+DROP TABLE trailing_null;
+
+-- Let's hit the detoasting logic related to composite objects. Borrowed
+-- from rowtypes.sql
+
+-- The object here is to ensure that toasted references inside
+-- composite values don't cause problems.  The large f1 value will
+-- be toasted inside pp, it must still work after being copied to people.
+CREATE TYPE fullname as (first text, last text, extra text);
+CREATE temp TABLE pp (f1 text, c1 varchar(10) null);
+INSERT INTO pp VALUES (repeat('abcdefghijkl', 100000), null);
+CREATE temp TABLE people (fn fullname, c1 varchar(10), c2 varchar(10));
+
+-- trailing null in our nested object 
+INSERT into people SELECT('Jim', f1, null)::fullname, c1, null from pp;
+SELECT (fn).first, length((fn).last), c1, c2 from people;
+ALTER TABLE people ADD COLUMN c3 varchar(10);
+SELECT (fn).first, length((fn).last), c1, c2, c3 from people;
+
+DROP TABLE pp;
+DROP TABLE people;
+DROP TYPE fullname;
-- 
1.7.0.4