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

Commitc684898

Browse files
committed
Fix domain_in() bug exhibited by Darcy Buskermolen. The idea of an EState
that's shorter-lived than the expression state being evaluated in it reallydoesn't work :-( --- we end up with fn_extra caches getting deleted whilestill in use. Rather than abandon the notion of caching expression stateacross domain_in calls altogether, I chose to make domain_in a bit cozierwith ExprContext. All we really need for evaluating variable-freeexpressions is an ExprContext, not an EState, so I invented the notion of a"standalone" ExprContext. domain_in can prevent resource leakages by doinga ReScanExprContext on this rather than having to free it entirely; so wecan make the ExprContext have the same lifespan (and particularly the sameper_query memory context) as the expression state structs.
1 parentbf7b205 commitc684898

File tree

4 files changed

+105
-32
lines changed

4 files changed

+105
-32
lines changed

‎src/backend/executor/execUtils.c

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.138 2006/07/31 20:09:04 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.139 2006/08/04 21:33:36 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -17,6 +17,7 @@
1717
*CreateExecutorStateCreate/delete executor working state
1818
*FreeExecutorState
1919
*CreateExprContext
20+
*CreateStandaloneExprContext
2021
*FreeExprContext
2122
*ReScanExprContext
2223
*
@@ -331,6 +332,68 @@ CreateExprContext(EState *estate)
331332
returnecontext;
332333
}
333334

335+
/* ----------------
336+
*CreateStandaloneExprContext
337+
*
338+
*Create a context for standalone expression evaluation.
339+
*
340+
* An ExprContext made this way can be used for evaluation of expressions
341+
* that contain no Params, subplans, or Var references (it might work to
342+
* put tuple references into the scantuple field, but it seems unwise).
343+
*
344+
* The ExprContext struct is allocated in the caller's current memory
345+
* context, which also becomes its "per query" context.
346+
*
347+
* It is caller's responsibility to free the ExprContext when done,
348+
* or at least ensure that any shutdown callbacks have been called
349+
* (ReScanExprContext() is suitable). Otherwise, non-memory resources
350+
* might be leaked.
351+
* ----------------
352+
*/
353+
ExprContext*
354+
CreateStandaloneExprContext(void)
355+
{
356+
ExprContext*econtext;
357+
358+
/* Create the ExprContext node within the caller's memory context */
359+
econtext=makeNode(ExprContext);
360+
361+
/* Initialize fields of ExprContext */
362+
econtext->ecxt_scantuple=NULL;
363+
econtext->ecxt_innertuple=NULL;
364+
econtext->ecxt_outertuple=NULL;
365+
366+
econtext->ecxt_per_query_memory=CurrentMemoryContext;
367+
368+
/*
369+
* Create working memory for expression evaluation in this context.
370+
*/
371+
econtext->ecxt_per_tuple_memory=
372+
AllocSetContextCreate(CurrentMemoryContext,
373+
"ExprContext",
374+
ALLOCSET_DEFAULT_MINSIZE,
375+
ALLOCSET_DEFAULT_INITSIZE,
376+
ALLOCSET_DEFAULT_MAXSIZE);
377+
378+
econtext->ecxt_param_exec_vals=NULL;
379+
econtext->ecxt_param_list_info=NULL;
380+
381+
econtext->ecxt_aggvalues=NULL;
382+
econtext->ecxt_aggnulls=NULL;
383+
384+
econtext->caseValue_datum= (Datum)0;
385+
econtext->caseValue_isNull= true;
386+
387+
econtext->domainValue_datum= (Datum)0;
388+
econtext->domainValue_isNull= true;
389+
390+
econtext->ecxt_estate=NULL;
391+
392+
econtext->ecxt_callbacks=NULL;
393+
394+
returnecontext;
395+
}
396+
334397
/* ----------------
335398
*FreeExprContext
336399
*
@@ -352,9 +415,11 @@ FreeExprContext(ExprContext *econtext)
352415
ShutdownExprContext(econtext);
353416
/* And clean up the memory used */
354417
MemoryContextDelete(econtext->ecxt_per_tuple_memory);
355-
/* Unlink self from owning EState */
418+
/* Unlink self from owning EState, if any */
356419
estate=econtext->ecxt_estate;
357-
estate->es_exprcontexts=list_delete_ptr(estate->es_exprcontexts,econtext);
420+
if (estate)
421+
estate->es_exprcontexts=list_delete_ptr(estate->es_exprcontexts,
422+
econtext);
358423
/* And delete the ExprContext node */
359424
pfree(econtext);
360425
}

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

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,21 @@
1111
*
1212
* The overhead required for constraint checking can be high, since examining
1313
* the catalogs to discover the constraints for a given domain is not cheap.
14-
* We havetwo mechanisms for minimizing this cost:
14+
* We havethree mechanisms for minimizing this cost:
1515
*1.In a nest of domains, we flatten the checking of all the levels
1616
*into just one operation.
1717
*2.We cache the list of constraint items in the FmgrInfo struct
1818
*passed by the caller.
19-
*
20-
* We also have to create an EState to evaluate CHECK expressions in.
21-
* Creating and destroying an EState is somewhat expensive, and so it's
22-
* tempting to cache the EState too. However, that would mean that the
23-
* EState never gets an explicit FreeExecutorState call, which is a bad idea
24-
* because it risks leaking non-memory resources.
19+
*3.If there are CHECK constraints, we cache a standalone ExprContext
20+
*to evaluate them in.
2521
*
2622
*
2723
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
2824
* Portions Copyright (c) 1994, Regents of the University of California
2925
*
3026
*
3127
* IDENTIFICATION
32-
* $PostgreSQL: pgsql/src/backend/utils/adt/domains.c,v 1.2 2006/07/14 14:52:24 momjian Exp $
28+
* $PostgreSQL: pgsql/src/backend/utils/adt/domains.c,v 1.3 2006/08/04 21:33:36 tgl Exp $
3329
*
3430
*-------------------------------------------------------------------------
3531
*/
@@ -54,6 +50,10 @@ typedef struct DomainIOData
5450
FmgrInfoproc;
5551
/* List of constraint items to check */
5652
List*constraint_list;
53+
/* Context for evaluating CHECK constraints in */
54+
ExprContext*econtext;
55+
/* Memory context this cache is in */
56+
MemoryContextmcxt;
5757
}DomainIOData;
5858

5959

@@ -95,6 +95,10 @@ domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary,
9595
my_extra->constraint_list=GetDomainConstraints(domainType);
9696
MemoryContextSwitchTo(oldcontext);
9797

98+
/* We don't make an ExprContext until needed */
99+
my_extra->econtext=NULL;
100+
my_extra->mcxt=mcxt;
101+
98102
/* Mark cache valid */
99103
my_extra->domain_type=domainType;
100104
}
@@ -107,7 +111,7 @@ domain_state_setup(DomainIOData *my_extra, Oid domainType, bool binary,
107111
staticvoid
108112
domain_check_input(Datumvalue,boolisnull,DomainIOData*my_extra)
109113
{
110-
EState*estate=NULL;
114+
ExprContext*econtext=my_extra->econtext;
111115
ListCell*l;
112116

113117
foreach(l,my_extra->constraint_list)
@@ -125,25 +129,26 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra)
125129
break;
126130
caseDOM_CONSTRAINT_CHECK:
127131
{
128-
ExprContext*econtext;
129132
DatumconResult;
130133
boolconIsNull;
131-
Datumsave_datum;
132-
boolsave_isNull;
133134

134-
if (estate==NULL)
135-
estate=CreateExecutorState();
136-
econtext=GetPerTupleExprContext(estate);
135+
/* Make the econtext if we didn't already */
136+
if (econtext==NULL)
137+
{
138+
MemoryContextoldcontext;
139+
140+
oldcontext=MemoryContextSwitchTo(my_extra->mcxt);
141+
econtext=CreateStandaloneExprContext();
142+
MemoryContextSwitchTo(oldcontext);
143+
my_extra->econtext=econtext;
144+
}
137145

138146
/*
139147
* Set up value to be returned by CoerceToDomainValue
140-
* nodes.We must save and restore prior setting of
141-
*econtext's domainValue fields, in case this node is
142-
*itself within a check expression for another domain.
148+
* nodes. Unlike ExecEvalCoerceToDomain, this econtext
149+
*couldn't be shared with anything else, so no need
150+
*to save and restore fields.
143151
*/
144-
save_datum=econtext->domainValue_datum;
145-
save_isNull=econtext->domainValue_isNull;
146-
147152
econtext->domainValue_datum=value;
148153
econtext->domainValue_isNull=isnull;
149154

@@ -158,9 +163,6 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra)
158163
errmsg("value for domain %s violates check constraint \"%s\"",
159164
format_type_be(my_extra->domain_type),
160165
con->name)));
161-
econtext->domainValue_datum=save_datum;
162-
econtext->domainValue_isNull=save_isNull;
163-
164166
break;
165167
}
166168
default:
@@ -170,8 +172,13 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra)
170172
}
171173
}
172174

173-
if (estate)
174-
FreeExecutorState(estate);
175+
/*
176+
* Before exiting, call any shutdown callbacks and reset econtext's
177+
* per-tuple memory. This avoids leaking non-memory resources,
178+
* if anything in the expression(s) has any.
179+
*/
180+
if (econtext)
181+
ReScanExprContext(econtext);
175182
}
176183

177184

‎src/include/executor/executor.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.127 2006/06/16 18:42:23 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.128 2006/08/04 21:33:36 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -224,6 +224,7 @@ extern void end_tup_output(TupOutputState *tstate);
224224
externEState*CreateExecutorState(void);
225225
externvoidFreeExecutorState(EState*estate);
226226
externExprContext*CreateExprContext(EState*estate);
227+
externExprContext*CreateStandaloneExprContext(void);
227228
externvoidFreeExprContext(ExprContext*econtext);
228229
externvoidReScanExprContext(ExprContext*econtext);
229230

‎src/include/nodes/execnodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.157 2006/08/02 18:58:21 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.158 2006/08/04 21:33:36 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -118,7 +118,7 @@ typedef struct ExprContext
118118
DatumdomainValue_datum;
119119
booldomainValue_isNull;
120120

121-
/* Link to containing EState */
121+
/* Link to containing EState(NULL if a standalone ExprContext)*/
122122
structEState*ecxt_estate;
123123

124124
/* Functions to call back when ExprContext is shut down */

0 commit comments

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