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

From: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Do we really need to switch to per-tuple memory context in ATRewriteTable() when Table Rewrite is not happening
Date: 2018-01-03 08:52:14
Message-ID: CAE9k0P=1R1OfCp3hUMs1=5CV0TB5xM9J1+mntPjKuVgFvcGG=g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Cyril Auburtin 2018-01-03 09:05:29 tsquery pre-parser
Previous Message Andres Freund 2018-01-03 08:46:44 Re: copy_file_range is now a Linux kernel call