8000 Don't assume that a tuple's header size is unchanged during toasting. · postgres/postgres@73e8ee9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 73e8ee9

Browse files
tglsfdctglsfdc
committed
Don't assume that a tuple's header size is unchanged during toasting.
This assumption can be wrong when the toaster is passed a raw on-disk tuple, because the tuple might pre-date an ALTER TABLE ADD COLUMN operation that added columns without rewriting the table. In such a case the tuple's natts value is smaller than what we expect from the tuple descriptor, and so its t_hoff value could be smaller too. In fact, the tuple might not have a null bitmap at all, and yet our current opinion of it is that it contains some trailing nulls. In such a situation, toast_insert_or_update did the wrong thing, because to save a few lines of code it would use the old t_hoff value as the offset where heap_fill_tuple should start filling data. This did not leave enough room for the new nulls bitmap, with the result that the first few bytes of data could be overwritten with null flag bits, as in a recent report from Hubert Depesz Lubaczewski. The particular case reported requires ALTER TABLE ADD COLUMN followed by CREATE TABLE AS SELECT * FROM ... or INSERT ... SELECT * FROM ..., and further requires that there be some out-of-line toasted fields in one of the tuples to be copied; else we'll not reach the troublesome code. The problem can only manifest in this form in 8.4 and later, because before commit a77eaa6, CREATE TABLE AS or INSERT/SELECT wouldn't result in raw disk tuples getting passed directly to heap_insert --- there would always have been at least a junkfilter in between, and that would reconstitute the tuple header with an up-to-date t_natts and hence t_hoff. But I'm backpatching the tuptoaster change all the way anyway, because I'm not convinced there are no older code paths that present a similar risk.
1 parent b24e6ca commit 73e8ee9

File tree

1 file changed

+52
-30
lines changed

1 file changed

+52
-30
lines changed

‎src/backend/access/heap/tuptoaster.c

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,6 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
514514
if (newtup->t_data->t_infomask & HEAP_HASOID)
515515
maxDataLen += sizeof(Oid);
516516
maxDataLen = MAXALIGN(maxDataLen);
517-
Assert(maxDataLen == newtup->t_data->t_hoff);
518517
/* now convert to a limit on the tuple data size */
519518
maxDataLen = TOAST_TUPLE_TARGET - maxDataLen;
520519

@@ -748,41 +747,54 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
748747
{
749748
HeapTupleHeader olddata = newtup->t_data;
750749
HeapTupleHeader new_data;
751-
int32 new_len;
750+
int32 new_header_len;
751+
int32 new_data_len;
752+
int32 new_tuple_len;
752753

753754
/*
754-
* Calculate the new size of the tuple. Header size should not
755-
* change, but data size might.
755+
* Calculate the new size of the tuple.
756+
*
757+
* Note: we used to assume here that the old tuple's t_hoff must equal
758+
* the new_header_len value, but that was incorrect. The old tuple
759+
* might have a smaller-than-current natts, if there's been an ALTER
760+
* TABLE ADD COLUMN since it was stored; and that would lead to a
761+
* different conclusion about the size of the null bitmap, or even
762+
* whether there needs to be one at all.
756763
*/
757-
new_len = offsetof(HeapTupleHeaderData, t_bits);
764+
new_header_len = offsetof(HeapTupleHeaderData, t_bits);
758765
if (has_nulls)
759-
new_len += BITMAPLEN(numAttrs);
766+
new_header_len += BITMAPLEN(numAttrs);
760767
if (olddata->t_infomask & HEAP_HASOID)
761-
new_len += sizeof(Oid);
762-
new_len = MAXALIGN(new_len);
763-
Assert(new_len == olddata->t_hoff);
764-
new_len += heap_compute_data_size(tupleDesc,
765-
toast_values, toast_isnull);
768+
new_header_len += sizeof(Oid);
769+
new_header_len = MAXALIGN(new_header_len);
770+
new_data_len = heap_compute_data_size(tupleDesc,
771+
toast_values, toast_isnull);
772+
new_tuple_len = new_header_len + new_data_len;
766773

767774
/*
768775
* Allocate and zero the space needed, and fill HeapTupleData fields.
769776
*/
770-
result_tuple = (HeapTuple) palloc0(HEAPTUPLESIZE + new_len);
771-
result_tuple->t_len = new_len;
777+
result_tuple = (HeapTuple) palloc0(HEAPTUPLESIZE + new_tuple_len);
778+
result_tuple->t_len = new_tuple_len;
772779
result_tuple->t_self = newtup->t_self;
773780
result_tuple->t_tableOid = newtup->t_tableOid;
774781
new_data = (HeapTupleHeader) ((char *) result_tuple + HEAPTUPLESIZE);
775782
result_tuple->t_data = new_data;
776783

777784
/*
778-
* Put the existing tuple header and the changed values into place
785+
* Copy the existing tuple header, but adjust natts and t_hoff.
779786
*/
780-
memcpy(new_data, olddata, olddata->t_hoff);
787+
memcpy(new_data, olddata, offsetof(HeapTupleHeaderData, t_bits));
788+
new_data->t_natts = numAttrs;
789+
new_data->t_hoff = new_header_len;
790+
if (olddata->t_infomask & HEAP_HASOID)
791+
HeapTupleHeaderSetOid(new_data, HeapTupleHeaderGetOid(olddata));
781792

793+
/* Copy over the data, and fill the null bitmap if needed */
782794
heap_fill_tuple(tupleDesc,
783795
toast_values,
784796
toast_isnull,
785-
(char *) new_data + olddata->t_hoff,
797+
(char *) new_data + new_header_len,
786798
&(new_data->t_infomask),
787799
has_nulls ? new_data->t_bits : NULL);
788800
}
@@ -903,7 +915,9 @@ toast_flatten_tuple_attribute(Datum value,
903915
TupleDesc tupleDesc;
904916
HeapTupleHeader olddata;
905917
HeapTupleHeader new_data;
906-
int32 new_len;
918+
int32 new_header_len;
919+
int32 new_data_len;
920+
int32 new_tuple_len;
907921
HeapTupleData tmptup;
908922
Form_pg_attribute *att;
909923
int numAttrs;
@@ -973,31 +987,39 @@ toast_flatten_tuple_attribute(Datum value,
973987
}
974988

975989
/*
976-
* Calculate the new size of the tuple. Header size should not change,
977-
* but data size might.
990+
* Calculate the new size of the tuple.
991+
*
992+
* This should match the reconstruction code in toast_insert_or_update.
978993
*/
979-
new_len = offsetof(HeapTupleHeaderData, t_bits);
994+
new_header_len = offsetof(HeapTupleHeaderData, t_bits);
980995
if (has_nulls)
981-
new_len += BITMAPLEN(numAttrs);
996+
new_header_len += BITMAPLEN(numAttrs);
982997
if (olddata->t_infomask & HEAP_HASOID)
983-
new_len += sizeof(Oid);
984-
new_len = MAXALIGN(new_len);
985-
Assert(new_len == olddata->t_hoff);
986-
new_len += heap_compute_data_size(tupleDesc, toast_values, toast_isnull);
998+
new_header_len += sizeof(Oid);
999+
new_header_len = MAXALIGN(new_header_len);
1000+
new_data_len = heap_compute_data_size(tupleDesc,
1001+
toast_values, toast_isnull);
1002+
new_tuple_len = new_header_len + new_data_len;
9871003

988-
new_data = (HeapTupleHeader) palloc0(new_len);
1004+
new_data = (HeapTupleHeader) palloc0(new_tuple_len);
9891005

9901006
/*
991-
* Put the tuple header and the changed values into place
1007+
* Copy the existing tuple header, but adjust natts and t_hoff.
9921008
*/
993-
memcpy(new_data, olddata, olddata->t_hoff);
1009+
memcpy(new_data, olddata, offsetof(HeapTupleHeaderData, t_bits));
1010+
new_data->t_natts = numAttrs;
1011+
new_data->t_hoff = new_header_len;
1012+
if (olddata->t_infomask & HEAP_HASOID)
1013+
HeapTupleHeaderSetOid(new_data, HeapTupleHeaderGetOid(olddata));
9941014

995-
HeapTupleHeaderSetDatumLength(new_data, new_len);
1015+
/* Reset the datum length field, too */
1016+
HeapTupleHeaderSetDatumLength(new_data, new_tuple_len);
9961017

1018+
/* Copy over the data, and fill the null bitmap if needed */
9971019
heap_fill_tuple(tupleDesc,
9981020
toast_values,
9991021
toast_isnull,
1000-
(char *) new_data + olddata->t_hoff,
1022+
(char *) new_data + new_header_len,
10011023
&(new_data->t_infomask),
10021024
has_nulls ? new_data->t_bits : NULL);
10031025

0 commit comments

Comments
 (0)
0