Обсуждение: Do we really need to switch to per-tuple memory context inATRewriteTable() when Table Rewrite is not happening
Hi All, Today while trying to understand the code for ALTER TABLE in PostgreSQL (basically the table rewrite part), I noticed that we are switching to a per-tuple memory context even when table rewrite is not required. For e.g.. consider the case where we do ADD CONSTRAINTS (NOT NULL or CHECK) using ALTER TABLE command. In this case, we just scan the tuple and verify it for the given constraint instead of forming a new tuple. So, not sure why do we switch to per-tuple memory context when just adding the constraint. Could someone please let me know the reason for doing so. Thanks in advance. I am basically talking about the following lines of code in ATRewriteTable() function. /* * Switch to per-tuple memory context and reset it for each tuple * produced, so we don't leak memory. */ oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); AFAICU, we should have done that only when 'tab->rewrite > 0' is true or may be when 'OIDNewHeap' is valid. Here are the steps that i have followed to understand ATRewriteTable(), CREATE TABLE tmp (initial int4); INSERT INTO tmp VALUES(10); INSERT INTO tmp VALUES(20); ALTER TABLE tmp ADD CONSTRAINT check_cons CHECK (initial > 5); ALTER TABLE tmp ALTER COLUMN initial SET NOT NULL; -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com
Ashutosh Sharma wrote: > I am basically talking about the following lines of code in > ATRewriteTable() function. > > /* > * Switch to per-tuple memory context and reset it for each tuple > * produced, so we don't leak memory. > */ > oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); Perhaps a memory context switch is so cheap that adding a branch to verify whether it's needed is more expensive than just doing it all the time. You could prove me wrong by measuring it. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>>>> "Ashutosh" == Ashutosh Sharma <ashu.coek88@gmail.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? -- Andrew (irc:RhodiumToad)
On Wed, Jan 3, 2018 at 7:25 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > Ashutosh Sharma wrote: > >> I am basically talking about the following lines of code in >> ATRewriteTable() function. >> >> /* >> * Switch to per-tuple memory context and reset it for each tuple >> * produced, so we don't leak memory. >> */ >> oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); > > Perhaps a memory context switch is so cheap that adding a branch to > verify whether it's needed is more expensive than just doing it all the > time. You could prove me wrong by measuring it. > might be...I'm not sure about the cost of context switching. As you said, it is quite possible that adding a check to verify whether switching is required or not could be more expensive than doing the context switching itself. -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Jan 3, 2018 at 8:06 PM, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: >>>>>> "Ashutosh" == Ashutosh Sharma <ashu.coek88@gmail.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)