Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitec7eef6

Browse files
committed
Avoid caching expression state trees for domain constraints across queries.
In commit8abb3cd I attempted to cachethe expression state trees constructed for domain CHECK constraints forthe life of the backend (assuming the domain's constraints don't getredefined). However, this turns out not to work very well, becauseexecQual.c will run those state trees with ecxt_per_query_memory pointingto a query-lifespan context, and in some situations we'll end up withpointers into that context getting stored into the state trees. Thishappens in particular with SQL-language functions, as reported byEmre Hasegeli, but there are many other cases.To fix, keep only the expression plan trees for domain CHECK constraintsin the typcache's data structure, and revert to performing ExecInitExpr(at least) once per query to set up expression state trees in the query'scontext.Eventually it'd be nice to undo this, but that will require some carefulthought about memory management for expression state trees, and it seemsfar too late for any such redesign in 9.5. This way is still much moreefficient than what happened before8abb3cd.
1 parent8d32717 commitec7eef6

File tree

4 files changed

+108
-4
lines changed

4 files changed

+108
-4
lines changed

‎src/backend/utils/cache/typcache.c

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ static TypeCacheEntry *firstDomainTypeEntry = NULL;
9494
* Data stored about a domain type's constraints. Note that we do not create
9595
* this struct for the common case of a constraint-less domain; we just set
9696
* domainData to NULL to indicate that.
97+
*
98+
* Within a DomainConstraintCache, we abuse the DomainConstraintState node
99+
* type a bit: check_expr fields point to expression plan trees, not plan
100+
* state trees. When needed, expression state trees are built by flat-copying
101+
* the DomainConstraintState nodes and applying ExecInitExpr to check_expr.
102+
* Such a state tree is not part of the DomainConstraintCache, but is
103+
* considered to belong to a DomainConstraintRef.
97104
*/
98105
structDomainConstraintCache
99106
{
@@ -152,6 +159,7 @@ static void load_domaintype_info(TypeCacheEntry *typentry);
152159
staticintdcs_cmp(constvoid*a,constvoid*b);
153160
staticvoiddecr_dcc_refcount(DomainConstraintCache*dcc);
154161
staticvoiddccref_deletion_callback(void*arg);
162+
staticList*prep_domain_constraints(List*constraints,MemoryContextexecctx);
155163
staticboolarray_element_has_equality(TypeCacheEntry*typentry);
156164
staticboolarray_element_has_compare(TypeCacheEntry*typentry);
157165
staticboolarray_element_has_hashing(TypeCacheEntry*typentry);
@@ -762,13 +770,14 @@ load_domaintype_info(TypeCacheEntry *typentry)
762770

763771
check_expr= (Expr*)stringToNode(constring);
764772

765-
/* ExecInitExprassumes we've planned the expression */
773+
/* ExecInitExprwill assume we've planned the expression */
766774
check_expr=expression_planner(check_expr);
767775

768776
r=makeNode(DomainConstraintState);
769777
r->constrainttype=DOM_CONSTRAINT_CHECK;
770778
r->name=pstrdup(NameStr(c->conname));
771-
r->check_expr=ExecInitExpr(check_expr,NULL);
779+
/* Must cast here because we're not storing an expr state node */
780+
r->check_expr= (ExprState*)check_expr;
772781

773782
MemoryContextSwitchTo(oldcxt);
774783

@@ -913,6 +922,40 @@ dccref_deletion_callback(void *arg)
913922
}
914923
}
915924

925+
/*
926+
* prep_domain_constraints --- prepare domain constraints for execution
927+
*
928+
* The expression trees stored in the DomainConstraintCache's list are
929+
* converted to executable expression state trees stored in execctx.
930+
*/
931+
staticList*
932+
prep_domain_constraints(List*constraints,MemoryContextexecctx)
933+
{
934+
List*result=NIL;
935+
MemoryContextoldcxt;
936+
ListCell*lc;
937+
938+
oldcxt=MemoryContextSwitchTo(execctx);
939+
940+
foreach(lc,constraints)
941+
{
942+
DomainConstraintState*r= (DomainConstraintState*)lfirst(lc);
943+
DomainConstraintState*newr;
944+
945+
newr=makeNode(DomainConstraintState);
946+
newr->constrainttype=r->constrainttype;
947+
newr->name=r->name;
948+
/* Must cast here because cache items contain expr plan trees */
949+
newr->check_expr=ExecInitExpr((Expr*)r->check_expr,NULL);
950+
951+
result=lappend(result,newr);
952+
}
953+
954+
MemoryContextSwitchTo(oldcxt);
955+
956+
returnresult;
957+
}
958+
916959
/*
917960
* InitDomainConstraintRef --- initialize a DomainConstraintRef struct
918961
*
@@ -926,6 +969,7 @@ InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref,
926969
/* Look up the typcache entry --- we assume it survives indefinitely */
927970
ref->tcache=lookup_type_cache(type_id,TYPECACHE_DOMAIN_INFO);
928971
/* For safety, establish the callback before acquiring a refcount */
972+
ref->refctx=refctx;
929973
ref->dcc=NULL;
930974
ref->callback.func=dccref_deletion_callback;
931975
ref->callback.arg= (void*)ref;
@@ -935,7 +979,8 @@ InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref,
935979
{
936980
ref->dcc=ref->tcache->domainData;
937981
ref->dcc->dccRefCount++;
938-
ref->constraints=ref->dcc->constraints;
982+
ref->constraints=prep_domain_constraints(ref->dcc->constraints,
983+
ref->refctx);
939984
}
940985
else
941986
ref->constraints=NIL;
@@ -969,6 +1014,14 @@ UpdateDomainConstraintRef(DomainConstraintRef *ref)
9691014

9701015
if (dcc)
9711016
{
1017+
/*
1018+
* Note: we just leak the previous list of executable domain
1019+
* constraints. Alternatively, we could keep those in a child
1020+
* context of ref->refctx and free that context at this point.
1021+
* However, in practice this code path will be taken so seldom
1022+
* that the extra bookkeeping for a child context doesn't seem
1023+
* worthwhile; we'll just allow a leak for the lifespan of refctx.
1024+
*/
9721025
ref->constraints=NIL;
9731026
ref->dcc=NULL;
9741027
decr_dcc_refcount(dcc);
@@ -978,7 +1031,8 @@ UpdateDomainConstraintRef(DomainConstraintRef *ref)
9781031
{
9791032
ref->dcc=dcc;
9801033
dcc->dccRefCount++;
981-
ref->constraints=dcc->constraints;
1034+
ref->constraints=prep_domain_constraints(dcc->constraints,
1035+
ref->refctx);
9821036
}
9831037
}
9841038
}

‎src/include/utils/typcache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ typedef struct TypeCacheEntry
130130
typedefstructDomainConstraintRef
131131
{
132132
List*constraints;/* list of DomainConstraintState nodes */
133+
MemoryContextrefctx;/* context holding DomainConstraintRef */
133134

134135
/* Management data --- treat these fields as private to typcache.c */
135136
TypeCacheEntry*tcache;/* owning typcache entry */

‎src/test/regress/expected/domain.out

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,31 @@ select dom_check(0);
682682
drop function dom_check(int);
683683
drop domain di;
684684
--
685+
-- Check use of a (non-inline-able) SQL function in a domain constraint;
686+
-- this has caused issues in the past
687+
--
688+
create function sql_is_distinct_from(anyelement, anyelement)
689+
returns boolean language sql
690+
as 'select $1 is distinct from $2 limit 1';
691+
create domain inotnull int
692+
check (sql_is_distinct_from(value, null));
693+
select 1::inotnull;
694+
inotnull
695+
----------
696+
1
697+
(1 row)
698+
699+
select null::inotnull;
700+
ERROR: value for domain inotnull violates check constraint "inotnull_check"
701+
create table dom_table (x inotnull);
702+
insert into dom_table values ('1');
703+
insert into dom_table values (1);
704+
insert into dom_table values (null);
705+
ERROR: value for domain inotnull violates check constraint "inotnull_check"
706+
drop table dom_table;
707+
drop domain inotnull;
708+
drop function sql_is_distinct_from(anyelement, anyelement);
709+
--
685710
-- Renaming
686711
--
687712
create domain testdomain1 as int;

‎src/test/regress/sql/domain.sql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,30 @@ drop function dom_check(int);
515515

516516
dropdomain di;
517517

518+
--
519+
-- Check use of a (non-inline-able) SQL function in a domain constraint;
520+
-- this has caused issues in the past
521+
--
522+
523+
createfunctionsql_is_distinct_from(anyelement, anyelement)
524+
returnsboolean language sql
525+
as'select $1 is distinct from $2 limit 1';
526+
527+
createdomaininotnullint
528+
check (sql_is_distinct_from(value,null));
529+
530+
select1::inotnull;
531+
selectnull::inotnull;
532+
533+
createtabledom_table (x inotnull);
534+
insert into dom_tablevalues ('1');
535+
insert into dom_tablevalues (1);
536+
insert into dom_tablevalues (null);
537+
538+
droptable dom_table;
539+
dropdomain inotnull;
540+
dropfunction sql_is_distinct_from(anyelement, anyelement);
541+
518542
--
519543
-- Renaming
520544
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp