diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 57257d281d40fe7c7472d53b249e7913185d699a..6598892b14bdcf74724eeda54fc79308f673aa75 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -94,6 +94,13 @@ static TypeCacheEntry *firstDomainTypeEntry = NULL; * 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. + * + * Within a DomainConstraintCache, we abuse the DomainConstraintState node + * type a bit: check_expr fields point to expression plan trees, not plan + * state trees. When needed, expression state trees are built by flat-copying + * the DomainConstraintState nodes and applying ExecInitExpr to check_expr. + * Such a state tree is not part of the DomainConstraintCache, but is + * considered to belong to a DomainConstraintRef. */ struct DomainConstraintCache { @@ -152,6 +159,7 @@ static void load_domaintype_info(TypeCacheEntry *typentry); static int dcs_cmp(const void *a, const void *b); static void decr_dcc_refcount(DomainConstraintCache *dcc); static void dccref_deletion_callback(void *arg); +static List *prep_domain_constraints(List *constraints, MemoryContext execctx); static bool array_element_has_equality(TypeCacheEntry *typentry); static bool array_element_has_compare(TypeCacheEntry *typentry); static bool array_element_has_hashing(TypeCacheEntry *typentry); @@ -762,13 +770,14 @@ load_domaintype_info(TypeCacheEntry *typentry) check_expr = (Expr *) stringToNode(constring); - /* ExecInitExpr assumes we've planned the expression */ + /* ExecInitExpr will assume 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); + /* Must cast here because we're not storing an expr state node */ + r->check_expr = (ExprState *) check_expr; MemoryContextSwitchTo(oldcxt); @@ -913,6 +922,40 @@ dccref_deletion_callback(void *arg) } } +/* + * prep_domain_constraints --- prepare domain constraints for execution + * + * The expression trees stored in the DomainConstraintCache's list are + * converted to executable expression state trees stored in execctx. + */ +static List * +prep_domain_constraints(List *constraints, MemoryContext execctx) +{ + List *result = NIL; + MemoryContext oldcxt; + ListCell *lc; + + oldcxt = MemoryContextSwitchTo(execctx); + + foreach(lc, constraints) + { + DomainConstraintState *r = (DomainConstraintState *) lfirst(lc); + DomainConstraintState *newr; + + newr = makeNode(DomainConstraintState); + newr->constrainttype = r->constrainttype; + newr->name = r->name; + /* Must cast here because cache items contain expr plan trees */ + newr->check_expr = ExecInitExpr((Expr *) r->check_expr, NULL); + + result = lappend(result, newr); + } + + MemoryContextSwitchTo(oldcxt); + + return result; +} + /* * InitDomainConstraintRef --- initialize a DomainConstraintRef struct * @@ -926,6 +969,7 @@ InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref, /* 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->refctx = refctx; ref->dcc = NULL; ref->callback.func = dccref_deletion_callback; ref->callback.arg = (void *) ref; @@ -935,7 +979,8 @@ InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref, { ref->dcc = ref->tcache->domainData; ref->dcc->dccRefCount++; - ref->constraints = ref->dcc->constraints; + ref->constraints = prep_domain_constraints(ref->dcc->constraints, + ref->refctx); } else ref->constraints = NIL; @@ -969,6 +1014,14 @@ UpdateDomainConstraintRef(DomainConstraintRef *ref) if (dcc) { + /* + * Note: we just leak the previous list of executable domain + * constraints. Alternatively, we could keep those in a child + * context of ref->refctx and free that context at this point. + * However, in practice this code path will be taken so seldom + * that the extra bookkeeping for a child context doesn't seem + * worthwhile; we'll just allow a leak for the lifespan of refctx. + */ ref->constraints = NIL; ref->dcc = NULL; decr_dcc_refcount(dcc); @@ -978,7 +1031,8 @@ UpdateDomainConstraintRef(DomainConstraintRef *ref) { ref->dcc = dcc; dcc->dccRefCount++; - ref->constraints = dcc->constraints; + ref->constraints = prep_domain_constraints(dcc->constraints, + ref->refctx); } } } diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h index 1a9befb9d7a546ef9ba8755f41e1f25cce524172..23618cc6100bf7594767e338ab970102098953b3 100644 --- a/src/include/utils/typcache.h +++ b/src/include/utils/typcache.h @@ -130,6 +130,7 @@ typedef struct TypeCacheEntry typedef struct DomainConstraintRef { List *constraints; /* list of DomainConstraintState nodes */ + MemoryContext refctx; /* context holding DomainConstraintRef */ /* Management data --- treat these fields as private to typcache.c */ TypeCacheEntry *tcache; /* owning typcache entry */ diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index c107d3749029e37ac2f949b7db57ff19495d42ad..41b70e287bde14cfb68281ab81426b73b2062ecc 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -682,6 +682,31 @@ select dom_check(0); drop function dom_check(int); drop domain di; -- +-- Check use of a (non-inline-able) SQL function in a domain constraint; +-- this has caused issues in the past +-- +create function sql_is_distinct_from(anyelement, anyelement) +returns boolean language sql +as 'select $1 is distinct from $2 limit 1'; +create domain inotnull int + check (sql_is_distinct_from(value, null)); +select 1::inotnull; + inotnull +---------- + 1 +(1 row) + +select null::inotnull; +ERROR: value for domain inotnull violates check constraint "inotnull_check" +create table dom_table (x inotnull); +insert into dom_table values ('1'); +insert into dom_table values (1); +insert into dom_table values (null); +ERROR: value for domain inotnull violates check constraint "inotnull_check" +drop table dom_table; +drop domain inotnull; +drop function sql_is_distinct_from(anyelement, anyelement); +-- -- Renaming -- create domain testdomain1 as int; diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index ab1fcd3f22cb2816d6546599c9b0161ac79a77f9..407d3efc35ae70e081ebc6ec56be09b83ac381c7 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -515,6 +515,30 @@ drop function dom_check(int); drop domain di; +-- +-- Check use of a (non-inline-able) SQL function in a domain constraint; +-- this has caused issues in the past +-- + +create function sql_is_distinct_from(anyelement, anyelement) +returns boolean language sql +as 'select $1 is distinct from $2 limit 1'; + +create domain inotnull int + check (sql_is_distinct_from(value, null)); + +select 1::inotnull; +select null::inotnull; + +create table dom_table (x inotnull); +insert into dom_table values ('1'); +insert into dom_table values (1); +insert into dom_table values (null); + +drop table dom_table; +drop domain inotnull; +drop function sql_is_distinct_from(anyelement, anyelement); + -- -- Renaming --