From 8abb3cda0ddc00a0ab98977a1633a95b97068d4e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 1 Mar 2015 14:06:50 -0500 Subject: [PATCH] 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. --- src/backend/commands/tablecmds.c | 4 +- src/backend/commands/typecmds.c | 127 --------- src/backend/executor/execQual.c | 13 +- src/backend/utils/adt/domains.c | 78 +++--- src/backend/utils/cache/typcache.c | 405 ++++++++++++++++++++++++++- src/include/commands/typecmds.h | 2 - src/include/nodes/execnodes.h | 5 +- src/include/utils/typcache.h | 37 +++ src/test/regress/expected/domain.out | 30 ++ src/test/regress/sql/domain.sql | 27 ++ 10 files changed, 547 insertions(+), 181 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 07ab4b434f..745502072e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4824,7 +4824,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, { defval = (Expr *) build_column_default(rel, attribute.attnum); - if (!defval && GetDomainConstraints(typeOid) != NIL) + if (!defval && DomainHasConstraints(typeOid)) { Oid baseTypeId; int32 baseTypeMod; @@ -7778,7 +7778,7 @@ ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno) { CoerceToDomain *d = (CoerceToDomain *) expr; - if (GetDomainConstraints(d->resulttype) != NIL) + if (DomainHasConstraints(d->resulttype)) return true; expr = (Node *) d->arg; } diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index b77e1b4140..60ab3aaf12 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -31,15 +31,11 @@ */ #include "postgres.h" -#include "access/genam.h" -#include "access/heapam.h" #include "access/htup_details.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" #include "catalog/catalog.h" -#include "catalog/dependency.h" #include "catalog/heap.h" -#include "catalog/indexing.h" #include "catalog/objectaccess.h" #include "catalog/pg_authid.h" #include "catalog/pg_collation.h" @@ -59,14 +55,12 @@ #include "executor/executor.h" #include "miscadmin.h" #include "nodes/makefuncs.h" -#include "optimizer/planner.h" #include "optimizer/var.h" #include "parser/parse_coerce.h" #include "parser/parse_collate.h" #include "parser/parse_expr.h" #include "parser/parse_func.h" #include "parser/parse_type.h" -#include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" @@ -75,7 +69,6 @@ #include "utils/ruleutils.h" #include "utils/snapmgr.h" #include "utils/syscache.h" -#include "utils/tqual.h" /* result structure for get_rels_with_domain() */ @@ -3081,126 +3074,6 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, return ccbin; } -/* - * GetDomainConstraints - get a list of the current constraints of domain - * - * Returns a possibly-empty list of DomainConstraintState nodes. - * - * This is called by the executor during plan startup for a CoerceToDomain - * expression node. The given constraints will be checked for each value - * passed through the node. - * - * We allow this to be called for non-domain types, in which case the result - * is always NIL. - */ -List * -GetDomainConstraints(Oid typeOid) -{ - List *result = NIL; - bool notNull = false; - Relation conRel; - - conRel = heap_open(ConstraintRelationId, AccessShareLock); - - for (;;) - { - HeapTuple tup; - HeapTuple conTup; - Form_pg_type typTup; - ScanKeyData key[1]; - SysScanDesc scan; - - tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid)); - if (!HeapTupleIsValid(tup)) - elog(ERROR, "cache lookup failed for type %u", typeOid); - typTup = (Form_pg_type) GETSTRUCT(tup); - - if (typTup->typtype != TYPTYPE_DOMAIN) - { - /* Not a domain, so done */ - ReleaseSysCache(tup); - break; - } - - /* Test for NOT NULL Constraint */ - if (typTup->typnotnull) - notNull = true; - - /* Look for CHECK Constraints on this domain */ - ScanKeyInit(&key[0], - Anum_pg_constraint_contypid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(typeOid)); - - scan = systable_beginscan(conRel, ConstraintTypidIndexId, true, - NULL, 1, key); - - while (HeapTupleIsValid(conTup = systable_getnext(scan))) - { - Form_pg_constraint c = (Form_pg_constraint) GETSTRUCT(conTup); - Datum val; - bool isNull; - Expr *check_expr; - DomainConstraintState *r; - - /* Ignore non-CHECK constraints (presently, shouldn't be any) */ - if (c->contype != CONSTRAINT_CHECK) - continue; - - /* - * Not expecting conbin to be NULL, but we'll test for it anyway - */ - val = fastgetattr(conTup, Anum_pg_constraint_conbin, - conRel->rd_att, &isNull); - if (isNull) - elog(ERROR, "domain \"%s\" constraint \"%s\" has NULL conbin", - NameStr(typTup->typname), NameStr(c->conname)); - - check_expr = (Expr *) stringToNode(TextDatumGetCString(val)); - - /* ExecInitExpr assumes we've planned the expression */ - check_expr = expression_planner(check_expr); - - r = makeNode(DomainConstraintState); - r->constrainttype = DOM_CONSTRAINT_CHECK; - r->name = pstrdup(NameStr(c->conname)); - r->check_expr = ExecInitExpr(check_expr, NULL); - - /* - * use lcons() here because constraints of lower domains should be - * applied earlier. - */ - result = lcons(r, result); - } - - systable_endscan(scan); - - /* loop to next domain in stack */ - typeOid = typTup->typbasetype; - ReleaseSysCache(tup); - } - - heap_close(conRel, AccessShareLock); - - /* - * Only need to add one NOT NULL check regardless of how many domains in - * the stack request it. - */ - if (notNull) - { - DomainConstraintState *r = makeNode(DomainConstraintState); - - r->constrainttype = DOM_CONSTRAINT_NOTNULL; - r->name = pstrdup("NOT NULL"); - r->check_expr = NULL; - - /* lcons to apply the nullness check FIRST */ - result = lcons(r, result); - } - - return result; -} - /* * Execute ALTER TYPE RENAME diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index fec76d4f1b..d94fe581df 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -41,7 +41,6 @@ #include "access/tupconvert.h" #include "catalog/objectaccess.h" #include "catalog/pg_type.h" -#include "commands/typecmds.h" #include "executor/execdebug.h" #include "executor/nodeSubplan.h" #include "funcapi.h" @@ -3929,7 +3928,10 @@ ExecEvalCoerceToDomain(CoerceToDomainState *cstate, ExprContext *econtext, if (isDone && *isDone == ExprEndResult) return result; /* nothing to check */ - foreach(l, cstate->constraints) + /* Make sure we have up-to-date constraints */ + UpdateDomainConstraintRef(cstate->constraint_ref); + + foreach(l, cstate->constraint_ref->constraints) { DomainConstraintState *con = (DomainConstraintState *) lfirst(l); @@ -5050,7 +5052,12 @@ ExecInitExpr(Expr *node, PlanState *parent) cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalCoerceToDomain; cstate->arg = ExecInitExpr(ctest->arg, parent); - cstate->constraints = GetDomainConstraints(ctest->resulttype); + /* We spend an extra palloc to reduce header inclusions */ + cstate->constraint_ref = (DomainConstraintRef *) + palloc(sizeof(DomainConstraintRef)); + InitDomainConstraintRef(ctest->resulttype, + cstate->constraint_ref, + CurrentMemoryContext); state = (ExprState *) cstate; } break; diff --git a/src/backend/utils/adt/domains.c b/src/backend/utils/adt/domains.c index d84d4e8b57..ac8c25266e 100644 --- a/src/backend/utils/adt/domains.c +++ b/src/backend/utils/adt/domains.c @@ -12,10 +12,9 @@ * The overhead required for constraint checking can be high, since examining * the catalogs to discover the constraints for a given domain is not cheap. * We have three mechanisms for minimizing this cost: - * 1. In a nest of domains, we flatten the checking of all the levels - * into just one operation. - * 2. We cache the list of constraint items in the FmgrInfo struct - * passed by the caller. + * 1. We rely on the typcache to keep up-to-date copies of the constraints. + * 2. In a nest of domains, we flatten the checking of all the levels + * into just one operation (the typcache does this for us). * 3. If there are CHECK constraints, we cache a standalone ExprContext * to evaluate them in. * @@ -33,12 +32,12 @@ #include "access/htup_details.h" #include "catalog/pg_type.h" -#include "commands/typecmds.h" #include "executor/executor.h" #include "lib/stringinfo.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/syscache.h" +#include "utils/typcache.h" /* @@ -52,8 +51,8 @@ typedef struct DomainIOData Oid typioparam; int32 typtypmod; FmgrInfo proc; - /* List of constraint items to check */ - List *constraint_list; + /* Reference to cached list of constraint items to check */ + DomainConstraintRef constraint_ref; /* Context for evaluating CHECK constraints in */ ExprContext *econtext; /* Memory context this cache is in */ @@ -63,16 +62,19 @@ typedef struct DomainIOData /* * domain_state_setup - initialize the cache for a new domain type. + * + * Note: we can't re-use the same cache struct for a new domain type, + * since there's no provision for releasing the DomainConstraintRef. + * If a call site needs to deal with a new domain type, we just leak + * the old struct for the duration of the query. */ -static void -domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary, - MemoryContext mcxt) +static DomainIOData * +domain_state_setup(Oid domainType, bool binary, MemoryContext mcxt) { + DomainIOData *my_extra; Oid baseType; - MemoryContext oldcontext; - /* Mark cache invalid */ - my_extra->domain_type = InvalidOid; + my_extra = (DomainIOData *) MemoryContextAlloc(mcxt, sizeof(DomainIOData)); /* Find out the base type */ my_extra->typtypmod = -1; @@ -95,9 +97,7 @@ domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary, fmgr_info_cxt(my_extra->typiofunc, &my_extra->proc, mcxt); /* Look up constraints for domain */ - oldcontext = MemoryContextSwitchTo(mcxt); - my_extra->constraint_list = GetDomainConstraints(domainType); - MemoryContextSwitchTo(oldcontext); + InitDomainConstraintRef(domainType, &my_extra->constraint_ref, mcxt); /* We don't make an ExprContext until needed */ my_extra->econtext = NULL; @@ -105,6 +105,8 @@ domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary, /* Mark cache valid */ my_extra->domain_type = domainType; + + return my_extra; } /* @@ -118,7 +120,10 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra) ExprContext *econtext = my_extra->econtext; ListCell *l; - foreach(l, my_extra->constraint_list) + /* Make sure we have up-to-date constraints */ + UpdateDomainConstraintRef(&my_extra->constraint_ref); + + foreach(l, my_extra->constraint_ref.constraints) { DomainConstraintState *con = (DomainConstraintState *) lfirst(l); @@ -215,20 +220,16 @@ domain_in(PG_FUNCTION_ARGS) /* * We arrange to look up the needed info just once per series of calls, - * assuming the domain type doesn't change underneath us. + * assuming the domain type doesn't change underneath us (which really + * shouldn't happen, but cope if it does). */ my_extra = (DomainIOData *) fcinfo->flinfo->fn_extra; - if (my_extra == NULL) + if (my_extra == NULL || my_extra->domain_type != domainType) { - my_extra = (DomainIOData *) MemoryContextAlloc(fcinfo->flinfo->fn_mcxt, - sizeof(DomainIOData)); - domain_state_setup(my_extra, domainType, false, - fcinfo->flinfo->fn_mcxt); + my_extra = domain_state_setup(domainType, false, + fcinfo->flinfo->fn_mcxt); fcinfo->flinfo->fn_extra = (void *) my_extra; } - else if (my_extra->domain_type != domainType) - domain_state_setup(my_extra, domainType, false, - fcinfo->flinfo->fn_mcxt); /* * Invoke the base type's typinput procedure to convert the data. @@ -275,20 +276,16 @@ domain_recv(PG_FUNCTION_ARGS) /* * We arrange to look up the needed info just once per series of calls, - * assuming the domain type doesn't change underneath us. + * assuming the domain type doesn't change underneath us (which really + * shouldn't happen, but cope if it does). */ my_extra = (DomainIOData *) fcinfo->flinfo->fn_extra; - if (my_extra == NULL) + if (my_extra == NULL || my_extra->domain_type != domainType) { - my_extra = (DomainIOData *) MemoryContextAlloc(fcinfo->flinfo->fn_mcxt, - sizeof(DomainIOData)); - domain_state_setup(my_extra, domainType, true, - fcinfo->flinfo->fn_mcxt); + my_extra = domain_state_setup(domainType, true, + fcinfo->flinfo->fn_mcxt); fcinfo->flinfo->fn_extra = (void *) my_extra; } - else if (my_extra->domain_type != domainType) - domain_state_setup(my_extra, domainType, true, - fcinfo->flinfo->fn_mcxt); /* * Invoke the base type's typreceive procedure to convert the data. @@ -326,20 +323,17 @@ domain_check(Datum value, bool isnull, Oid domainType, /* * We arrange to look up the needed info just once per series of calls, - * assuming the domain type doesn't change underneath us. + * assuming the domain type doesn't change underneath us (which really + * shouldn't happen, but cope if it does). */ if (extra) my_extra = (DomainIOData *) *extra; - if (my_extra == NULL) + if (my_extra == NULL || my_extra->domain_type != domainType) { - my_extra = (DomainIOData *) MemoryContextAlloc(mcxt, - sizeof(DomainIOData)); - domain_state_setup(my_extra, domainType, true, mcxt); + my_extra = domain_state_setup(domainType, true, mcxt); if (extra) *extra = (void *) my_extra; } - else if (my_extra->domain_type != domainType) - domain_state_setup(my_extra, domainType, true, mcxt); /* * Do the necessary checks to ensure it's a valid domain value. diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 0492718437..44b5937415 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -18,15 +18,16 @@ * * Once created, a type cache entry lives as long as the backend does, so * there is no need for a call to release a cache entry. If the type is - * dropped, the cache entry simply becomes wasted storage. (For present uses, - * it would be okay to flush type cache entries at the ends of transactions, - * if we needed to reclaim space.) + * dropped, the cache entry simply becomes wasted storage. This is not + * expected to happen often, and assuming that typcache entries are good + * permanently allows caching pointers to them in long-lived places. * * We have some provisions for updating cache entries if the stored data * becomes obsolete. Information dependent on opclasses is cleared if we * detect updates to pg_opclass. We also support clearing the tuple * descriptor and operator/function parts of a rowtype's cache entry, * since those may need to change as a consequence of ALTER TABLE. + * Domain constraint changes are also tracked properly. * * * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group @@ -46,16 +47,20 @@ #include "access/htup_details.h" #include "access/nbtree.h" #include "catalog/indexing.h" +#include "catalog/pg_constraint.h" #include "catalog/pg_enum.h" #include "catalog/pg_operator.h" #include "catalog/pg_range.h" #include "catalog/pg_type.h" #include "commands/defrem.h" +#include "executor/executor.h" +#include "optimizer/planner.h" #include "utils/builtins.h" #include "utils/catcache.h" #include "utils/fmgroids.h" #include "utils/inval.h" #include "utils/lsyscache.h" +#include "utils/memutils.h" #include "utils/rel.h" #include "utils/snapmgr.h" #include "utils/syscache.h" @@ -65,6 +70,9 @@ /* The main type cache hashtable searched by lookup_type_cache */ static HTAB *TypeCacheHash = NULL; +/* List of type cache entries for domain types */ +static TypeCacheEntry *firstDomainTypeEntry = NULL; + /* Private flag bits in the TypeCacheEntry.flags field */ #define TCFLAGS_CHECKED_BTREE_OPCLASS 0x0001 #define TCFLAGS_CHECKED_HASH_OPCLASS 0x0002 @@ -80,6 +88,19 @@ static HTAB *TypeCacheHash = NULL; #define TCFLAGS_CHECKED_FIELD_PROPERTIES 0x0800 #define TCFLAGS_HAVE_FIELD_EQUALITY 0x1000 #define TCFLAGS_HAVE_FIELD_COMPARE 0x2000 +#define TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS 0x4000 + +/* + * Data stored about a domain type's constraints. Note that we do not create + * this struct for the common case of a constraint-less domain; we just set + * domainData to NULL to indicate that. + */ +struct DomainConstraintCache +{ + List *constraints; /* list of DomainConstraintState nodes */ + MemoryContext dccContext; /* memory context holding all associated data */ + long dccRefCount; /* number of references to this struct */ +}; /* Private information to support comparisons of enum values */ typedef struct @@ -127,6 +148,9 @@ static int32 NextRecordTypmod = 0; /* number of entries used */ static void load_typcache_tupdesc(TypeCacheEntry *typentry); static void load_rangetype_info(TypeCacheEntry *typentry); +static void load_domaintype_info(TypeCacheEntry *typentry); +static void decr_dcc_refcount(DomainConstraintCache *dcc); +static void dccref_deletion_callback(void *arg); static bool array_element_has_equality(TypeCacheEntry *typentry); static bool array_element_has_compare(TypeCacheEntry *typentry); static bool array_element_has_hashing(TypeCacheEntry *typentry); @@ -136,6 +160,7 @@ static bool record_fields_have_compare(TypeCacheEntry *typentry); static void cache_record_field_properties(TypeCacheEntry *typentry); static void TypeCacheRelCallback(Datum arg, Oid relid); static void TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue); +static void TypeCacheConstrCallback(Datum arg, int cacheid, uint32 hashvalue); static void load_enum_cache_data(TypeCacheEntry *tcache); static EnumItem *find_enumitem(TypeCacheEnumData *enumdata, Oid arg); static int enum_oid_cmp(const void *left, const void *right); @@ -172,6 +197,8 @@ lookup_type_cache(Oid type_id, int flags) /* Also set up callbacks for SI invalidations */ CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0); CacheRegisterSyscacheCallback(CLAOID, TypeCacheOpcCallback, (Datum) 0); + CacheRegisterSyscacheCallback(CONSTROID, TypeCacheConstrCallback, (Datum) 0); + CacheRegisterSyscacheCallback(TYPEOID, TypeCacheConstrCallback, (Datum) 0); /* Also make sure CacheMemoryContext exists */ if (!CacheMemoryContext) @@ -217,6 +244,13 @@ lookup_type_cache(Oid type_id, int flags) typentry->typtype = typtup->typtype; typentry->typrelid = typtup->typrelid; + /* If it's a domain, immediately thread it into the domain cache list */ + if (typentry->typtype == TYPTYPE_DOMAIN) + { + typentry->nextDomain = firstDomainTypeEntry; + firstDomainTypeEntry = typentry; + } + ReleaseSysCache(tp); } @@ -503,6 +537,16 @@ lookup_type_cache(Oid type_id, int flags) load_rangetype_info(typentry); } + /* + * If requested, get information about a domain type + */ + if ((flags & TYPECACHE_DOMAIN_INFO) && + (typentry->flags & TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS) == 0 && + typentry->typtype == TYPTYPE_DOMAIN) + { + load_domaintype_info(typentry); + } + return typentry; } @@ -591,6 +635,327 @@ load_rangetype_info(TypeCacheEntry *typentry) } +/* + * load_domaintype_info --- helper routine to set up domain constraint info + * + * Note: we assume we're called in a relatively short-lived context, so it's + * okay to leak data into the current context while scanning pg_constraint. + * We build the new DomainConstraintCache data in a context underneath + * CurrentMemoryContext, and reparent it under CacheMemoryContext when + * complete. + */ +static void +load_domaintype_info(TypeCacheEntry *typentry) +{ + Oid typeOid = typentry->type_id; + DomainConstraintCache *dcc; + bool notNull = false; + Relation conRel; + MemoryContext oldcxt; + + /* + * If we're here, any existing constraint info is stale, so release it. + * For safety, be sure to null the link before trying to delete the data. + */ + if (typentry->domainData) + { + dcc = typentry->domainData; + typentry->domainData = NULL; + decr_dcc_refcount(dcc); + } + + /* + * We try to optimize the common case of no domain constraints, so don't + * create the dcc object and context until we find a constraint. + */ + dcc = NULL; + + /* + * Scan pg_constraint for relevant constraints. We want to find + * constraints for not just this domain, but any ancestor domains, so the + * outer loop crawls up the domain stack. + */ + conRel = heap_open(ConstraintRelationId, AccessShareLock); + + for (;;) + { + HeapTuple tup; + HeapTuple conTup; + Form_pg_type typTup; + ScanKeyData key[1]; + SysScanDesc scan; + + tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typeOid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for type %u", typeOid); + typTup = (Form_pg_type) GETSTRUCT(tup); + + if (typTup->typtype != TYPTYPE_DOMAIN) + { + /* Not a domain, so done */ + ReleaseSysCache(tup); + break; + } + + /* Test for NOT NULL Constraint */ + if (typTup->typnotnull) + notNull = true; + + /* Look for CHECK Constraints on this domain */ + ScanKeyInit(&key[0], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(typeOid)); + + scan = systable_beginscan(conRel, ConstraintTypidIndexId, true, + NULL, 1, key); + + while (HeapTupleIsValid(conTup = systable_getnext(scan))) + { + Form_pg_constraint c = (Form_pg_constraint) GETSTRUCT(conTup); + Datum val; + bool isNull; + char *constring; + Expr *check_expr; + DomainConstraintState *r; + + /* Ignore non-CHECK constraints (presently, shouldn't be any) */ + if (c->contype != CONSTRAINT_CHECK) + continue; + + /* Not expecting conbin to be NULL, but we'll test for it anyway */ + val = fastgetattr(conTup, Anum_pg_constraint_conbin, + conRel->rd_att, &isNull); + if (isNull) + elog(ERROR, "domain \"%s\" constraint \"%s\" has NULL conbin", + NameStr(typTup->typname), NameStr(c->conname)); + + /* Convert conbin to C string in caller context */ + constring = TextDatumGetCString(val); + + /* Create the DomainConstraintCache object and context if needed */ + if (dcc == NULL) + { + MemoryContext cxt; + + cxt = AllocSetContextCreate(CurrentMemoryContext, + "Domain constraints", + ALLOCSET_SMALL_INITSIZE, + ALLOCSET_SMALL_MINSIZE, + ALLOCSET_SMALL_MAXSIZE); + dcc = (DomainConstraintCache *) + MemoryContextAlloc(cxt, sizeof(DomainConstraintCache)); + dcc->constraints = NIL; + dcc->dccContext = cxt; + dcc->dccRefCount = 0; + } + + /* Create node trees in DomainConstraintCache's context */ + oldcxt = MemoryContextSwitchTo(dcc->dccContext); + + check_expr = (Expr *) stringToNode(constring); + + /* ExecInitExpr assumes we've planned the expression */ + check_expr = expression_planner(check_expr); + + r = makeNode(DomainConstraintState); + r->constrainttype = DOM_CONSTRAINT_CHECK; + r->name = pstrdup(NameStr(c->conname)); + r->check_expr = ExecInitExpr(check_expr, NULL); + + /* + * Use lcons() here because constraints of parent domains should + * be applied earlier. + */ + dcc->constraints = lcons(r, dcc->constraints); + + MemoryContextSwitchTo(oldcxt); + } + + systable_endscan(scan); + + /* loop to next domain in stack */ + typeOid = typTup->typbasetype; + ReleaseSysCache(tup); + } + + heap_close(conRel, AccessShareLock); + + /* + * Only need to add one NOT NULL check regardless of how many domains in + * the stack request it. + */ + if (notNull) + { + DomainConstraintState *r; + + /* Create the DomainConstraintCache object and context if needed */ + if (dcc == NULL) + { + MemoryContext cxt; + + cxt = AllocSetContextCreate(CurrentMemoryContext, + "Domain constraints", + ALLOCSET_SMALL_INITSIZE, + ALLOCSET_SMALL_MINSIZE, + ALLOCSET_SMALL_MAXSIZE); + dcc = (DomainConstraintCache *) + MemoryContextAlloc(cxt, sizeof(DomainConstraintCache)); + dcc->constraints = NIL; + dcc->dccContext = cxt; + dcc->dccRefCount = 0; + } + + /* Create node trees in DomainConstraintCache's context */ + oldcxt = MemoryContextSwitchTo(dcc->dccContext); + + r = makeNode(DomainConstraintState); + + r->constrainttype = DOM_CONSTRAINT_NOTNULL; + r->name = pstrdup("NOT NULL"); + r->check_expr = NULL; + + /* lcons to apply the nullness check FIRST */ + dcc->constraints = lcons(r, dcc->constraints); + + MemoryContextSwitchTo(oldcxt); + } + + /* + * If we made a constraint object, move it into CacheMemoryContext and + * attach it to the typcache entry. + */ + if (dcc) + { + MemoryContextSetParent(dcc->dccContext, CacheMemoryContext); + typentry->domainData = dcc; + dcc->dccRefCount++; /* count the typcache's reference */ + } + + /* Either way, the typcache entry's domain data is now valid. */ + typentry->flags |= TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS; +} + +/* + * decr_dcc_refcount --- decrement a DomainConstraintCache's refcount, + * and free it if no references remain + */ +static void +decr_dcc_refcount(DomainConstraintCache *dcc) +{ + Assert(dcc->dccRefCount > 0); + if (--(dcc->dccRefCount) <= 0) + MemoryContextDelete(dcc->dccContext); +} + +/* + * Context reset/delete callback for a DomainConstraintRef + */ +static void +dccref_deletion_callback(void *arg) +{ + DomainConstraintRef *ref = (DomainConstraintRef *) arg; + DomainConstraintCache *dcc = ref->dcc; + + /* Paranoia --- be sure link is nulled before trying to release */ + if (dcc) + { + ref->constraints = NIL; + ref->dcc = NULL; + decr_dcc_refcount(dcc); + } +} + +/* + * InitDomainConstraintRef --- initialize a DomainConstraintRef struct + * + * Caller must tell us the MemoryContext in which the DomainConstraintRef + * lives. The ref will be cleaned up when that context is reset/deleted. + */ +void +InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref, + MemoryContext refctx) +{ + /* Look up the typcache entry --- we assume it survives indefinitely */ + ref->tcache = lookup_type_cache(type_id, TYPECACHE_DOMAIN_INFO); + /* For safety, establish the callback before acquiring a refcount */ + ref->dcc = NULL; + ref->callback.func = dccref_deletion_callback; + ref->callback.arg = (void *) ref; + MemoryContextRegisterResetCallback(refctx, &ref->callback); + /* Acquire refcount if there are constraints, and set up exported list */ + if (ref->tcache->domainData) + { + ref->dcc = ref->tcache->domainData; + ref->dcc->dccRefCount++; + ref->constraints = ref->dcc->constraints; + } + else + ref->constraints = NIL; +} + +/* + * UpdateDomainConstraintRef --- recheck validity of domain constraint info + * + * If the domain's constraint set changed, ref->constraints is updated to + * point at a new list of cached constraints. + * + * In the normal case where nothing happened to the domain, this is cheap + * enough that it's reasonable (and expected) to check before *each* use + * of the constraint info. + */ +void +UpdateDomainConstraintRef(DomainConstraintRef *ref) +{ + TypeCacheEntry *typentry = ref->tcache; + + /* Make sure typcache entry's data is up to date */ + if ((typentry->flags & TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS) == 0 && + typentry->typtype == TYPTYPE_DOMAIN) + load_domaintype_info(typentry); + + /* Transfer to ref object if there's new info, adjusting refcounts */ + if (ref->dcc != typentry->domainData) + { + /* Paranoia --- be sure link is nulled before trying to release */ + DomainConstraintCache *dcc = ref->dcc; + + if (dcc) + { + ref->constraints = NIL; + ref->dcc = NULL; + decr_dcc_refcount(dcc); + } + dcc = typentry->domainData; + if (dcc) + { + ref->dcc = dcc; + dcc->dccRefCount++; + ref->constraints = dcc->constraints; + } + } +} + +/* + * DomainHasConstraints --- utility routine to check if a domain has constraints + * + * This is defined to return false, not fail, if type is not a domain. + */ +bool +DomainHasConstraints(Oid type_id) +{ + TypeCacheEntry *typentry; + + /* + * Note: a side effect is to cause the typcache's domain data to become + * valid. This is fine since we'll likely need it soon if there is any. + */ + typentry = lookup_type_cache(type_id, TYPECACHE_DOMAIN_INFO); + + return (typentry->domainData != NULL); +} + + /* * array_element_has_equality and friends are helper routines to check * whether we should believe that array_eq and related functions will work @@ -1003,6 +1368,40 @@ TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue) } } +/* + * TypeCacheConstrCallback + * Syscache inval callback function + * + * This is called when a syscache invalidation event occurs for any + * pg_constraint or pg_type row. We flush information about domain + * constraints when this happens. + * + * It's slightly annoying that we can't tell whether the inval event was for a + * domain constraint/type record or not; there's usually more update traffic + * for table constraints/types than domain constraints, so we'll do a lot of + * useless flushes. Still, this is better than the old no-caching-at-all + * approach to domain constraints. + */ +static void +TypeCacheConstrCallback(Datum arg, int cacheid, uint32 hashvalue) +{ + TypeCacheEntry *typentry; + + /* + * Because this is called very frequently, and typically very few of the + * typcache entries are for domains, we don't use hash_seq_search here. + * Instead we thread all the domain-type entries together so that we can + * visit them cheaply. + */ + for (typentry = firstDomainTypeEntry; + typentry != NULL; + typentry = typentry->nextDomain) + { + /* Reset domain constraint validity information */ + typentry->flags &= ~TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS; + } +} + /* * Check if given OID is part of the subset that's sortable by comparisons diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index e18a71463e..0a638002c3 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -39,8 +39,6 @@ extern Oid AlterDomainDropConstraint(List *names, const char *constrName, extern void checkDomainOwner(HeapTuple tup); -extern List *GetDomainConstraints(Oid typeOid); - extern Oid RenameType(RenameStmt *stmt); extern Oid AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype); extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 41288eda6e..59b17f3c99 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -942,8 +942,9 @@ typedef struct CoerceToDomainState { ExprState xprstate; ExprState *arg; /* input expression */ - /* Cached list of constraints that need to be checked */ - List *constraints; /* list of DomainConstraintState nodes */ + /* Cached set of constraints that need to be checked */ + /* use struct pointer to avoid including typcache.h here */ + struct DomainConstraintRef *constraint_ref; } CoerceToDomainState; /* diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h index b544180b46..1a9befb9d7 100644 --- a/src/include/utils/typcache.h +++ b/src/include/utils/typcache.h @@ -20,6 +20,9 @@ #include "fmgr.h" +/* DomainConstraintCache is an opaque struct known only within typcache.c */ +typedef struct DomainConstraintCache DomainConstraintCache; + /* TypeCacheEnumData is an opaque struct known only within typcache.c */ struct TypeCacheEnumData; @@ -84,6 +87,12 @@ typedef struct TypeCacheEntry FmgrInfo rng_canonical_finfo; /* canonicalization function, if any */ FmgrInfo rng_subdiff_finfo; /* difference function, if any */ + /* + * Domain constraint data if it's a domain type. NULL if not domain, or + * if domain has no constraints, or if information hasn't been requested. + */ + DomainConstraintCache *domainData; + /* Private data, for internal use of typcache.c only */ int flags; /* flags about what we've computed */ @@ -92,6 +101,9 @@ typedef struct TypeCacheEntry * information hasn't been requested. */ struct TypeCacheEnumData *enumData; + + /* We also maintain a list of all known domain-type cache entries */ + struct TypeCacheEntry *nextDomain; } TypeCacheEntry; /* Bit flags to indicate which fields a given caller needs to have set */ @@ -107,9 +119,34 @@ typedef struct TypeCacheEntry #define TYPECACHE_BTREE_OPFAMILY 0x0200 #define TYPECACHE_HASH_OPFAMILY 0x0400 #define TYPECACHE_RANGE_INFO 0x0800 +#define TYPECACHE_DOMAIN_INFO 0x1000 + +/* + * Callers wishing to maintain a long-lived reference to a domain's constraint + * set must store it in one of these. Use InitDomainConstraintRef() and + * UpdateDomainConstraintRef() to manage it. Note: DomainConstraintState is + * considered an executable expression type, so it's defined in execnodes.h. + */ +typedef struct DomainConstraintRef +{ + List *constraints; /* list of DomainConstraintState nodes */ + + /* Management data --- treat these fields as private to typcache.c */ + TypeCacheEntry *tcache; /* owning typcache entry */ + DomainConstraintCache *dcc; /* current constraints, or NULL if none */ + MemoryContextCallback callback; /* used to release refcount when done */ +} DomainConstraintRef; + extern TypeCacheEntry *lookup_type_cache(Oid type_id, int flags); +extern void InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref, + MemoryContext refctx); + +extern void UpdateDomainConstraintRef(DomainConstraintRef *ref); + +extern bool DomainHasConstraints(Oid type_id); + extern TupleDesc lookup_rowtype_tupdesc(Oid type_id, int32 typmod); extern TupleDesc lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod, diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index 78e7704956..c107d37490 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -652,6 +652,36 @@ ERROR: value for domain orderedpair violates check constraint "orderedpair_chec CONTEXT: PL/pgSQL function array_elem_check(integer) line 5 at assignment drop function array_elem_check(int); -- +-- Check enforcement of changing constraints in plpgsql +-- +create domain di as int; +create function dom_check(int) returns di as $$ +declare d di; +begin + d := $1; + return d; +end +$$ language plpgsql immutable; +select dom_check(0); + dom_check +----------- + 0 +(1 row) + +alter domain di add constraint pos check (value > 0); +select dom_check(0); -- fail +ERROR: value for domain di violates check constraint "pos" +CONTEXT: PL/pgSQL function dom_check(integer) line 4 at assignment +alter domain di drop constraint pos; +select dom_check(0); + dom_check +----------- + 0 +(1 row) + +drop function dom_check(int); +drop domain di; +-- -- Renaming -- create domain testdomain1 as int; diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 5af36af1ef..ab1fcd3f22 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -487,6 +487,33 @@ select array_elem_check(-1); drop function array_elem_check(int); +-- +-- Check enforcement of changing constraints in plpgsql +-- + +create domain di as int; + +create function dom_check(int) returns di as $$ +declare d di; +begin + d := $1; + return d; +end +$$ language plpgsql immutable; + +select dom_check(0); + +alter domain di add constraint pos check (value > 0); + +select dom_check(0); -- fail + +alter domain di drop constraint pos; + +select dom_check(0); + +drop function dom_check(int); + +drop domain di; -- -- Renaming -- 2.40.0