Re: Do we really need to switch to per-tuple memory context in ATRewriteTable() when Table Rewrite is not happening

On Wed, Jan 3, 2018 at 8:06 PM, Andrew Gierth
<andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>>>>>> "Ashutosh" == Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> writes:
>
> Ashutosh> Hi All,
>
> Ashutosh> Today while trying to understand the code for ALTER TABLE in
> Ashutosh> PostgreSQL (basically the table rewrite part), I noticed that
> Ashutosh> we are switching to a per-tuple memory context even when
> Ashutosh> table rewrite is not required. For e.g.. consider the case
> Ashutosh> where we do ADD CONSTRAINTS (NOT NULL or CHECK) using ALTER
> Ashutosh> TABLE command. In this case, we just scan the tuple and
> Ashutosh> verify it for the given constraint instead of forming a new
> Ashutosh> tuple. So, not sure why do we switch to per-tuple memory
> Ashutosh> context when just adding the constraint.
>
> What makes you think that evaluating the constraint won't allocate
> memory?
>
> Switching contexts is virtually free (just an assignment to a global
> var). Resetting a context that's not been allocated in since its last
> reset is also virtually free (just checks a flag). In contrast, every
> single function (except special cases like index comparators) is free to
> allocate memory in its caller's context, for temporary use and for
> returning the result; how else would a condition like
> CHECK(substring(col for 3)='FOO') work, without allocating memory for
> the substring result?
>

I am not saying that we do not do memory allocation for constraints.
We do that in some cases as the one you mentioned above, but, for that
we are already doing context switching inside ExecCheck().

ExecCheck()--> ExecEvalExprSwitchContext()->MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory)...state->evalfunc(state,
econtext, isNull) ...MemoryContextSwitchTo(oldContext);

So, my point is, why are we doing an extra context switching inside
ATRewriteTable() for constraints,

/*
* Switch to per-tuple memory context and reset it for each tuple
* produced, so we don't leak memory.
*/
oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

> --
> Andrew (irc:RhodiumToad)