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
