Обсуждение: Why MemoryContextSwitch in ExecRelCheck ?

Can anybody explain the following code detail ?

A comment in execMain.c tells us:

--start code snippet--* NB: the CurrentMemoryContext when this is called must be the context* to be used as the
per-querycontext for the query plan.      ExecutorRun()* and ExecutorEnd() must be called in this same memory context.*
----------------------------------------------------------------*/
TupleDesc
ExecutorStart(QueryDesc *queryDesc, EState *estate)
--end code snippet--

Nevertheless in ExecRelCheck a context switch to per-query memory
context is made:

--start code snippet--       /*        * If first time through for this result relation, build expression        *
nodetreesfor rel's constraint expressions.  Keep them in the        * per-query memory context so they'll survive
throughoutthe query.        */       if (resultRelInfo->ri_ConstraintExprs == NULL)       {               oldContext =
MemoryContextSwitchTo(estate->es_query_cxt);              resultRelInfo->ri_ConstraintExprs =
(List**) palloc(ncheck * sizeof(List *));               for (i = 0; i < ncheck; i++)               {
  qual = (List *) stringToNode(check[i].ccbin);                       resultRelInfo->ri_ConstraintExprs[i] = qual;
        }               MemoryContextSwitchTo(oldContext);       }
 
--end code snippet--

Is this a switch from per-query memory context to per-query memory
context, hence not necessary, or do I miss something ?

-- 
Holger Krug
hkrug@rationalizer.com

Holger Krug <hkrug@rationalizer.com> writes:
> Nevertheless in ExecRelCheck a context switch to per-query memory
> context is made:
> Is this a switch from per-query memory context to per-query memory
> context, hence not necessary, or do I miss something ?

[ thinks ... ]  It might be unnecessary.  I'm not convinced that the
per-query context would always be the active one when ExecRelCheck is
called, however.  There are various per-tuple contexts that might be
used as well.

MemoryContextSwitchTo() is cheap enough that I prefer to call it when
there's any doubt, rather than build a routine that will fail silently
if it's called in the wrong context.  There are two typical scenarios
for routines that are building data structures that will outlive the
routine's execution:

1. Data structure is to be returned to the caller.  In this case the
caller is responsible for identifying the context to allocate the data
structure in, either explicitly or by passing it as the current context.

2. Data structure is owned and managed by the routine, which must know
which context it's supposed to live in.  In these cases I think the
routine ought always to explicitly switch to that context, not assume
that it's being called in that context.

I've been trying to migrate away from running with CurrentMemoryContext
set to anything longer-lived than a per-tuple context, though the
project is by no means complete.
        regards, tom lane

On Mon, Jan 07, 2002 at 12:28:12PM -0500, Tom Lane wrote:
> Holger Krug <hkrug@rationalizer.com> writes:
> > Nevertheless in ExecRelCheck a context switch to per-query memory
> > context is made:
> > Is this a switch from per-query memory context to per-query memory
> > context, hence not necessary, or do I miss something ?
> 
> [ thinks ... ]  It might be unnecessary.  I'm not convinced that the
> per-query context would always be the active one when ExecRelCheck is
> called, however.  There are various per-tuple contexts that might be
> used as well.

I think, there aren't, but nevertheless, you're principles stated
below are convincing.
> MemoryContextSwitchTo() is cheap enough that I prefer to call it when
> there's any doubt, rather than build a routine that will fail silently
> if it's called in the wrong context.  There are two typical scenarios
> for routines that are building data structures that will outlive the
> routine's execution:
> 
> 1. Data structure is to be returned to the caller.  In this case the
> caller is responsible for identifying the context to allocate the data
> structure in, either explicitly or by passing it as the current context.
> 
> 2. Data structure is owned and managed by the routine, which must know
> which context it's supposed to live in. In these cases I think the
> routine ought always to explicitly switch to that context, not assume
> that it's being called in that context.

OK. I wondered, because this is not done for the trigger related
cache, but only for the check related cache. Now I understand, it's
work in progress. (I think, very good work, indeed, because the code
is astonishingly well readable.)


-- 
Holger Krug
hkrug@rationalizer.com

Holger Krug <hkrug@rationalizer.com> writes:
>> [ thinks ... ]  It might be unnecessary.  I'm not convinced that the
>> per-query context would always be the active one when ExecRelCheck is
>> called, however.  There are various per-tuple contexts that might be
>> used as well.

> I think, there aren't,

Right now, it might well be the case that ExecRelCheck is always called
in the per-query context.  The point I was trying to make is that I'd
like to change the code so that we don't run so much code with current
context set to per-query context; at which point ExecRelCheck will fail
if it hasn't got that MemoryContextSwitchTo.  So, yeah, it's work in
progress.
        regards, tom lane