Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcc8cca3

Browse files
committed
Fix order of operations in ExecEvalFieldStoreDeForm().
If the given composite datum is toasted out-of-line,DatumGetHeapTupleHeader will perform database accesses to detoast it.That can invalidate the result of get_cached_rowtype, as documented(perhaps not plainly enough) in that function's API spec; which leadsto strange errors or crashes when we try to use the TupleDesc to readthe tuple. In short then, trying to update a field of a compositecolumn could fail intermittently if the overall column value is wideenough to require toasting.We can fix the bug at no cost by just changing the order ofoperations, since we don't need the TupleDesc until after detoasting.(Other callers of get_cached_rowtype appear to get this right already,so there's only one bug.)Note that the added regression test case reveals this bug reliablyonly with debug_discard_caches/CLOBBER_CACHE_ALWAYS.Per bug #17994 from Alexander Lakhin. Sadly, this patch does not fixthe missing-values issue revealed in the bug discussion; we'll needsome more work to cover that.Discussion:https://postgr.es/m/17994-5c7100b51b4790e9@postgresql.org
1 parentdf5dcf4 commitcc8cca3

File tree

3 files changed

+31
-12
lines changed

3 files changed

+31
-12
lines changed

‎src/backend/executor/execExprInterp.c

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,7 +1971,8 @@ CheckOpSlotCompatibility(ExprEvalStep *op, TupleTableSlot *slot)
19711971
* changed: if not NULL, *changed is set to true on any update
19721972
*
19731973
* The returned TupleDesc is not guaranteed pinned; caller must pin it
1974-
* to use it across any operation that might incur cache invalidation.
1974+
* to use it across any operation that might incur cache invalidation,
1975+
* including for example detoasting of input tuples.
19751976
* (The TupleDesc is always refcounted, so just use IncrTupleDescRefCount.)
19761977
*
19771978
* NOTE: because composite types can change contents, we must be prepared
@@ -3130,17 +3131,6 @@ ExecEvalFieldSelect(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
31303131
void
31313132
ExecEvalFieldStoreDeForm(ExprState*state,ExprEvalStep*op,ExprContext*econtext)
31323133
{
3133-
TupleDesctupDesc;
3134-
3135-
/* Lookup tupdesc if first time through or if type changes */
3136-
tupDesc=get_cached_rowtype(op->d.fieldstore.fstore->resulttype,-1,
3137-
op->d.fieldstore.rowcache,NULL);
3138-
3139-
/* Check that current tupdesc doesn't have more fields than we allocated */
3140-
if (unlikely(tupDesc->natts>op->d.fieldstore.ncolumns))
3141-
elog(ERROR,"too many columns in composite type %u",
3142-
op->d.fieldstore.fstore->resulttype);
3143-
31443134
if (*op->resnull)
31453135
{
31463136
/* Convert null input tuple into an all-nulls row */
@@ -3156,13 +3146,28 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte
31563146
DatumtupDatum=*op->resvalue;
31573147
HeapTupleHeadertuphdr;
31583148
HeapTupleDatatmptup;
3149+
TupleDesctupDesc;
31593150

31603151
tuphdr=DatumGetHeapTupleHeader(tupDatum);
31613152
tmptup.t_len=HeapTupleHeaderGetDatumLength(tuphdr);
31623153
ItemPointerSetInvalid(&(tmptup.t_self));
31633154
tmptup.t_tableOid=InvalidOid;
31643155
tmptup.t_data=tuphdr;
31653156

3157+
/*
3158+
* Lookup tupdesc if first time through or if type changes. Because
3159+
* we don't pin the tupdesc, we must not do this lookup until after
3160+
* doing DatumGetHeapTupleHeader: that could do database access while
3161+
* detoasting the datum.
3162+
*/
3163+
tupDesc=get_cached_rowtype(op->d.fieldstore.fstore->resulttype,-1,
3164+
op->d.fieldstore.rowcache,NULL);
3165+
3166+
/* Check that current tupdesc doesn't have more fields than allocated */
3167+
if (unlikely(tupDesc->natts>op->d.fieldstore.ncolumns))
3168+
elog(ERROR,"too many columns in composite type %u",
3169+
op->d.fieldstore.fstore->resulttype);
3170+
31663171
heap_deform_tuple(&tmptup,tupDesc,
31673172
op->d.fieldstore.values,
31683173
op->d.fieldstore.nulls);

‎src/test/regress/expected/rowtypes.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,15 @@ select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
145145
Jim | abcdefghijklabcdefgh | 1200000
146146
(2 rows)
147147

148+
-- try an update on a toasted composite value, too
149+
update people set fn.first = 'Jack';
150+
select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
151+
first | substr | length
152+
-------+----------------------+---------
153+
Jack | Blow | 4
154+
Jack | abcdefghijklabcdefgh | 1200000
155+
(2 rows)
156+
148157
-- Test row comparison semantics. Prior to PG 8.2 we did this in a totally
149158
-- non-spec-compliant way.
150159
select ROW(1,2) < ROW(1,3) as true;

‎src/test/regress/sql/rowtypes.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ insert into people select ('Jim', f1, null)::fullname, current_date from pp;
8080

8181
select (fn).first, substr((fn).last,1,20), length((fn).last)from people;
8282

83+
-- try an update on a toasted composite value, too
84+
update peoplesetfn.first='Jack';
85+
86+
select (fn).first, substr((fn).last,1,20), length((fn).last)from people;
87+
8388
-- Test row comparison semantics. Prior to PG 8.2 we did this in a totally
8489
-- non-spec-compliant way.
8590

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp