mirror of
https://github.com/postgres/postgres.git
synced 2025-07-09 22:41:56 +03:00
Redesign the caching done by get_cached_rowtype().
Previously, get_cached_rowtype() cached a pointer to a reference-counted
tuple descriptor from the typcache, relying on the ExprContextCallback
mechanism to release the tupdesc refcount when the expression tree
using the tupdesc was destroyed. This worked fine when it was designed,
but the introduction of within-DO-block COMMITs broke it. The refcount
is logged in a transaction-lifespan resource owner, but plpgsql won't
destroy simple expressions made within the DO block (before its first
commit) until the DO block is exited. That results in a warning about
a leaked tupdesc refcount when the COMMIT destroys the original resource
owner, and then an error about the active resource owner not holding a
matching refcount when the expression is destroyed.
To fix, get rid of the need to have a shutdown callback at all, by
instead caching a pointer to the relevant typcache entry. Those
survive for the life of the backend, so we needn't worry about the
pointer becoming stale. (For registered RECORD types, we can still
cache a pointer to the tupdesc, knowing that it won't change for the
life of the backend.) This mechanism has been in use in plpgsql
and expandedrecord.c since commit 4b93f5799
, and seems to work well.
This change requires modifying the ExprEvalStep structs used by the
relevant expression step types, which is slightly worrisome for
back-patching. However, there seems no good reason for extensions
to be familiar with the details of these particular sub-structs.
Per report from Rohit Bhogate. Back-patch to v11 where within-DO-block
COMMITs became a thing.
Discussion: https://postgr.es/m/CAAV6ZkQRCVBh8qAY+SZiHnz+U+FqAGBBDaDTjF2yiKa2nJSLKg@mail.gmail.com
This commit is contained in:
src
backend
executor
include
executor
pl
plpgsql
src
@ -32,6 +32,20 @@ typedef void (*ExecEvalSubroutine) (ExprState *state,
|
||||
struct ExprEvalStep *op,
|
||||
ExprContext *econtext);
|
||||
|
||||
/* ExprEvalSteps that cache a composite type's tupdesc need one of these */
|
||||
/* (it fits in-line in some step types, otherwise allocate out-of-line) */
|
||||
typedef struct ExprEvalRowtypeCache
|
||||
{
|
||||
/*
|
||||
* cacheptr points to composite type's TypeCacheEntry if tupdesc_id is not
|
||||
* 0; or for an anonymous RECORD type, it points directly at the cached
|
||||
* tupdesc for the type, and tupdesc_id is 0. (We'd use separate fields
|
||||
* if space were not at a premium.) Initial state is cacheptr == NULL.
|
||||
*/
|
||||
void *cacheptr;
|
||||
uint64 tupdesc_id; /* last-seen tupdesc identifier, or 0 */
|
||||
} ExprEvalRowtypeCache;
|
||||
|
||||
/*
|
||||
* Discriminator for ExprEvalSteps.
|
||||
*
|
||||
@ -340,8 +354,8 @@ typedef struct ExprEvalStep
|
||||
/* for EEOP_NULLTEST_ROWIS[NOT]NULL */
|
||||
struct
|
||||
{
|
||||
/* cached tupdesc pointer - filled at runtime */
|
||||
TupleDesc argdesc;
|
||||
/* cached descriptor for composite type - filled at runtime */
|
||||
ExprEvalRowtypeCache rowcache;
|
||||
} nulltest_row;
|
||||
|
||||
/* for EEOP_PARAM_EXEC/EXTERN */
|
||||
@ -466,8 +480,8 @@ typedef struct ExprEvalStep
|
||||
{
|
||||
AttrNumber fieldnum; /* field number to extract */
|
||||
Oid resulttype; /* field's type */
|
||||
/* cached tupdesc pointer - filled at runtime */
|
||||
TupleDesc argdesc;
|
||||
/* cached descriptor for composite type - filled at runtime */
|
||||
ExprEvalRowtypeCache rowcache;
|
||||
} fieldselect;
|
||||
|
||||
/* for EEOP_FIELDSTORE_DEFORM / FIELDSTORE_FORM */
|
||||
@ -476,9 +490,9 @@ typedef struct ExprEvalStep
|
||||
/* original expression node */
|
||||
FieldStore *fstore;
|
||||
|
||||
/* cached tupdesc pointer - filled at runtime */
|
||||
/* note that a DEFORM and FORM pair share the same tupdesc */
|
||||
TupleDesc *argdesc;
|
||||
/* cached descriptor for composite type - filled at runtime */
|
||||
/* note that a DEFORM and FORM pair share the same cache */
|
||||
ExprEvalRowtypeCache *rowcache;
|
||||
|
||||
/* workspace for column values */
|
||||
Datum *values;
|
||||
@ -518,12 +532,12 @@ typedef struct ExprEvalStep
|
||||
/* for EEOP_CONVERT_ROWTYPE */
|
||||
struct
|
||||
{
|
||||
ConvertRowtypeExpr *convert; /* original expression */
|
||||
Oid inputtype; /* input composite type */
|
||||
Oid outputtype; /* output composite type */
|
||||
/* these three fields are filled at runtime: */
|
||||
TupleDesc indesc; /* tupdesc for input type */
|
||||
TupleDesc outdesc; /* tupdesc for output type */
|
||||
ExprEvalRowtypeCache *incache; /* cache for input type */
|
||||
ExprEvalRowtypeCache *outcache; /* cache for output type */
|
||||
TupleConversionMap *map; /* column mapping */
|
||||
bool initialized; /* initialized for current types? */
|
||||
} convert_rowtype;
|
||||
|
||||
/* for EEOP_SCALARARRAYOP */
|
||||
|
Reference in New Issue
Block a user