Re: Proposal: Local indexes for partitioned table

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Pg Hackers <pgsql-hackers@postgresql.org>, Maksim Milyutin <milyutinma@gmail.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Date: 2017-10-27T21:25:43Z
Lists: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Oct 27, 2017 at 7:27 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> I noticed that RelationBuildPartitionKey is generating a partition key
>> in a temp context, then creating a private context and copying the key
>> into that.  That seems leftover from some previous iteration of some
>> other patch; I think it's pretty reasonable to create the new context
>> right from the start and allocate the key there directly instead.  Then
>> there's no need for copy_partition_key at all.

> We could do that, but the motivation for the current system was to
> avoid leaking memory in a long-lived context.  I think the same
> technique is used elsewhere for similar reasons.  I admit I haven't
> checked whether there would actually be a leak here if we did it as
> you propose, but I wouldn't find it at all surprising.

Another key point is to avoid leaving a corrupted relcache entry behind
if you fail partway through.  I think that the current coding is
RelationBuildPartitionKey is safe, although it's far too undercommented
for my taste.  (The ordering of the last few steps is *critical*.)

It might work to build the new key in a context that's initially a
child of CurrentMemoryContext, then reparent it to be a child of
CacheMemoryContext when done.  But you'd have to look at whether that
would end up wasting long-lived memory, if the build process generates
any cruft in the same context.

			regards, tom lane


Commits

  1. Local partitioned indexes

  2. Fix StoreCatalogInheritance1 to use 32bit inhseqno

  3. Get rid of copy_partition_key

  4. Simplify index_[constraint_]create API