Re: Commit 4dba331cb3 broke ATTACH PARTITION behaviour. - Mailing list pgsql-hackers

At Thu, 29 Mar 2018 13:04:06 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in
<20180329160406.ii2wgbkmlnfxtwbt@alvherre.pgsql>
> Rushabh Lathia wrote:
> > On Thu, Mar 29, 2018 at 7:46 PM, Alvaro Herrera <alvherre@alvh.no-ip.org>
> > wrote:
> 
> > > Hmm, offhand I don't quite see why this error fails to be thrown.
> >
> > ATTACH PARTITION should throw an error, because partition table "foo"
> > already have two partition with key values (1, 2,3 4). And table "foo_d"
> > which we are attaching here has :
> 
> Oh, I understand how it's supposed to work.  I was just saying I don't
> understand how this bug occurs.  Is it because we fail to determine the
> correct partition constraint for the default partition in time for its
> verification scan?

The reason is that CommandCounterIncrement added in
StorePartitionBound reveals the just added default partition to
get_default_oid_from_partdesc too early.  The revealed partition
has immature constraint and it overrites the right constraint
generated just above.

ATExecAttachPartition checks for default partition oid twice but
the second is just needless before the commit and harms after it.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8f83aa4675..96b62bd4b6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14082,11 +14082,9 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
     }
 
     /*
-     * Check whether default partition has a row that would fit the partition
-     * being attached.
+     * Check whether existing default partition has a row that would fit the
+     * partition being attached.
      */
-    defaultPartOid =
-        get_default_oid_from_partdesc(RelationGetPartitionDesc(rel));
     if (OidIsValid(defaultPartOid))
     {
         Relation    defaultrel;