Re: plpgsql versus domains

Andres Freund <andres@2ndquadrant.com> writes:
> On 2015-02-26 13:53:18 -0500, Tom Lane wrote:
>> Andres Freund <andres@2ndquadrant.com> writes:
>>> You're probably going to kill me because of the increased complexity,
>>> but how about making typecache.c smarter, more in the vein of
>>> relcache.c, and store the relevant info in there?

>> I thought that's what I was proposing in point #1.

> Sure, but my second point was to avoid duplicating that information into
> ->fcinfo and such and instead reference the typecache entry from
> there. So that if the typecache entry is being rebuilt (a new mechanism
> I'm afraid), whatever uses ->fcinfo gets the updated
> functions/definition.

Here's a draft patch for that.  This fixes the delayed-domain-constraint
problem pretty thoroughly, in that any attempt to check a domain's
constraints will use fully up-to-date info.

This is the first attempt at weaponizing the memory context reset/delete
feature, and I'm fairly happy with it, except for one thing: I had to
#include utils/memnodes.h into typcache.h in order to preserve the
intended property that the callback structs could be included directly
into structs using them.  Now, that's not awful in itself, because
typcache.h isn't used everywhere; but if this feature gets popular we'll
find memnodes.h being included pretty much everywhere, which is not so
great.  I'm thinking about moving struct MemoryContextCallback and the
extern for MemoryContextRegisterResetCallback into palloc.h to avoid this.
Maybe that's too far in the other direction.  Thoughts?  Compromise ideas?

Also, instrumenting the code showed that TypeCacheConstrCallback gets
called quite a lot during the standard regression tests, which is why
I went out of my way to make it quick.  Almost all of those cache flushes
are from non-domain-related updates on pg_type or pg_constraint, so
they're not really necessary from a logical perspective, and they're
surely going to hurt performance for heavy users of domains.  I think to
fix this we'd have to split pg_constraint into two catalogs, one for table
constraints and one for domain constraints; which would be a good thing
anyway from a normal-form-theory perspective.  And we'd have to get rid of
pg_type.typnotnull and instead store domain NOT NULL constraints in this
hypothetical domain constraint catalog.  I don't plan to do anything about
that myself right now, because I'm not sure that production databases
would have the kind of traffic on pg_type and pg_constraint that the
regression tests exhibit.  But at some point we might have to fix it.

            regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 07ab4b4..7455020 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** ATExecAddColumn(List **wqueue, AlteredTa
*** 4824,4830 ****
      {
          defval = (Expr *) build_column_default(rel, attribute.attnum);

!         if (!defval && GetDomainConstraints(typeOid) != NIL)
          {
              Oid            baseTypeId;
              int32        baseTypeMod;
--- 4824,4830 ----
      {
          defval = (Expr *) build_column_default(rel, attribute.attnum);

!         if (!defval && DomainHasConstraints(typeOid))
          {
              Oid            baseTypeId;
              int32        baseTypeMod;
*************** ATColumnChangeRequiresRewrite(Node *expr
*** 7778,7784 ****
          {
              CoerceToDomain *d = (CoerceToDomain *) expr;

!             if (GetDomainConstraints(d->resulttype) != NIL)
                  return true;
              expr = (Node *) d->arg;
          }
--- 7778,7784 ----
          {
              CoerceToDomain *d = (CoerceToDomain *) expr;

!             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 b77e1b4..d800033 100644
*** a/src/backend/commands/typecmds.c
--- b/src/backend/commands/typecmds.c
***************
*** 59,65 ****
  #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"
--- 59,64 ----
*************** domainAddConstraint(Oid domainOid, Oid d
*** 3081,3206 ****
      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
--- 3080,3085 ----
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index fec76d4..d94fe58 100644
*** a/src/backend/executor/execQual.c
--- b/src/backend/executor/execQual.c
***************
*** 41,47 ****
  #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"
--- 41,46 ----
*************** ExecEvalCoerceToDomain(CoerceToDomainSta
*** 3929,3935 ****
      if (isDone && *isDone == ExprEndResult)
          return result;            /* nothing to check */

!     foreach(l, cstate->constraints)
      {
          DomainConstraintState *con = (DomainConstraintState *) lfirst(l);

--- 3928,3937 ----
      if (isDone && *isDone == ExprEndResult)
          return result;            /* nothing to check */

!     /* Make sure we have up-to-date constraints */
!     UpdateDomainConstraintRef(cstate->constraint_ref);
!
!     foreach(l, cstate->constraint_ref->constraints)
      {
          DomainConstraintState *con = (DomainConstraintState *) lfirst(l);

*************** ExecInitExpr(Expr *node, PlanState *pare
*** 5050,5056 ****

                  cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalCoerceToDomain;
                  cstate->arg = ExecInitExpr(ctest->arg, parent);
!                 cstate->constraints = GetDomainConstraints(ctest->resulttype);
                  state = (ExprState *) cstate;
              }
              break;
--- 5052,5063 ----

                  cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalCoerceToDomain;
                  cstate->arg = ExecInitExpr(ctest->arg, parent);
!                 /* 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 d84d4e8..9c8eb11 100644
*** a/src/backend/utils/adt/domains.c
--- b/src/backend/utils/adt/domains.c
***************
*** 12,21 ****
   * 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.
   *    3.  If there are CHECK constraints, we cache a standalone ExprContext
   *        to evaluate them in.
   *
--- 12,20 ----
   * 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.  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,44 ****

  #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"


  /*
--- 32,43 ----

  #include "access/htup_details.h"
  #include "catalog/pg_type.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"


  /*
*************** typedef struct DomainIOData
*** 52,59 ****
      Oid            typioparam;
      int32        typtypmod;
      FmgrInfo    proc;
!     /* List of constraint items to check */
!     List       *constraint_list;
      /* Context for evaluating CHECK constraints in */
      ExprContext *econtext;
      /* Memory context this cache is in */
--- 51,58 ----
      Oid            typioparam;
      int32        typtypmod;
      FmgrInfo    proc;
!     /* 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 */
*************** domain_state_setup(DomainIOData *my_extr
*** 69,75 ****
                     MemoryContext mcxt)
  {
      Oid            baseType;
-     MemoryContext oldcontext;

      /* Mark cache invalid */
      my_extra->domain_type = InvalidOid;
--- 68,73 ----
*************** domain_state_setup(DomainIOData *my_extr
*** 95,103 ****
      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);

      /* We don't make an ExprContext until needed */
      my_extra->econtext = NULL;
--- 93,99 ----
      fmgr_info_cxt(my_extra->typiofunc, &my_extra->proc, mcxt);

      /* Look up constraints for domain */
!     InitDomainConstraintRef(domainType, &my_extra->constraint_ref, mcxt);

      /* We don't make an ExprContext until needed */
      my_extra->econtext = NULL;
*************** domain_check_input(Datum value, bool isn
*** 118,124 ****
      ExprContext *econtext = my_extra->econtext;
      ListCell   *l;

!     foreach(l, my_extra->constraint_list)
      {
          DomainConstraintState *con = (DomainConstraintState *) lfirst(l);

--- 114,123 ----
      ExprContext *econtext = my_extra->econtext;
      ListCell   *l;

!     /* 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);

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 0492718..44b5937 100644
*** a/src/backend/utils/cache/typcache.c
--- b/src/backend/utils/cache/typcache.c
***************
*** 18,32 ****
   *
   * 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.)
   *
   * 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.
   *
   *
   * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
--- 18,33 ----
   *
   * 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.  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,61 ****
--- 47,66 ----
  #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,70 ****
--- 70,78 ----
  /* 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
*************** static HTAB *TypeCacheHash = NULL;
*** 80,85 ****
--- 88,106 ----
  #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
*************** static int32 NextRecordTypmod = 0;        /* n
*** 127,132 ****
--- 148,156 ----

  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);
*************** static bool record_fields_have_compare(T
*** 136,141 ****
--- 160,166 ----
  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);
*************** lookup_type_cache(Oid type_id, int flags
*** 172,177 ****
--- 197,204 ----
          /* 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)
*************** lookup_type_cache(Oid type_id, int flags
*** 217,222 ****
--- 244,256 ----
          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);
      }

*************** lookup_type_cache(Oid type_id, int flags
*** 503,508 ****
--- 537,552 ----
          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;
  }

*************** load_rangetype_info(TypeCacheEntry *type
*** 592,597 ****
--- 636,962 ----


  /*
+  * 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
   * on the given array type or composite type.
*************** TypeCacheOpcCallback(Datum arg, int cach
*** 1003,1008 ****
--- 1368,1407 ----
      }
  }

+ /*
+  * 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 e18a714..0a63800 100644
*** a/src/include/commands/typecmds.h
--- b/src/include/commands/typecmds.h
*************** extern Oid AlterDomainDropConstraint(Lis
*** 39,46 ****

  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,
--- 39,44 ----
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 41288ed..59b17f3 100644
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
*************** typedef struct CoerceToDomainState
*** 942,949 ****
  {
      ExprState    xprstate;
      ExprState  *arg;            /* input expression */
!     /* Cached list of constraints that need to be checked */
!     List       *constraints;    /* list of DomainConstraintState nodes */
  } CoerceToDomainState;

  /*
--- 942,950 ----
  {
      ExprState    xprstate;
      ExprState  *arg;            /* input expression */
!     /* 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 b544180..0fe1ce9 100644
*** a/src/include/utils/typcache.h
--- b/src/include/utils/typcache.h
***************
*** 18,25 ****
--- 18,29 ----

  #include "access/tupdesc.h"
  #include "fmgr.h"
+ #include "nodes/memnodes.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;

*************** typedef struct TypeCacheEntry
*** 84,89 ****
--- 88,99 ----
      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 */

*************** typedef struct TypeCacheEntry
*** 92,97 ****
--- 102,110 ----
       * 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 */
*************** typedef struct TypeCacheEntry
*** 107,115 ****
--- 120,152 ----
  #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 78e7704..c107d37 100644
*** a/src/test/regress/expected/domain.out
--- b/src/test/regress/expected/domain.out
*************** ERROR:  value for domain orderedpair vio
*** 652,657 ****
--- 652,687 ----
  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 5af36af..ab1fcd3 100644
*** a/src/test/regress/sql/domain.sql
--- b/src/test/regress/sql/domain.sql
*************** select array_elem_check(-1);
*** 487,492 ****
--- 487,519 ----

  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