diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 3f1fa544f9..4246f07b86 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -1929,7 +1929,8 @@ CheckOpSlotCompatibility(ExprEvalStep *op, TupleTableSlot *slot) * changed: if not NULL, *changed is set to true on any update * * The returned TupleDesc is not guaranteed pinned; caller must pin it - * to use it across any operation that might incur cache invalidation. + * to use it across any operation that might incur cache invalidation, + * including for example detoasting of input tuples. * (The TupleDesc is always refcounted, so just use IncrTupleDescRefCount.) * * NOTE: because composite types can change contents, we must be prepared @@ -3033,17 +3034,6 @@ ExecEvalFieldSelect(ExprState *state, ExprEvalStep *op, ExprContext *econtext) void ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econtext) { - TupleDesc tupDesc; - - /* Lookup tupdesc if first time through or if type changes */ - tupDesc = get_cached_rowtype(op->d.fieldstore.fstore->resulttype, -1, - op->d.fieldstore.rowcache, NULL); - - /* Check that current tupdesc doesn't have more fields than we allocated */ - if (unlikely(tupDesc->natts > op->d.fieldstore.ncolumns)) - elog(ERROR, "too many columns in composite type %u", - op->d.fieldstore.fstore->resulttype); - if (*op->resnull) { /* Convert null input tuple into an all-nulls row */ @@ -3059,6 +3049,7 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte Datum tupDatum = *op->resvalue; HeapTupleHeader tuphdr; HeapTupleData tmptup; + TupleDesc tupDesc; tuphdr = DatumGetHeapTupleHeader(tupDatum); tmptup.t_len = HeapTupleHeaderGetDatumLength(tuphdr); @@ -3066,6 +3057,20 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte tmptup.t_tableOid = InvalidOid; tmptup.t_data = tuphdr; + /* + * Lookup tupdesc if first time through or if type changes. Because + * we don't pin the tupdesc, we must not do this lookup until after + * doing DatumGetHeapTupleHeader: that could do database access while + * detoasting the datum. + */ + tupDesc = get_cached_rowtype(op->d.fieldstore.fstore->resulttype, -1, + op->d.fieldstore.rowcache, NULL); + + /* Check that current tupdesc doesn't have more fields than allocated */ + if (unlikely(tupDesc->natts > op->d.fieldstore.ncolumns)) + elog(ERROR, "too many columns in composite type %u", + op->d.fieldstore.fstore->resulttype); + heap_deform_tuple(&tmptup, tupDesc, op->d.fieldstore.values, op->d.fieldstore.nulls); diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index e722d10a1f..69058244fc 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -139,6 +139,15 @@ select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people; Jim | abcdefghijklabcdefgh | 1200000 (2 rows) +-- try an update on a toasted composite value, too +update people set fn.first = 'Jack'; +select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people; + first | substr | length +-------+----------------------+--------- + Jack | Blow | 4 + Jack | abcdefghijklabcdefgh | 1200000 +(2 rows) + -- Test row comparison semantics. Prior to PG 8.2 we did this in a totally -- non-spec-compliant way. select ROW(1,2) < ROW(1,3) as true; diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 984a6c5f79..efd5610785 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -76,6 +76,11 @@ insert into people select ('Jim', f1, null)::fullname, current_date from pp; select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people; +-- try an update on a toasted composite value, too +update people set fn.first = 'Jack'; + +select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people; + -- Test row comparison semantics. Prior to PG 8.2 we did this in a totally -- non-spec-compliant way.