• Tom Lane's avatar
    Redesign the caching done by get_cached_rowtype(). · c2db458c
    Tom Lane authored
    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 4b93f579, 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
    c2db458c
execExpr.c 114 KB