Re: Should new partitions inherit their tablespace from their parent?

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Should new partitions inherit their tablespace from their parent?
Date: 2018-12-11 02:43:16
Message-ID: 20181211024316.GE1473@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 10, 2018 at 10:56:47PM +0900, Michael Paquier wrote:
> Cool, thanks for updating the assertion. At quick glance the patch
> seems fine to me. Let's keep ATExecSetTableSpaceNoStorage as routine
> name as you suggest then.

+ /*
+ * For partitions, when no other tablespace is specified, we default
+ * the tablespace to the parent partitioned table's.
+ */
+ Assert(list_length(stmt->inhRelations) == 1);
+
+ parent = linitial(stmt->inhRelations);
+
+ parentrel = heap_openrv(parent, AccessExclusiveLock);
So, in order to determine which tablespace should be used here, an
exclusive lock is taken on the parent because its partition descriptor
gets updated by the addition of the new partition. This process is
actually done already in MergeAttributes() as well, but it won't get
triggered if a tablespace is defined directly in the CREATE TABLE
statement. I think that we should add a comment to explain the
dependency between both, as well as why an exclusive lock is needed, so
something among those lines perhaps? Here is an idea:
+ /*
+ * Grab an exclusive lock on the parent because its partition
+ * descriptor will be changed by the addition of the new partition.
+ * The same lock level is taken as when merging attributes below
+ * in MergeAttributes() to protect from lock upgrade deadlocks.
+ */

The position where the tablespace is chosen is definitely the good one.

What do you think?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-12-11 04:19:18 Re: Thinking about EXPLAIN ALTER TABLE
Previous Message Robert Haas 2018-12-11 02:25:12 Re: Thinking about EXPLAIN ALTER TABLE