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
signature.asc
Description: PGP signature
