8000 Use the typcache to cache constraints for domain types. · postgrespro/postgres@8abb3cd · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 8abb3cd

    Browse files
    tglsfdctglsfdc
    committed
    Use the typcache to cache constraints for domain types.
    Previously, we cached domain constraints for the life of a query, or really for the life of the FmgrInfo struct that was used to invoke domain_in() or domain_check(). But plpgsql (and probably other places) are set up to cache such FmgrInfos for the whole lifespan of a session, which meant they could be enforcing really stale sets of constraints. On the other hand, searching pg_constraint once per query gets kind of expensive too: testing says that as much as half the runtime of a trivial query such as "SELECT 0::domaintype" went into that. To fix this, delegate the responsibility for tracking a domain's constraints to the typcache, which has the infrastructure needed to detect syscache invalidation events that signal possible changes. This not only removes unnecessary repeat reads of pg_constraint, but ensures that we never apply stale constraint data: whatever we use is the current data according to syscache rules. Unfortunately, the current configuration of the system catalogs means we have to flush cached domain-constraint data whenever either pg_type or pg_constraint changes, which happens rather a lot (eg, creation or deletion of a temp table will do it). It might be worth rearranging things to split pg_constraint into two catalogs, of which the domain constraint one would probably be very low-traffic. That's a job for another patch though, and in any case this patch should improve matters materially even with that handicap. This patch makes use of the recently-added memory context reset callback feature to manage the lifespan of domain constraint caches, so that we don't risk deleting a cache that might be in the midst of evaluation. Although this is a bug fix as well as a performance improvement, no back-patch. There haven't been many if any field complaints about stale domain constraint checks, so it doesn't seem worth taking the risk of modifying data structures as basic as MemoryContexts in back branches.
    1 parent b8a18ad commit 8abb3cd

    File tree

    10 files changed

    +547
    -181
    lines changed

    10 files547

    -181
    lines changed

    ‎src/backend/commands/tablecmds.c

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -4824,7 +4824,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
    48244824
    {
    48254825
    defval = (Expr *) build_column_default(rel, attribute.attnum);
    48264826

    4827-
    if (!defval && GetDomainConstraints(typeOid) != NIL)
    4827+
    if (!defval && DomainHasConstraints(typeOid))
    48284828
    {
    48294829
    Oid baseTypeId;
    48304830
    int32 baseTypeMod;
    @@ -7778,7 +7778,7 @@ ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno)
    77787778
    {
    77797779
    CoerceToDomain *d = (CoerceToDomain *) expr;
    77807780

    7781-
    if (GetDomainConstraints(d->resulttype) != NIL)
    7781+
    if (DomainHasConstraints(d->resulttype))
    77827782
    return true;
    77837783
    expr = (Node *) d->arg;
    77847784
    }

    ‎src/backend/commands/typecmds.c

    Lines changed: 0 additions & 127 deletions
    Original file line numberDiff line numberDiff line change
    @@ -31,15 +31,11 @@
    3131
    */
    3232
    #include "postgres.h"
    3333

    34-
    #include "access/genam.h"
    35-
    #include "access/heapam.h"
    3634
    #include "access/htup_details.h"
    3735
    #include "access/xact.h"
    3836
    #include "catalog/binary_upgrade.h"
    3937
    #include "catalog/catalog.h"
    40-
    #include "catalog/dependency.h"
    4138
    #include "catalog/heap.h"
    42-
    #include "catalog/indexing.h"
    4339
    #include "catalog/objectaccess.h"
    4440
    #include "catalog/pg_authid.h"
    4541
    #include "catalog/pg_collation.h"
    @@ -59,14 +55,12 @@
    5955
    #include "executor/executor.h"
    6056
    #include "miscadmin.h"
    6157
    #include "nodes/makefuncs.h"
    62-
    #include "optimizer/planner.h"
    6358
    #include "optimizer/var.h"
    6459
    #include "parser/parse_coerce.h"
    6560
    #include "parser/parse_collate.h"
    6661
    #include "parser/parse_expr.h"
    6762
    #include "parser/parse_func.h"
    6863
    #include "parser/parse_type.h"
    69-
    #include "utils/acl.h"
    7064
    #include "utils/builtins.h"
    7165
    #include "utils/fmgroids.h"
    7266
    #include "utils/lsyscache.h"
    @@ -75,7 +69,6 @@
    7569
    #include "utils/ruleutils.h"
    7670
    #include "utils/snapmgr.h"
    7771
    #include "utils/syscache.h"
    78-
    #include "utils/tqual.h"
    7972

    8073

    8174
    /* result structure for get_rels_with_domain() */
    @@ -3081,126 +3074,6 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
    30813074
    return ccbin;
    30823075
    }
    30833076

    3084-
    /*
    3085-
    * GetDomainConstraints - get a list of the current constraints of domain
    3086-
    *
    3087-
    * Returns a possibly-empty list of DomainConstraintState nodes.
    3088-
    *
    3089-
    * This is called by the executor during plan startup for a CoerceToDomain
    3090-
    * expression node. The given constraints will be checked for each value
    3091-
    * passed through the node.
    3092-
    *
    3093-
    * We allow this to be called for non-domain types, in which case the result
    3094-
    * is always NIL.
    3095-
    */
    3096-
    List *
    3097-
    GetDomainConstraints(Oid typeOid)
    3098-
    {
    3099-
    List *result = NIL;
    3100-
    bool notNull = false;
    3101-
    Relation conRel;
    3102-
    3103-
    conRel = heap_open(ConstraintRelationId, AccessShareLock);
    3104-
    3105-
    for (;;)
    3106-
    {
    3107-
    HeapTuple tup;
    3108-
    HeapTuple conTup;
    3109-
    Form_pg_type typTup;
    3110-
    ScanKeyData key[1];
    3111-
    SysScanDesc scan;
    3112-
    3113-
    tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid));
    3114-
    if (!HeapTupleIsValid(tup))
    3115-
    elog(ERROR, "cache lookup failed for type %u", typeOid);
    3116-
    typTup = (Form_pg_type) GETSTRUCT(tup);
    3117-
    3118-
    if (typTup->typtype != TYPTYPE_DOMAIN)
    3119-
    {
    3120-
    /* Not a domain, so done */
    3121-
    ReleaseSysCache(tup);
    3122-
    break;
    3123-
    }
    3124-
    3125-
    /* Test for NOT NULL Constraint */
    3126-
    if (typTup->typnotnull)
    3127-
    notNull = true;
    3128-
    3129-
    /* Look for CHECK Constraints on this domain */
    3130-
    ScanKeyInit(&key[0],
    3131-
    Anum_pg_constraint_contypid,
    3132-
    BTEqualStrategyNumber, F_OIDEQ,
    3133-
    ObjectIdGetDatum(typeOid));
    3134-
    3135-
    scan = systable_beginscan(conRel, ConstraintTypidIndexId, true,
    3136-
    NULL, 1, key);
    3137-
    3138-
    while (HeapTupleIsValid(conTup = systable_getnext(scan)))
    3139-
    {
    3140-
    Form_pg_constraint c = (Form_pg_constraint) GETSTRUCT(conTup);
    3141-
    Datum val;
    3142-
    bool isNull;
    3143-
    Expr *check_expr;
    3144-
    DomainConstraintState *r;
    3145-
    3146-
    /* Ignore non-CHECK constraints (presently, shouldn't be any) */
    3147-
    if (c->contype != CONSTRAINT_CHECK)
    3148-
    continue;
    3149-
    3150-
    /*
    3151-
    * Not expecting conbin to be NULL, but we'll test for it anyway
    3152-
    */
    3153-
    val = fastgetattr(conTup, Anum_pg_constraint_conbin,
    3154-
    conRel->rd_att, &isNull);
    3155-
    if (isNull)
    3156-
    elog(ERROR, "domain \"%s\" constraint \"%s\" has NULL conbin",
    3157-
    NameStr(typTup->typname), NameStr(c->conname));
    3158-
    3159-
    check_expr = (Expr *) stringToNode(TextDatumGetCString(val));
    3160-
    3161-
    /* ExecInitExpr assumes we've planned the expression */
    3162-
    check_expr = expression_planner(check_expr);
    3163-
    3164-
    r = makeNode(DomainConstraintState);
    3165-
    r->constrainttype = DOM_CONSTRAINT_CHECK;
    3166-
    r->name = pstrdup(NameStr(c->conname));
    3167-
    r->check_expr = ExecInitExpr(check_expr, NULL);
    3168-
    3169-
    /*
    3170-
    * use lcons() here because constraints of lower domains should be
    3171-
    * applied earlier.
    3172-
    */
    3173-
    result = lcons(r, result);
    3174-
    }
    3175-
    3176-
    systable_endscan(scan);
    3177-
    3178-
    /* loop to next domain in stack */
    3179-
    typeOid = typTup->typbasetype;
    3180-
    ReleaseSysCache(tup);
    3181-
    }
    3182-
    3183-
    heap_close(conRel, AccessShareLock);
    3184-
    3185-
    /*
    3186-
    * Only need to add one NOT NULL check regardless of how many domains in
    3187-
    * the stack request it.
    3188-
    */
    3189-
    if (notNull)
    3190-
    {
    3191-
    DomainConstraintState *r = makeNode(DomainConstraintState);
    3192-
    3193-
    r->constrainttype = DOM_CONSTRAINT_NOTNULL;
    3194-
    r->name = pstrdup("NOT NULL");
    3195-
    r->check_expr = NULL;
    3196-
    3197-
    /* lcons to apply the nullness check FIRST */
    3198-
    result = lcons(r, result);
    3199-
    }
    3200-
    3201-
    return result;
    3202-
    }
    3203-
    32043077

    32053078
    /*
    32063079
    * Execute ALTER TYPE RENAME

    ‎src/backend/executor/execQual.c

    Lines changed: 10 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -41,7 +41,6 @@
    4141
    #include "access/tupconvert.h"
    4242
    #include "catalog/objectaccess.h"
    4343
    #include "catalog/pg_type.h"
    44-
    #include "commands/typecmds.h"
    4544
    #include "executor/execdebug.h"
    4645
    #include "executor/nodeSubplan.h"
    4746
    #include "funcapi.h"
    @@ -3929,7 +3928,10 @@ ExecEvalCoerceToDomain(CoerceToDomainState *cstate, ExprContext *econtext,
    39293928
    if (isDone && *isDone == ExprEndResult)
    39303929
    return result; /* nothing to check */
    39313930

    3932-
    foreach(l, cstate->constraints)
    3931+
    /* Make sure we have up-to-date constraints */
    3932+
    UpdateDomainConstraintRef(cstate->constraint_ref);
    3933+
    3934+
    foreach(l, cstate->constraint_ref->constraints)
    39333935
    {
    39343936
    DomainConstraintState *con = (DomainConstraintState *) lfirst(l);
    39353937

    @@ -5050,7 +5052,12 @@ ExecInitExpr(Expr *node, PlanState *parent)
    50505052

    50515053
    cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalCoerceToDomain;
    50525054
    cstate->arg = ExecInitExpr(ctest->arg, parent);
    5053-
    cstate->constraints = GetDomainConstraints(ctest->resulttype);
    5055+
    /* We spend an extra palloc to reduce header inclusions */
    5056+
    cstate->constraint_ref = (DomainConstraintRef *)
    5057+
    palloc(sizeof(DomainConstraintRef));
    5058+
    InitDomainConstraintRef(ctest->resulttype,
    5059+
    cstate->constraint_ref,
    5060+
    CurrentMemoryContext);
    50545061
    state = (ExprState *) cstate;
    50555062
    }
    50565063
    break;

    ‎src/backend/utils/adt/domains.c

    Lines changed: 36 additions & 42 deletions
    Original file line numberDiff line numberDiff line change
    @@ -12,10 +12,9 @@
    1212
    * The overhead required for constraint checking can be high, since examining
    1313
    * the catalogs to discover the constraints for a given domain is not cheap.
    1414
    * We have three mechanisms for minimizing this cost:
    15-
    * 1. In a nest of domains, we flatten the checking of all the levels
    16-
    * into just one operation.
    17-
    * 2. We cache the list of constraint items in the FmgrInfo struct
    18-
    * passed by the caller.
    15+
    * 1. We rely on the typcache to keep up-to-date copies of the constraints.
    16+
    * 2. In a nest of domains, we flatten the checking of all the levels
    17+
    * into just one operation (the typcache does this for us).
    1918
    * 3. If there are CHECK constraints, we cache a standalone ExprContext
    2019
    * to evaluate them in.
    2120
    *
    @@ -33,12 +32,12 @@
    3332

    3433
    #include "access/htup_details.h"
    3534
    #include "catalog/pg_type.h"
    36-
    #include "commands/typecmds.h"
    3735
    #include "executor/executor.h"
    3836
    #include "lib/stringinfo.h"
    3937
    #include "utils/builtins.h"
    4038
    #include "utils/lsyscache.h"
    4139
    #include "utils/syscache.h"
    40+
    #include "utils/typcache.h"
    4241

    4342

    4443
    /*
    @@ -52,8 +51,8 @@ typedef struct DomainIOData
    5251
    Oid typioparam;
    5352
    int32 typtypmod;
    5453
    FmgrInfo proc;
    55-
    /* List of constraint items to check */
    56-
    List *constraint_list;
    54+
    /* Reference to cached list of constraint items to check */
    55+
    DomainConstraintRef constraint_ref;
    5756
    /* Context for evaluating CHECK constraints in */
    5857
    ExprContext *econtext;
    5958
    /* Memory context this cache is in */
    @@ -63,16 +62,19 @@ typedef struct DomainIOData
    6362

    6463
    /*
    6564
    * domain_state_setup - initialize the cache for a new domain type.
    65+
    *
    66+
    * Note: we can't re-use the same cache struct for a new domain type,
    67+
    * since there's no provision for releasing the DomainConstraintRef.
    68+
    * If a call site needs to deal with a new domain type, we just leak
    69+
    * the old struct for the duration of the query.
    6670
    */
    67-
    static void
    68-
    domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary,
    69-
    MemoryContext mcxt)
    71+
    static DomainIOData *
    72+
    domain_state_setup(Oid domainType, bool binary, MemoryContext mcxt)
    7073
    {
    74+
    DomainIOData *my_extra;
    7175
    Oid baseType;
    72-
    MemoryContext oldcontext;
    7376

    74-
    /* Mark cache invalid */
    75-
    my_extra->domain_type = InvalidOid;
    77+
    my_extra = (DomainIOData *) MemoryContextAlloc(mcxt, sizeof(DomainIOData));
    7678

    7779
    /* Find out the base type */
    7880
    my_extra->typtypmod = -1;
    @@ -95,16 +97,16 @@ domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary,
    9597
    fmgr_info_cxt(my_extra->typiofunc, &my_extra->proc, mcxt);
    9698

    9799
    /* Look up constraints for domain */
    98-
    oldcontext = MemoryContextSwitchTo(mcxt);
    99-
    my_extra->constraint_list = GetDomainConstraints(domainType);
    100-
    MemoryContextSwitchTo(oldcontext);
    100+
    InitDomainConstraintRef(domainType, &my_extra->constraint_ref, mcxt);
    101101

    102102
    /* We don't make an ExprContext until needed */
    103103
    my_extra->econtext = NULL;
    104104
    my_extra->mcxt = mcxt;
    105105

    106106
    /* Mark cache valid */
    107107
    my_extra->domain_type = domainType;
    108+
    109+
    return my_extra;
    108110
    }
    109111

    110112
    /*
    @@ -118,7 +120,10 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra)
    118120
    ExprContext *econtext = my_extra->econtext;
    119121
    ListCell *l;
    120122

    121-
    foreach(l, my_extra->constraint_list)
    123+
    /* Make sure we have up-to-date constraints */
    124+
    UpdateDomainConstraintRef(&my_extra->constraint_ref);
    125+
    126+
    foreach(l, my_extra->constraint_ref.constraints)
    122127
    {
    123128
    DomainConstraintState *con = (DomainConstraintState *) lfirst(l);
    124129

    @@ -215,20 +220,16 @@ domain_in(PG_FUNCTION_ARGS)
    215220

    216221
    /*
    217222
    * We arrange to look up the needed info just once per series of calls,
    218-
    * assuming the domain type doesn't change underneath us.
    223+
    * assuming the domain type doesn't change underneath us (which really
    224+
    * shouldn't happen, but cope if it does).
    219225
    */
    220226
    my_extra = (DomainIOData *) fcinfo->flinfo->fn_extra;
    221-
    if (my_extra == NULL)
    227+
    if (my_extra == NULL || my_extra->domain_type != domainType)
    222228
    {
    223-
    my_extra = (DomainIOData *) MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
    224-
    sizeof(DomainIOData));
    225-
    domain_state_setup(my_extra, domainType, false,
    226-
    fcinfo->flinfo->fn_mcxt);
    229+
    my_extra = domain_state_setup(domainType, false,
    230+
    fcinfo->flinfo->fn_mcxt);
    227231
    fcinfo->flinfo->fn_extra = (void *) my_extra;
    228232
    }
    229-
    else if (my_extra->domain_type != domainType)
    230-
    domain_state_setup(my_extra, domainType, false,
    231-
    fcinfo->flinfo->fn_mcxt);
    232233

    233234
    /*
    234235
    * Invoke the base type's typinput procedure to convert the data.
    @@ -275,20 +276,16 @@ domain_recv(PG_FUNCTION_ARGS)
    275276

    276277
    /*
    277278
    * We arrange to look up the needed info just once per series of calls,
    278-
    * assuming the domain type doesn't change underneath us.
    279+
    * assuming the domain type doesn't change underneath us (which really
    280+
    * shouldn't happen, but cope if it does).
    279281
    */
    280282
    my_extra = (DomainIOData *) fcinfo->flinfo->fn_extra;
    281-
    if (my_extra == NULL)
    283+
    if (my_extra == NULL || my_extra->domain_type != domainType)
    282284
    {
    283-
    my_extra = (DomainIOData *) MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
    284-
    sizeof(DomainIOData));
    285-
    domain_state_setup(my_extra, domainType, true,
    286-
    fcinfo->flinfo->fn_mcxt);
    285+
    my_extra = domain_state_setup(domainType, true,
    286+
    fcinfo->flinfo->fn_mcxt);
    287287
    fcinfo->flinfo->fn_extra = (void *) my_extra;
    288288
    }
    289-
    else if (my_extra->domain_type != domainType)
    290-
    domain_state_setup(my_extra, domainType, true,
    291-
    fcinfo->flinfo->fn_mcxt);
    292289

    293290
    /*
    294291
    * Invoke the base type's typreceive procedure to convert the data.
    @@ -326,20 +323,17 @@ domain_check(Datum value, bool isnull, Oid domainType,
    326323

    327324
    /*
    328325
    * We arrange to look up the needed info just once per series of calls,
    329-
    * assuming the domain type doesn't change underneath us.
    326+
    * assuming the domain type doesn't change underneath us (which really
    327+
    * shouldn't happen, but cope if it does).
    330328
    */
    331329
    if (extra)
    332330
    my_extra = (DomainIOData *) *extra;
    333-
    if (my_extra == NULL)
    331+
    if (my_extra == NULL || my_extra->domain_type != domainType)
    334332
    {
    335-
    my_extra = (DomainIOData *) MemoryContextAlloc(mcxt,
    336-
    sizeof(DomainIOData));
    337-
    domain_state_setup(my_extra, domainType, true, mcxt);
    333+
    my_extra = domain_state_setup(domainType, true, mcxt);
    338334
    if (extra)
    339335
    *extra = (void *) my_extra;
    340336
    }
    341-
    else if (my_extra->domain_type != domainType)
    342-
    domain_state_setup(my_extra, domainType, true, mcxt);
    343337

    344338
    /*
    345339
    * Do the necessary checks to ensure it's a valid domain value.

    0 commit comments

    Comments
     (0)
    0