Skip to content

Commit 70debcf

Browse files
tglsfdctglsfdc
committed
Fix failure to detoast fields in composite elements of structured types.
If we have an array of records stored on disk, the individual record fields cannot contain out-of-line TOAST pointers: the tuptoaster.c mechanisms are only prepared to deal with TOAST pointers appearing in top-level fields of a stored row. The same applies for ranges over composite types, nested composites, etc. However, the existing code only took care of expanding sub-field TOAST pointers for the case of nested composites, not for other structured types containing composites. For example, given a command such as UPDATE tab SET arraycol = ARRAY[(ROW(x,42)::mycompositetype] ... where x is a direct reference to a field of an on-disk tuple, if that field is long enough to be toasted out-of-line then the TOAST pointer would be inserted as-is into the array column. If the source record for x is later deleted, the array field value would become a dangling pointer, leading to errors along the line of "missing chunk number 0 for toast value ..." when the value is referenced. A reproducible test case for this was provided by Jan Pecek, but it seems likely that some of the "missing chunk number" reports we've heard in the past were caused by similar issues. Code-wise, the problem is that PG_DETOAST_DATUM() is not adequate to produce a self-contained Datum value if the Datum is of composite type. Seen in this light, the problem is not just confined to arrays and ranges, but could also affect some other places where detoasting is done in that way, for example form_index_tuple(). I tried teaching the array code to apply toast_flatten_tuple_attribute() along with PG_DETOAST_DATUM() when the array element type is composite, but this was messy and imposed extra cache lookup costs whether or not any TOAST pointers were present, indeed sometimes when the array element type isn't even composite (since sometimes it takes a typcache lookup to find that out). The idea of extending that approach to all the places that currently use PG_DETOAST_DATUM() wasn't attractive at all. This patch instead solves the problem by decreeing that composite Datum values must not contain any out-of-line TOAST pointers in the first place; that is, we expand out-of-line fields at the point of constructing a composite Datum, not at the point where we're about to insert it into a larger tuple. This rule is applied only to true composite Datums, not to tuples that are being passed around the system as tuples, so it's not as invasive as it might sound at first. With this approach, the amount of code that has to be touched for a full solution is greatly reduced, and added cache lookup costs are avoided except when there actually is a TOAST pointer that needs to be inlined. The main drawback of this approach is that we might sometimes dereference a TOAST pointer that will never actually be used by the query, imposing a rather large cost that wasn't there before. On the other side of the coin, if the field value is used multiple times then we'll come out ahead by avoiding repeat detoastings. Experimentation suggests that common SQL coding patterns are unaffected either way, though. Applications that are very negatively affected could be advised to modify their code to not fetch columns they won't be using. In future, we might consider reverting this solution in favor of detoasting only at the point where data is about to be stored to disk, using some method that can drill down into multiple levels of nested structured types. That will require defining new APIs for structured types, though, so it doesn't seem feasible as a back-patchable fix. Note that this patch changes HeapTupleGetDatum() from a macro to a function call; this means that any third-party code using that macro will not get protection against creating TOAST-pointer-containing Datums until it's recompiled. The same applies to any uses of PG_RETURN_HEAPTUPLEHEADER(). It seems likely that this is not a big problem in practice: most of the tuple-returning functions in core and contrib produce outputs that could not possibly be toasted anyway, and the same probably holds for third-party extensions. This bug has existed since TOAST was invented, so back-patch to all supported branches.
1 parent f49df91 commit 70debcf

File tree

15 files changed

+235
-150
lines changed

15 files changed

+235
-150
lines changed

‎src/backend/access/common/heaptuple.c

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,41 @@ heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest)
658658
memcpy((char *) dest->t_data, (char *) src->t_data, src->t_len);
659659
}
660660

661+
/* ----------------
662+
* heap_copy_tuple_as_datum
663+
*
664+
* copy a tuple as a composite-type Datum
665+
* ----------------
666+
*/
667+
Datum
668+
heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc)
669+
{
670+
HeapTupleHeader td;
671+
672+
/*
673+
* If the tuple contains any external TOAST pointers, we have to inline
674+
* those fields to meet the conventions for composite-type Datums.
675+
*/
676+
if (HeapTupleHasExternal(tuple))
677+
return toast_flatten_tuple_to_datum(tuple->t_data,
678+
tuple->t_len,
679+
tupleDesc);
680+
681+
/*
682+
* Fast path for easy case: just make a palloc'd copy and insert the
683+
* correct composite-Datum header fields (since those may not be set if
684+
* the given tuple came from disk, rather than from heap_form_tuple).
685+
*/
686+
td = (HeapTupleHeader) palloc(tuple->t_len);
687+
memcpy((char *) td, (char *) tuple->t_data, tuple->t_len);
688+
689+
HeapTupleHeaderSetDatumLength(td, tuple->t_len);
690+
HeapTupleHeaderSetTypeId(td, tupleDesc->tdtypeid);
691+
HeapTupleHeaderSetTypMod(td, tupleDesc->tdtypmod);
692+
693+
return PointerGetDatum(td);
694+
}
695+
661696
/*
662697
* heap_form_tuple
663698
* construct a tuple from the given values[] and isnull[] arrays,
@@ -676,7 +711,6 @@ heap_form_tuple(TupleDesc tupleDescriptor,
676711
data_len;
677712
int hoff;
678713
bool hasnull = false;
679-
Form_pg_attribute *att = tupleDescriptor->attrs;
680714
int numberOfAttributes = tupleDescriptor->natts;
681715
int i;
682716

@@ -687,28 +721,14 @@ heap_form_tuple(TupleDesc tupleDescriptor,
687721
numberOfAttributes, MaxTupleAttributeNumber)));
688722

689723
/*
690-
* Check for nulls and embedded tuples; expand any toasted attributes in
691-
* embedded tuples. This preserves the invariant that toasting can only
692-
* go one level deep.
693-
*
694-
* We can skip calling toast_flatten_tuple_attribute() if the attribute
695-
* couldn't possibly be of composite type. All composite datums are
696-
* varlena and have alignment 'd'; furthermore they aren't arrays. Also,
697-
* if an attribute is already toasted, it must have been sent to disk
698-
* already and so cannot contain toasted attributes.
724+
* Check for nulls
699725
*/
700726
for (i = 0; i < numberOfAttributes; i++)
701727
{
702728
if (isnull[i])
703-
hasnull = true;
704-
else if (att[i]->attlen == -1 &&
705-
att[i]->attalign == 'd' &&
706-
att[i]->attndims == 0 &&
707-
!VARATT_IS_EXTENDED(DatumGetPointer(values[i])))
708729
{
709-
values[i] = toast_flatten_tuple_attribute(values[i],
710-
att[i]->atttypid,
711-
att[i]->atttypmod);
730+
hasnull = true;
731+
break;
712732
}
713733
}
714734

@@ -738,7 +758,8 @@ heap_form_tuple(TupleDesc tupleDescriptor,
738758

739759
/*
740760
* And fill in the information. Note we fill the Datum fields even though
741-
* this tuple may never become a Datum.
761+
* this tuple may never become a Datum. This lets HeapTupleHeaderGetDatum
762+
* identify the tuple type if needed.
742763
*/
743764
tuple->t_len = len;
744765
ItemPointerSetInvalid(&(tuple->t_self));
@@ -1428,7 +1449,6 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
14281449
data_len;
14291450
int hoff;
14301451
bool hasnull = false;
1431-
Form_pg_attribute *att = tupleDescriptor->attrs;
14321452
int numberOfAttributes = tupleDescriptor->natts;
14331453
int i;
14341454

@@ -1439,28 +1459,14 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
14391459
numberOfAttributes, MaxTupleAttributeNumber)));
14401460

14411461
/*
1442-
* Check for nulls and embedded tuples; expand any toasted attributes in
1443-
* embedded tuples. This preserves the invariant that toasting can only
1444-
* go one level deep.
1445-
*
1446-
* We can skip calling toast_flatten_tuple_attribute() if the attribute
1447-
* couldn't possibly be of composite type. All composite datums are
1448-
* varlena and have alignment 'd'; furthermore they aren't arrays. Also,
1449-
* if an attribute is already toasted, it must have been sent to disk
1450-
* already and so cannot contain toasted attributes.
1462+
* Check for nulls
14511463
*/
14521464
for (i = 0; i < numberOfAttributes; i++)
14531465
{
14541466
if (isnull[i])
1455-
hasnull = true;
1456-
else if (att[i]->attlen == -1 &&
1457-
att[i]->attalign == 'd' &&
1458-
att[i]->attndims == 0 &&
1459-
!VARATT_IS_EXTENDED(values[i]))
14601467
{
1461-
values[i] = toast_flatten_tuple_attribute(values[i],
1462-
att[i]->atttypid,
1463-
att[i]->atttypmod);
1468+
hasnull = true;
1469+
break;
14641470
}
14651471
}
14661472

‎src/backend/access/common/indextuple.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ index_form_tuple(TupleDesc tupleDescriptor,
158158
if (tupmask & HEAP_HASVARWIDTH)
159159
infomask |= INDEX_VAR_MASK;
160160

161+
/* Also assert we got rid of external attributes */
162+
#ifdef TOAST_INDEX_HACK
163+
Assert((tupmask & HEAP_HASEXTERNAL) == 0);
164+
#endif
165+
161166
/*
162167
* Here we make sure that the size will fit in the field reserved for it
163168
* in t_info.

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

Lines changed: 44 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,9 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
931931
*
932932
* "Flatten" a tuple to contain no out-of-line toasted fields.
933933
* (This does not eliminate compressed or short-header datums.)
934+
*
935+
* Note: we expect the caller already checked HeapTupleHasExternal(tup),
936+
* so there is no need for a short-circuit path.
934937
* ----------
935938
*/
936939
HeapTuple
@@ -1008,59 +1011,61 @@ toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc)
10081011

10091012

10101013
/* ----------
1011-
* toast_flatten_tuple_attribute -
1014+
* toast_flatten_tuple_to_datum -
1015+
*
1016+
* "Flatten" a tuple containing out-of-line toasted fields into a Datum.
1017+
* The result is always palloc'd in the current memory context.
1018+
*
1019+
* We have a general rule that Datums of container types (rows, arrays,
1020+
* ranges, etc) must not contain any external TOAST pointers. Without
1021+
* this rule, we'd have to look inside each Datum when preparing a tuple
1022+
* for storage, which would be expensive and would fail to extend cleanly
1023+
* to new sorts of container types.
1024+
*
1025+
* However, we don't want to say that tuples represented as HeapTuples
1026+
* can't contain toasted fields, so instead this routine should be called
1027+
* when such a HeapTuple is being converted into a Datum.
10121028
*
1013-
* If a Datum is of composite type, "flatten" it to contain no toasted fields.
1014-
* This must be invoked on any potentially-composite field that is to be
1015-
* inserted into a tuple. Doing this preserves the invariant that toasting
1016-
* goes only one level deep in a tuple.
1029+
* While we're at it, we decompress any compressed fields too. This is not
1030+
* necessary for correctness, but reflects an expectation that compression
1031+
* will be more effective if applied to the whole tuple not individual
1032+
* fields. We are not so concerned about that that we want to deconstruct
1033+
* and reconstruct tuples just to get rid of compressed fields, however.
1034+
* So callers typically won't call this unless they see that the tuple has
1035+
* at least one external field.
10171036
*
1018-
* Note that flattening does not mean expansion of short-header varlenas,
1019-
* so in one sense toasting is allowed within composite datums.
1037+
* On the other hand, in-line short-header varlena fields are left alone.
1038+
* If we "untoasted" them here, they'd just get changed back to short-header
1039+
* format anyway within heap_fill_tuple.
10201040
* ----------
10211041
*/
10221042
Datum
1023-
toast_flatten_tuple_attribute(Datum value,
1024-
Oid typeId, int32 typeMod)
1043+
toast_flatten_tuple_to_datum(HeapTupleHeader tup,
1044+
uint32 tup_len,
1045+
TupleDesc tupleDesc)
10251046
{
1026-
TupleDesc tupleDesc;
1027-
HeapTupleHeader olddata;
10281047
HeapTupleHeader new_data;
10291048
int32 new_header_len;
10301049
int32 new_data_len;
10311050
int32 new_tuple_len;
10321051
HeapTupleData tmptup;
1033-
Form_pg_attribute *att;
1034-
int numAttrs;
1052+
Form_pg_attribute *att = tupleDesc->attrs;
1053+
int numAttrs = tupleDesc->natts;
10351054
int i;
1036-
bool need_change = false;
10371055
bool has_nulls = false;
10381056
Datum toast_values[MaxTupleAttributeNumber];
10391057
bool toast_isnull[MaxTupleAttributeNumber];
10401058
bool toast_free[MaxTupleAttributeNumber];
10411059

1042-
/*
1043-
* See if it's a composite type, and get the tupdesc if so.
1044-
*/
1045-
tupleDesc = lookup_rowtype_tupdesc_noerror(typeId, typeMod, true);
1046-
if (tupleDesc == NULL)
1047-
return value; /* not a composite type */
1048-
1049-
att = tupleDesc->attrs;
1050-
numAttrs = tupleDesc->natts;
1051-
1052-
/*
1053-
* Break down the tuple into fields.
1054-
*/
1055-
olddata = DatumGetHeapTupleHeader(value);
1056-
Assert(typeId == HeapTupleHeaderGetTypeId(olddata));
1057-
Assert(typeMod == HeapTupleHeaderGetTypMod(olddata));
10581060
/* Build a temporary HeapTuple control structure */
1059-
tmptup.t_len = HeapTupleHeaderGetDatumLength(olddata);
1061+
tmptup.t_len = tup_len;
10601062
ItemPointerSetInvalid(&(tmptup.t_self));
10611063
tmptup.t_tableOid = InvalidOid;
1062-
tmptup.t_data = olddata;
1064+
tmptup.t_data = tup;
10631065

1066+
/*
1067+
* Break down the tuple into fields.
1068+
*/
10641069
Assert(numAttrs <= MaxTupleAttributeNumber);
10651070
heap_deform_tuple(&tmptup, tupleDesc, toast_values, toast_isnull);
10661071

@@ -1084,20 +1089,10 @@ toast_flatten_tuple_attribute(Datum value,
10841089
new_value = heap_tuple_untoast_attr(new_value);
10851090
toast_values[i] = PointerGetDatum(new_value);
10861091
toast_free[i] = true;
1087-
need_change = true;
10881092
}
10891093
}
10901094
}
10911095

1092-
/*
1093-
* If nothing to untoast, just return the original tuple.
1094-
*/
1095-
if (!need_change)
1096-
{
1097-
ReleaseTupleDesc(tupleDesc);
1098-
return value;
1099-
}
1100-
11011096
/*
11021097
* Calculate the new size of the tuple.
11031098
*
@@ -1106,7 +1101,7 @@ toast_flatten_tuple_attribute(Datum value,
11061101
new_header_len = offsetof(HeapTupleHeaderData, t_bits);
11071102
if (has_nulls)
11081103
new_header_len += BITMAPLEN(numAttrs);
1109-
if (olddata->t_infomask & HEAP_HASOID)
1104+
if (tup->t_infomask & HEAP_HASOID)
11101105
new_header_len += sizeof(Oid);
11111106
new_header_len = MAXALIGN(new_header_len);
11121107
new_data_len = heap_compute_data_size(tupleDesc,
@@ -1118,14 +1113,16 @@ toast_flatten_tuple_attribute(Datum value,
11181113
/*
11191114
* Copy the existing tuple header, but adjust natts and t_hoff.
11201115
*/
1121-
memcpy(new_data, olddata, offsetof(HeapTupleHeaderData, t_bits));
1116+
memcpy(new_data, tup, offsetof(HeapTupleHeaderData, t_bits));
11221117
HeapTupleHeaderSetNatts(new_data, numAttrs);
11231118
new_data->t_hoff = new_header_len;
1124-
if (olddata->t_infomask & HEAP_HASOID)
1125-
HeapTupleHeaderSetOid(new_data, HeapTupleHeaderGetOid(olddata));
1119+
if (tup->t_infomask & HEAP_HASOID)
1120+
HeapTupleHeaderSetOid(new_data, HeapTupleHeaderGetOid(tup));
11261121

1127-
/* Reset the datum length field, too */
1122+
/* Set the composite-Datum header fields correctly */
11281123
HeapTupleHeaderSetDatumLength(new_data, new_tuple_len);
1124+
HeapTupleHeaderSetTypeId(new_data, tupleDesc->tdtypeid);
1125+
HeapTupleHeaderSetTypMod(new_data, tupleDesc->tdtypmod);
11291126

11301127
/* Copy over the data, and fill the null bitmap if needed */
11311128
heap_fill_tuple(tupleDesc,
@@ -1142,7 +1139,6 @@ toast_flatten_tuple_attribute(Datum value,
11421139
for (i = 0; i < numAttrs; i++)
11431140
if (toast_free[i])
11441141
pfree(DatumGetPointer(toast_values[i]));
1145-
ReleaseTupleDesc(tupleDesc);
11461142

11471143
return PointerGetDatum(new_data);
11481144
}

‎src/backend/executor/execQual.c

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -882,8 +882,6 @@ ExecEvalWholeRowFast(WholeRowVarExprState *wrvstate, ExprContext *econtext,
882882
{
883883
Var *variable = (Var *) wrvstate->xprstate.expr;
884884
TupleTableSlot *slot = econtext->ecxt_scantuple;
885-
HeapTuple tuple;
886-
TupleDesc tupleDesc;
887885
HeapTupleHeader dtuple;
888886

889887
if (isDone)
@@ -894,32 +892,20 @@ ExecEvalWholeRowFast(WholeRowVarExprState *wrvstate, ExprContext *econtext,
894892
if (wrvstate->wrv_junkFilter != NULL)
895893
slot = ExecFilterJunk(wrvstate->wrv_junkFilter, slot);
896894

897-
tuple = ExecFetchSlotTuple(slot);
898-
tupleDesc = slot->tts_tupleDescriptor;
899-
900895
/*
901-
* We have to make a copy of the tuple so we can safely insert the Datum
902-
* overhead fields, which are not set in on-disk tuples.
896+
* Copy the slot tuple and make sure any toasted fields get detoasted.
903897
*/
904-
dtuple = (HeapTupleHeader) palloc(tuple->t_len);
905-
memcpy((char *) dtuple, (char *) tuple->t_data, tuple->t_len);
906-
907-
HeapTupleHeaderSetDatumLength(dtuple, tuple->t_len);
898+
dtuple = DatumGetHeapTupleHeader(ExecFetchSlotTupleDatum(slot));
908899

909900
/*
910-
* If the Var identifies a named composite type, label the tuple with that
911-
* type; otherwise use what is in the tupleDesc.
901+
* If the Var identifies a named composite type, label the datum with that
902+
* type; otherwise we'll use the slot's info.
912903
*/
913904
if (variable->vartype != RECORDOID)
914905
{
915906
HeapTupleHeaderSetTypeId(dtuple, variable->vartype);
916907
HeapTupleHeaderSetTypMod(dtuple, variable->vartypmod);
917908
}
918-
else
919-
{
920-
HeapTupleHeaderSetTypeId(dtuple, tupleDesc->tdtypeid);
921-
HeapTupleHeaderSetTypMod(dtuple, tupleDesc->tdtypmod);
922-
}
923909

924910
return PointerGetDatum(dtuple);
925911
}
@@ -977,13 +963,13 @@ ExecEvalWholeRowSlow(WholeRowVarExprState *wrvstate, ExprContext *econtext,
977963
}
978964

979965
/*
980-
* We have to make a copy of the tuple so we can safely insert the Datum
981-
* overhead fields, which are not set in on-disk tuples.
966+
* Copy the slot tuple and make sure any toasted fields get detoasted.
982967
*/
983-
dtuple = (HeapTupleHeader) palloc(tuple->t_len);
984-
memcpy((char *) dtuple, (char *) tuple->t_data, tuple->t_len);
968+
dtuple = DatumGetHeapTupleHeader(ExecFetchSlotTupleDatum(slot));
985969

986-
HeapTupleHeaderSetDatumLength(dtuple, tuple->t_len);
970+
/*
971+
* Reset datum's type ID fields to match the Var.
972+
*/
987973
HeapTupleHeaderSetTypeId(dtuple, variable->vartype);
988974
HeapTupleHeaderSetTypMod(dtuple, variable->vartypmod);
989975

0 commit comments

Comments
 (0)