Date: Fri, 5 Oct 2018 13:22:36 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@postgresql.org>
Subject: Assertion failure with ALTER TABLE ATTACH PARTITION with
 log_min_messages >= DEBUG1
Message-ID: <20181005042236.GG1629@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="aznLbwQ42o7LEaqN"
Content-Disposition: inline
Precedence: bulk


--aznLbwQ42o7LEaqN
Content-Type: multipart/mixed; boundary="fckbADODYWZD5TdN"
Content-Disposition: inline


--fckbADODYWZD5TdN
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi all,

Running installcheck on an instance with log_min_messages =3D DEBUG1, I
can bump into the following assertion failure:
#2  0x000056145231e82c in ExceptionalCondition
(conditionName=3D0x56145258ae0b "!(strvalue !=3D ((void *)0))",
errorType=3D0x56145258adfb "FailedAssertion",
    fileName=3D0x56145258adf0 "snprintf.c", lineNumber=3D440) at assert.c:54
[...]
#7  0x000056145231f518 in errmsg (fmt=3D0x5614524dac60 "validating foreign
key constraint \"%s\"") at elog.c:796
#8  0x0000561451f6ab54 in validateForeignKeyConstraint (conname=3D0x0,
rel=3D0x7f12833ca750, pkrel=3D0x7f12833cc468, pkindOid=3D36449,
constraintOid=3D36466) at tablecmds.c:8566
#9  0x0000561451f61589 in ATRewriteTables (parsetree=3D0x561453bde5e0,
wqueue=3D0x7ffe8f1d55e8, lockmode=3D8) at tablecmds.c:4549

Looking at the stack trace there is this log in
validateForeignKeyConstraint:
ereport(DEBUG1,
        (errmsg("validating foreign key constraint \"%s\"", conname)));

However conname is set to NULL in this code path.

This test case allows to reproduce easily the failure:
CREATE TABLE fk_notpartitioned_pk (a int, b int, PRIMARY KEY (a, b));
CREATE TABLE fk_partitioned_fk (b int, a int) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_fk_1 (b int, a int);
ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES
fk_notpartitioned_pk;
-- crash
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_1 FOR
  VALUES FROM (1,1) TO (2,2);

=46rom what I can see the problem comes from CloneForeignKeyConstraint
which forgets to assign the constraint name when cloning the FK
definition.  While looking at the ATTACH PARTITION code, I have noticed
that a variable gets overridden, which is in my opinion bad style.  So
the problem is rather close to what Tom has fixed in 3d0f68dd it seems.

Attached is a patch for all that, with which installcheck-world passes
for me.  I am surprised this was not noticed before, the recent snprintf
stanza is nicely helping, and this would need to be back-patched down to
v11.

Thanks,
--
Michael

--fckbADODYWZD5TdN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="attach-partition-assert.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_c=
onstraint.c
index 6781b00c6e..2063abb8ae 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -574,6 +574,7 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId=
, List **cloned)
=20
 		fkconstraint =3D makeNode(Constraint);
 		/* for now this is all we need */
+		fkconstraint->conname =3D pstrdup(NameStr(constrForm->conname));
 		fkconstraint->fk_upd_action =3D constrForm->confupdtype;
 		fkconstraint->fk_del_action =3D constrForm->confdeltype;
 		fkconstraint->deferrable =3D constrForm->condeferrable;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablec=
mds.c
index c145385f84..7df1fc2a76 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14275,21 +14275,21 @@ ATExecAttachPartition(List **wqueue, Relation rel=
, PartitionCmd *cmd)
 							   RelationGetRelid(attachrel), &cloned);
 	foreach(l, cloned)
 	{
-		ClonedConstraint *cloned =3D lfirst(l);
+		ClonedConstraint *clonedcon =3D lfirst(l);
 		NewConstraint *newcon;
 		Relation	clonedrel;
 		AlteredTableInfo *parttab;
=20
-		clonedrel =3D relation_open(cloned->relid, NoLock);
+		clonedrel =3D relation_open(clonedcon->relid, NoLock);
 		parttab =3D ATGetQueueEntry(wqueue, clonedrel);
=20
 		newcon =3D (NewConstraint *) palloc0(sizeof(NewConstraint));
-		newcon->name =3D cloned->constraint->conname;
+		newcon->name =3D clonedcon->constraint->conname;
 		newcon->contype =3D CONSTR_FOREIGN;
-		newcon->refrelid =3D cloned->refrelid;
-		newcon->refindid =3D cloned->conindid;
-		newcon->conid =3D cloned->conid;
-		newcon->qual =3D (Node *) cloned->constraint;
+		newcon->refrelid =3D clonedcon->refrelid;
+		newcon->refindid =3D clonedcon->conindid;
+		newcon->conid =3D clonedcon->conid;
+		newcon->qual =3D (Node *) clonedcon->constraint;
=20
 		parttab->constraints =3D lappend(parttab->constraints, newcon);
=20

--fckbADODYWZD5TdN--

--aznLbwQ42o7LEaqN
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlu25wwACgkQnvQgOdby
QH0KWA//X971NADjRcEEYDXocdgtNSpC3ChFlS6zAtLGbs2V1QNWBok4jtycC9yU
k7FiKNKUkvcxURDSojjkqUEJnBeURzfofwjjZqGBzaua1XzypYRfZTdFhIE4v7NW
TO7RcT9E1hy7UxtKMt746ENPO8NDCWG+aHUjSBmdGkljUgjzMUwBGHJZeVRy+fIS
A3w94t8ShjKQ2uSIZJHy4uqT8E+HsjZW7DyYrsXZD9Rg8aidl8b8TVMl/fBnMKLU
fHbWLymf+UDUyF4bZs1hS4WFYULUokQ0QAXuR67Bb7HES7ckEcyprv17JGVuI6rj
GPu0rXdPBzQW2c87xyjnRLZ230Syak5MHdHirhIJcDbJ3XvAdTCF2tyW1mc0BWQl
Iwlms4nNPiSQS0q1/+7arSHVlSvv7lsqEWnDNraFTTweFLthJn+QzbAfXSCm7xzD
VKnAcU769g5xry+yhLpIWjbFd9seCRjlup8UL1B/Ex/29dSWa4++Gn1vi4gstu96
09ug1G4Fds1QUFIoBW8Vc2YwIGi1KYVfg72qNVsnVAtuwvXuuXqeUn+ck1Zp5YcE
vncVTsP2E1UhLzSojkhqzqhuCpIcty7M6K1AlMoo3PcBr8UZvNBAakyD/xhIG4dp
9TL3uHq2QuravNe5iSwhplBtMdChNiiMQwqKSU5LRxkGNFDjwAY=
=WhCb
-----END PGP SIGNATURE-----

--aznLbwQ42o7LEaqN--