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

Commit97b7ad4

Browse files
committed
Redesign the caching done by get_cached_rowtype().
Previously, get_cached_rowtype() cached a pointer to a reference-countedtuple descriptor from the typcache, relying on the ExprContextCallbackmechanism to release the tupdesc refcount when the expression treeusing the tupdesc was destroyed. This worked fine when it was designed,but the introduction of within-DO-block COMMITs broke it. The refcountis logged in a transaction-lifespan resource owner, but plpgsql won'tdestroy simple expressions made within the DO block (before its firstcommit) until the DO block is exited. That results in a warning abouta leaked tupdesc refcount when the COMMIT destroys the original resourceowner, and then an error about the active resource owner not holding amatching refcount when the expression is destroyed.To fix, get rid of the need to have a shutdown callback at all, byinstead caching a pointer to the relevant typcache entry. Thosesurvive for the life of the backend, so we needn't worry about thepointer becoming stale. (For registered RECORD types, we can stillcache a pointer to the tupdesc, knowing that it won't change for thelife of the backend.) This mechanism has been in use in plpgsqland expandedrecord.c since commit4b93f57, and seems to work well.This change requires modifying the ExprEvalStep structs used by therelevant expression step types, which is slightly worrisome forback-patching. However, there seems no good reason for extensionsto be familiar with the details of these particular sub-structs.Per report from Rohit Bhogate. Back-patch to v11 where within-DO-blockCOMMITs became a thing.Discussion:https://postgr.es/m/CAAV6ZkQRCVBh8qAY+SZiHnz+U+FqAGBBDaDTjF2yiKa2nJSLKg@mail.gmail.com
1 parent37e7654 commit97b7ad4

File tree

5 files changed

+180
-94
lines changed

5 files changed

+180
-94
lines changed

‎src/backend/executor/execExpr.c

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
11321132
scratch.opcode=EEOP_FIELDSELECT;
11331133
scratch.d.fieldselect.fieldnum=fselect->fieldnum;
11341134
scratch.d.fieldselect.resulttype=fselect->resulttype;
1135-
scratch.d.fieldselect.argdesc=NULL;
1135+
scratch.d.fieldselect.rowcache.cacheptr=NULL;
11361136

11371137
ExprEvalPushStep(state,&scratch);
11381138
break;
@@ -1142,7 +1142,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
11421142
{
11431143
FieldStore*fstore= (FieldStore*)node;
11441144
TupleDesctupDesc;
1145-
TupleDesc*descp;
1145+
ExprEvalRowtypeCache*rowcachep;
11461146
Datum*values;
11471147
bool*nulls;
11481148
intncolumns;
@@ -1158,17 +1158,17 @@ ExecInitExprRec(Expr *node, ExprState *state,
11581158
values= (Datum*)palloc(sizeof(Datum)*ncolumns);
11591159
nulls= (bool*)palloc(sizeof(bool)*ncolumns);
11601160

1161-
/* createworkspace for runtime tupdesccache */
1162-
descp=(TupleDesc*)palloc(sizeof(TupleDesc));
1163-
*descp=NULL;
1161+
/* createshared composite-type-lookupcache struct */
1162+
rowcachep=palloc(sizeof(ExprEvalRowtypeCache));
1163+
rowcachep->cacheptr=NULL;
11641164

11651165
/* emit code to evaluate the composite input value */
11661166
ExecInitExprRec(fstore->arg,state,resv,resnull);
11671167

11681168
/* next, deform the input tuple into our workspace */
11691169
scratch.opcode=EEOP_FIELDSTORE_DEFORM;
11701170
scratch.d.fieldstore.fstore=fstore;
1171-
scratch.d.fieldstore.argdesc=descp;
1171+
scratch.d.fieldstore.rowcache=rowcachep;
11721172
scratch.d.fieldstore.values=values;
11731173
scratch.d.fieldstore.nulls=nulls;
11741174
scratch.d.fieldstore.ncolumns=ncolumns;
@@ -1226,7 +1226,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
12261226
/* finally, form result tuple */
12271227
scratch.opcode=EEOP_FIELDSTORE_FORM;
12281228
scratch.d.fieldstore.fstore=fstore;
1229-
scratch.d.fieldstore.argdesc=descp;
1229+
scratch.d.fieldstore.rowcache=rowcachep;
12301230
scratch.d.fieldstore.values=values;
12311231
scratch.d.fieldstore.nulls=nulls;
12321232
scratch.d.fieldstore.ncolumns=ncolumns;
@@ -1372,17 +1372,24 @@ ExecInitExprRec(Expr *node, ExprState *state,
13721372
caseT_ConvertRowtypeExpr:
13731373
{
13741374
ConvertRowtypeExpr*convert= (ConvertRowtypeExpr*)node;
1375+
ExprEvalRowtypeCache*rowcachep;
1376+
1377+
/* cache structs must be out-of-line for space reasons */
1378+
rowcachep=palloc(2*sizeof(ExprEvalRowtypeCache));
1379+
rowcachep[0].cacheptr=NULL;
1380+
rowcachep[1].cacheptr=NULL;
13751381

13761382
/* evaluate argument into step's result area */
13771383
ExecInitExprRec(convert->arg,state,resv,resnull);
13781384

13791385
/* and push conversion step */
13801386
scratch.opcode=EEOP_CONVERT_ROWTYPE;
1381-
scratch.d.convert_rowtype.convert=convert;
1382-
scratch.d.convert_rowtype.indesc=NULL;
1383-
scratch.d.convert_rowtype.outdesc=NULL;
1387+
scratch.d.convert_rowtype.inputtype=
1388+
exprType((Node*)convert->arg);
1389+
scratch.d.convert_rowtype.outputtype=convert->resulttype;
1390+
scratch.d.convert_rowtype.incache=&rowcachep[0];
1391+
scratch.d.convert_rowtype.outcache=&rowcachep[1];
13841392
scratch.d.convert_rowtype.map=NULL;
1385-
scratch.d.convert_rowtype.initialized= false;
13861393

13871394
ExprEvalPushStep(state,&scratch);
13881395
break;
@@ -2007,7 +2014,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
20072014
(int)ntest->nulltesttype);
20082015
}
20092016
/* initialize cache in case it's a row test */
2010-
scratch.d.nulltest_row.argdesc=NULL;
2017+
scratch.d.nulltest_row.rowcache.cacheptr=NULL;
20112018

20122019
/* first evaluate argument into result variable */
20132020
ExecInitExprRec(ntest->arg,state,

‎src/backend/executor/execExprInterp.c

Lines changed: 92 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ static void ExecInitInterpreter(void);
145145
staticvoidCheckVarSlotCompatibility(TupleTableSlot*slot,intattnum,Oidvartype);
146146
staticvoidCheckOpSlotCompatibility(ExprEvalStep*op,TupleTableSlot*slot);
147147
staticTupleDescget_cached_rowtype(Oidtype_id,int32typmod,
148-
TupleDesc*cache_field,ExprContext*econtext);
149-
staticvoidShutdownTupleDescRef(Datumarg);
148+
ExprEvalRowtypeCache*rowcache,
149+
bool*changed);
150150
staticvoidExecEvalRowNullInt(ExprState*state,ExprEvalStep*op,
151151
ExprContext*econtext,boolcheckisnull);
152152

@@ -1942,56 +1942,78 @@ CheckOpSlotCompatibility(ExprEvalStep *op, TupleTableSlot *slot)
19421942
* get_cached_rowtype: utility function to lookup a rowtype tupdesc
19431943
*
19441944
* type_id, typmod: identity of the rowtype
1945-
*cache_field: where to cache the TupleDesc pointer in expression state node
1946-
*(field must be initialized to NULL)
1947-
*econtext: expression context we are executing in
1945+
*rowcache: space for caching identity info
1946+
*(rowcache->cacheptr must be initialized to NULL)
1947+
*changed: if not NULL, *changed is set to true on any update
19481948
*
1949-
* NOTE: because the shutdown callback will be called during plan rescan,
1950-
* must be prepared to re-do this during any node execution; cannot call
1951-
* just once during expression initialization.
1949+
* The returned TupleDesc is not guaranteed pinned; caller must pin it
1950+
* to use it across any operation that might incur cache invalidation.
1951+
* (The TupleDesc is always refcounted, so just use IncrTupleDescRefCount.)
1952+
*
1953+
* NOTE: because composite types can change contents, we must be prepared
1954+
* to re-do this during any node execution; cannot call just once during
1955+
* expression initialization.
19521956
*/
19531957
staticTupleDesc
19541958
get_cached_rowtype(Oidtype_id,int32typmod,
1955-
TupleDesc*cache_field,ExprContext*econtext)
1959+
ExprEvalRowtypeCache*rowcache,
1960+
bool*changed)
19561961
{
1957-
TupleDesctupDesc=*cache_field;
1958-
1959-
/* Do lookup if no cached value or if requested type changed */
1960-
if (tupDesc==NULL||
1961-
type_id!=tupDesc->tdtypeid||
1962-
typmod!=tupDesc->tdtypmod)
1962+
if (type_id!=RECORDOID)
19631963
{
1964-
tupDesc=lookup_rowtype_tupdesc(type_id,typmod);
1964+
/*
1965+
* It's a named composite type, so use the regular typcache. Do a
1966+
* lookup first time through, or if the composite type changed. Note:
1967+
* "tupdesc_id == 0" may look redundant, but it protects against the
1968+
* admittedly-theoretical possibility that type_id was RECORDOID the
1969+
* last time through, so that the cacheptr isn't TypeCacheEntry *.
1970+
*/
1971+
TypeCacheEntry*typentry= (TypeCacheEntry*)rowcache->cacheptr;
19651972

1966-
if (*cache_field)
1973+
if (unlikely(typentry==NULL||
1974+
rowcache->tupdesc_id==0||
1975+
typentry->tupDesc_identifier!=rowcache->tupdesc_id))
19671976
{
1968-
/* Release old tupdesc; but callback is already registered */
1969-
ReleaseTupleDesc(*cache_field);
1970-
}
1971-
else
1972-
{
1973-
/* Need to register shutdown callback to release tupdesc */
1974-
RegisterExprContextCallback(econtext,
1975-
ShutdownTupleDescRef,
1976-
PointerGetDatum(cache_field));
1977-
}
1978-
*cache_field=tupDesc;
1977+
typentry=lookup_type_cache(type_id,TYPECACHE_TUPDESC);
1978+
if (typentry->tupDesc==NULL)
1979+
ereport(ERROR,
1980+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
1981+
errmsg("type %s is not composite",
1982+
format_type_be(type_id))));
1983+
rowcache->cacheptr= (void*)typentry;
1984+
rowcache->tupdesc_id=typentry->tupDesc_identifier;
1985+
if (changed)
1986+
*changed= true;
1987+
}
1988+
returntypentry->tupDesc;
1989+
}
1990+
else
1991+
{
1992+
/*
1993+
* A RECORD type, once registered, doesn't change for the life of the
1994+
* backend. So we don't need a typcache entry as such, which is good
1995+
* because there isn't one. It's possible that the caller is asking
1996+
* about a different type than before, though.
1997+
*/
1998+
TupleDesctupDesc= (TupleDesc)rowcache->cacheptr;
1999+
2000+
if (unlikely(tupDesc==NULL||
2001+
rowcache->tupdesc_id!=0||
2002+
type_id!=tupDesc->tdtypeid||
2003+
typmod!=tupDesc->tdtypmod))
2004+
{
2005+
tupDesc=lookup_rowtype_tupdesc(type_id,typmod);
2006+
/* Drop pin acquired by lookup_rowtype_tupdesc */
2007+
ReleaseTupleDesc(tupDesc);
2008+
rowcache->cacheptr= (void*)tupDesc;
2009+
rowcache->tupdesc_id=0;/* not a valid value for non-RECORD */
2010+
if (changed)
2011+
*changed= true;
2012+
}
2013+
returntupDesc;
19792014
}
1980-
returntupDesc;
19812015
}
19822016

1983-
/*
1984-
* Callback function to release a tupdesc refcount at econtext shutdown
1985-
*/
1986-
staticvoid
1987-
ShutdownTupleDescRef(Datumarg)
1988-
{
1989-
TupleDesc*cache_field= (TupleDesc*)DatumGetPointer(arg);
1990-
1991-
if (*cache_field)
1992-
ReleaseTupleDesc(*cache_field);
1993-
*cache_field=NULL;
1994-
}
19952017

19962018
/*
19972019
* Fast-path functions, for very simple expressions
@@ -2583,8 +2605,7 @@ ExecEvalRowNullInt(ExprState *state, ExprEvalStep *op,
25832605

25842606
/* Lookup tupdesc if first time through or if type changes */
25852607
tupDesc=get_cached_rowtype(tupType,tupTypmod,
2586-
&op->d.nulltest_row.argdesc,
2587-
econtext);
2608+
&op->d.nulltest_row.rowcache,NULL);
25882609

25892610
/*
25902611
* heap_attisnull needs a HeapTuple not a bare HeapTupleHeader.
@@ -3017,8 +3038,7 @@ ExecEvalFieldSelect(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
30173038

30183039
/* Lookup tupdesc if first time through or if type changes */
30193040
tupDesc=get_cached_rowtype(tupType,tupTypmod,
3020-
&op->d.fieldselect.argdesc,
3021-
econtext);
3041+
&op->d.fieldselect.rowcache,NULL);
30223042

30233043
/*
30243044
* Find field's attr record. Note we don't support system columns
@@ -3076,9 +3096,9 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte
30763096
{
30773097
TupleDesctupDesc;
30783098

3079-
/* Lookup tupdesc if first time through orafter rescan */
3099+
/* Lookup tupdesc if first time through orif type changes */
30803100
tupDesc=get_cached_rowtype(op->d.fieldstore.fstore->resulttype,-1,
3081-
op->d.fieldstore.argdesc,econtext);
3101+
op->d.fieldstore.rowcache,NULL);
30823102

30833103
/* Check that current tupdesc doesn't have more fields than we allocated */
30843104
if (unlikely(tupDesc->natts>op->d.fieldstore.ncolumns))
@@ -3120,10 +3140,14 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte
31203140
void
31213141
ExecEvalFieldStoreForm(ExprState*state,ExprEvalStep*op,ExprContext*econtext)
31223142
{
3143+
TupleDesctupDesc;
31233144
HeapTupletuple;
31243145

3125-
/* argdesc should already be valid from the DeForm step */
3126-
tuple=heap_form_tuple(*op->d.fieldstore.argdesc,
3146+
/* Lookup tupdesc (should be valid already) */
3147+
tupDesc=get_cached_rowtype(op->d.fieldstore.fstore->resulttype,-1,
3148+
op->d.fieldstore.rowcache,NULL);
3149+
3150+
tuple=heap_form_tuple(tupDesc,
31273151
op->d.fieldstore.values,
31283152
op->d.fieldstore.nulls);
31293153

@@ -3334,13 +3358,13 @@ ExecEvalSubscriptingRefAssign(ExprState *state, ExprEvalStep *op)
33343358
void
33353359
ExecEvalConvertRowtype(ExprState*state,ExprEvalStep*op,ExprContext*econtext)
33363360
{
3337-
ConvertRowtypeExpr*convert=op->d.convert_rowtype.convert;
33383361
HeapTupleresult;
33393362
DatumtupDatum;
33403363
HeapTupleHeadertuple;
33413364
HeapTupleDatatmptup;
33423365
TupleDescindesc,
33433366
outdesc;
3367+
boolchanged= false;
33443368

33453369
/* NULL in -> NULL out */
33463370
if (*op->resnull)
@@ -3349,24 +3373,19 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
33493373
tupDatum=*op->resvalue;
33503374
tuple=DatumGetHeapTupleHeader(tupDatum);
33513375

3352-
/* Lookup tupdescs if first time through or after rescan */
3353-
if (op->d.convert_rowtype.indesc==NULL)
3354-
{
3355-
get_cached_rowtype(exprType((Node*)convert->arg),-1,
3356-
&op->d.convert_rowtype.indesc,
3357-
econtext);
3358-
op->d.convert_rowtype.initialized= false;
3359-
}
3360-
if (op->d.convert_rowtype.outdesc==NULL)
3361-
{
3362-
get_cached_rowtype(convert->resulttype,-1,
3363-
&op->d.convert_rowtype.outdesc,
3364-
econtext);
3365-
op->d.convert_rowtype.initialized= false;
3366-
}
3367-
3368-
indesc=op->d.convert_rowtype.indesc;
3369-
outdesc=op->d.convert_rowtype.outdesc;
3376+
/*
3377+
* Lookup tupdescs if first time through or if type changes. We'd better
3378+
* pin them since type conversion functions could do catalog lookups and
3379+
* hence cause cache invalidation.
3380+
*/
3381+
indesc=get_cached_rowtype(op->d.convert_rowtype.inputtype,-1,
3382+
op->d.convert_rowtype.incache,
3383+
&changed);
3384+
IncrTupleDescRefCount(indesc);
3385+
outdesc=get_cached_rowtype(op->d.convert_rowtype.outputtype,-1,
3386+
op->d.convert_rowtype.outcache,
3387+
&changed);
3388+
IncrTupleDescRefCount(outdesc);
33703389

33713390
/*
33723391
* We used to be able to assert that incoming tuples are marked with
@@ -3377,8 +3396,8 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
33773396
Assert(HeapTupleHeaderGetTypeId(tuple)==indesc->tdtypeid||
33783397
HeapTupleHeaderGetTypeId(tuple)==RECORDOID);
33793398

3380-
/* if first time through, initialize conversion map */
3381-
if (!op->d.convert_rowtype.initialized)
3399+
/* if first time through,or after change,initialize conversion map */
3400+
if (changed)
33823401
{
33833402
MemoryContextold_cxt;
33843403

@@ -3387,7 +3406,6 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
33873406

33883407
/* prepare map from old to new attribute numbers */
33893408
op->d.convert_rowtype.map=convert_tuples_by_name(indesc,outdesc);
3390-
op->d.convert_rowtype.initialized= true;
33913409

33923410
MemoryContextSwitchTo(old_cxt);
33933411
}
@@ -3417,6 +3435,9 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
34173435
*/
34183436
*op->resvalue=heap_copy_tuple_as_datum(&tmptup,outdesc);
34193437
}
3438+
3439+
DecrTupleDescRefCount(indesc);
3440+
DecrTupleDescRefCount(outdesc);
34203441
}
34213442

34223443
/*

0 commit comments

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