Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gVQJX-0000vs-DV for pgsql-hackers@arkaria.postgresql.org; Sat, 08 Dec 2018 00:17:47 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gVQJV-0002ZO-5Q for pgsql-hackers@arkaria.postgresql.org; Sat, 08 Dec 2018 00:17:45 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gVQJU-0002ZH-MZ for pgsql-hackers@lists.postgresql.org; Sat, 08 Dec 2018 00:17:44 +0000 Received: from tamriel.snowman.net ([96.255.250.162]) by makus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gVQJM-0006Mm-UW for pgsql-hackers@postgresql.org; Sat, 08 Dec 2018 00:17:42 +0000 Received: by tamriel.snowman.net (Postfix, from userid 1000) id 328255F7A5; Fri, 7 Dec 2018 19:17:36 -0500 (EST) Date: Fri, 7 Dec 2018 19:17:36 -0500 From: Stephen Frost To: Amit Langote Cc: Noah Misch , Amit Langote , Pg Hackers , Robert Haas , Tom Lane Subject: Re: pg_dump emits ALTER TABLE ONLY partitioned_table Message-ID: <20181208001735.GT3415@tamriel.snowman.net> References: <20170427141726.GF21223@tamriel.snowman.net> <696c2d04-80aa-57e2-c1c5-6df25e2bbed5@lab.ntt.co.jp> <20170503030520.GR21223@tamriel.snowman.net> <20170504214702.GP21223@tamriel.snowman.net> <20170506031749.GD843225@rfd.leadboat.com> <3e2aa1e9-e621-9ad5-b24d-e24dbc9650ba@lab.ntt.co.jp> <20170508034245.GY21223@tamriel.snowman.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="0BIv9nKhOLc5Ty2i" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0BIv9nKhOLc5Ty2i Content-Type: multipart/mixed; boundary="OjtEKJ7haQoFl5CE" Content-Disposition: inline --OjtEKJ7haQoFl5CE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Greetings, * Amit Langote (Langote_Amit_f8@lab.ntt.co.jp) wrote: > On 2017/05/08 12:42, Stephen Frost wrote: > > * Amit Langote (Langote_Amit_f8@lab.ntt.co.jp) wrote: > >> Thanks for committing the patch after improving it quite a bit, and so= rry > >> that I couldn't reply promptly during the last week due to vacation. > >=20 > > No worries, hopefully you have an opportunity to review the additional > > changes I made and understand why they were necessary. Certainly, feel > > free to reach out if you have any questions or notice anything else > > which should be improved. >=20 > Do you intend to push the other patch to add regression tests for the > non-inherited constraints? Here it is attached again for you to look ove= r. In a blast from the past, I had a long-lost note about trying to push this that I just stumbled over. Apologies for it getting lost in the cracks. I've rebased it and will plan to push it later this weekend, barring objections. Thanks! Stephen --OjtEKJ7haQoFl5CE Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="addl-partition-tests-v1_master.patch" Content-Transfer-Encoding: quoted-printable diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_du= mp.pl new file mode 100644 index 2afd950..8f4bb55 *** a/src/bin/pg_dump/t/002_pg_dump.pl --- b/src/bin/pg_dump/t/002_pg_dump.pl *************** my %tests =3D ( *** 2246,2257 **** create_order =3D> 91, create_sql =3D> 'CREATE TABLE dump_test_second_schema.measurement_y2006m2 ! PARTITION OF dump_test.measurement FOR VALUES ! FROM (\'2006-02-01\') TO (\'2006-03-01\');', regexp =3D> qr/^ \Q-- Name: measurement_y2006m2;\E.*\n \Q--\E\n\n ! \QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION O= F dump_test.measurement\E\n \QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n /xm, like =3D> { --- 2246,2261 ---- create_order =3D> 91, create_sql =3D> 'CREATE TABLE dump_test_second_schema.measurement_y2006m2 ! PARTITION OF dump_test.measurement ( ! unitsales DEFAULT 0 CHECK (unitsales >=3D 0) ! ) ! FOR VALUES FROM (\'2006-02-01\') TO (\'2006-03-01\');', regexp =3D> qr/^ \Q-- Name: measurement_y2006m2;\E.*\n \Q--\E\n\n ! \QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION O= F dump_test.measurement (\E\n ! \s+\QCONSTRAINT measurement_y2006m2_unitsales_check CHECK ((unitsales = >=3D 0))\E\n ! \)\n \QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n /xm, like =3D> { diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/= expected/create_table.out new file mode 100644 index b26b4e7..7e52c27 *** a/src/test/regress/expected/create_table.out --- b/src/test/regress/expected/create_table.out *************** CREATE TABLE part_b PARTITION OF parted *** 703,718 **** ) FOR VALUES IN ('b'); ERROR: column "b" specified more than once CREATE TABLE part_b PARTITION OF parted ( ! b NOT NULL DEFAULT 1 CHECK (b >=3D 0), ! CONSTRAINT check_a CHECK (length(a) > 0) ) FOR VALUES IN ('b'); NOTICE: merging constraint "check_a" with inherited definition ! -- conislocal should be false for any merged constraints ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass AND conname =3D 'check_a'; conislocal | coninhcount=20 ------------+------------- f | 1 ! (1 row) =20 -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('= c') PARTITION BY RANGE (c); --- 703,744 ---- ) FOR VALUES IN ('b'); ERROR: column "b" specified more than once CREATE TABLE part_b PARTITION OF parted ( ! b NOT NULL DEFAULT 1, ! CONSTRAINT check_a CHECK (length(a) > 0), ! CONSTRAINT check_b CHECK (b >=3D 0) ) FOR VALUES IN ('b'); NOTICE: merging constraint "check_a" with inherited definition ! -- conislocal should be false for any merged constraints, true otherwise ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass ORDER BY conislocal, coninhcount; conislocal | coninhcount=20 ------------+------------- f | 1 ! t | 0 ! (2 rows) !=20 ! -- Once check_b is added to the parent, it should be made non-local for p= art_b ! ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >=3D 0); ! NOTICE: merging constraint "check_b" with inherited definition ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass; ! conislocal | coninhcount=20 ! ------------+------------- ! f | 1 ! f | 1 ! (2 rows) !=20 ! -- Neither check_a nor check_b are droppable from part_b ! ALTER TABLE part_b DROP CONSTRAINT check_a; ! ERROR: cannot drop inherited constraint "check_a" of relation "part_b" ! ALTER TABLE part_b DROP CONSTRAINT check_b; ! ERROR: cannot drop inherited constraint "check_b" of relation "part_b" ! -- And dropping it from parted should leave no trace of them on part_b, u= nlike ! -- traditional inheritance where they will be left behind, because they w= ould ! -- be local constraints. ! ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b; ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass; ! conislocal | coninhcount=20 ! ------------+------------- ! (0 rows) =20 -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('= c') PARTITION BY RANGE (c); *************** drop table parted_collate_must_match; *** 757,765 **** b | integer | | not null | 1 | plain | = |=20 Partition of: parted FOR VALUES IN ('b') Partition constraint: ((a IS NOT NULL) AND (a =3D 'b'::text)) - Check constraints: - "check_a" CHECK (length(a) > 0) - "part_b_b_check" CHECK (b >=3D 0) =20 -- Both partition bound and partition key in describe output \d+ part_c --- 783,788 ---- *************** Check constraints: *** 771,778 **** Partition of: parted FOR VALUES IN ('c') Partition constraint: ((a IS NOT NULL) AND (a =3D 'c'::text)) Partition key: RANGE (b) - Check constraints: - "check_a" CHECK (length(a) > 0) Partitions: part_c_1_10 FOR VALUES FROM (1) TO (10) =20 -- a level-2 partition's constraint will include the parent's expressions --- 794,799 ---- *************** Partitions: part_c_1_10 FOR VALUES FROM *** 784,791 **** b | integer | | not null | 0 | plain | = |=20 Partition of: part_c FOR VALUES FROM (1) TO (10) Partition constraint: ((a IS NOT NULL) AND (a =3D 'c'::text) AND (b IS NO= T NULL) AND (b >=3D 1) AND (b < 10)) - Check constraints: - "check_a" CHECK (length(a) > 0) =20 -- Show partition count in the parent's describe output -- Tempted to include \d+ output listing partitions with bound info but --- 805,810 ---- *************** Check constraints: *** 798,805 **** a | text | | |=20 b | integer | | not null | 0 Partition key: LIST (a) - Check constraints: - "check_a" CHECK (length(a) > 0) Number of partitions: 3 (Use \d+ to list them.) =20 \d hash_parted --- 817,822 ---- diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/c= reate_table.sql new file mode 100644 index c6f048f..a2cae96 *** a/src/test/regress/sql/create_table.sql --- b/src/test/regress/sql/create_table.sql *************** CREATE TABLE part_b PARTITION OF parted *** 639,649 **** ) FOR VALUES IN ('b'); =20 CREATE TABLE part_b PARTITION OF parted ( ! b NOT NULL DEFAULT 1 CHECK (b >=3D 0), ! CONSTRAINT check_a CHECK (length(a) > 0) ) FOR VALUES IN ('b'); ! -- conislocal should be false for any merged constraints ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass AND conname =3D 'check_a'; =20 -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('= c') PARTITION BY RANGE (c); --- 639,664 ---- ) FOR VALUES IN ('b'); =20 CREATE TABLE part_b PARTITION OF parted ( ! b NOT NULL DEFAULT 1, ! CONSTRAINT check_a CHECK (length(a) > 0), ! CONSTRAINT check_b CHECK (b >=3D 0) ) FOR VALUES IN ('b'); ! -- conislocal should be false for any merged constraints, true otherwise ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass ORDER BY conislocal, coninhcount; !=20 ! -- Once check_b is added to the parent, it should be made non-local for p= art_b ! ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >=3D 0); ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass; !=20 ! -- Neither check_a nor check_b are droppable from part_b ! ALTER TABLE part_b DROP CONSTRAINT check_a; ! ALTER TABLE part_b DROP CONSTRAINT check_b; !=20 ! -- And dropping it from parted should leave no trace of them on part_b, u= nlike ! -- traditional inheritance where they will be left behind, because they w= ould ! -- be local constraints. ! ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b; ! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid =3D 'par= t_b'::regclass; =20 -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('= c') PARTITION BY RANGE (c); --OjtEKJ7haQoFl5CE-- --0BIv9nKhOLc5Ty2i Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJcCw2fAAoJEO1sijiDR2RVlywQAIzFfEzBbrBfUYH3h84bnYuQ JEPcF3qbpzE18ds34+ToDquBT9hNVgmB4sR0x7ZNJ5I8v1cKsXU2rVShgdpJoMMp tDgpn7QA+vBCibOoyLfY7kBeGRFcCXlbilSubQK/4APorPMsEj//6wWEEs83Swo1 rd3eE1i3ZrCWiZ8Zjt5O01F6VMtFl0F/9cAf7llvOyRdJRTVB35mSsUu6dDU4Bg7 VDKaNztbyTMzmqOVFmgD2B7CucMfVVRFj9oTE7LM2qWLA+khKF832zBHqCLg2DBl 1BthArjx7VB/qJe9Pg7Hmk88dSaFWnlUmY0s37OvYh77/S7ri2AuT4P8yxipR7UK n5IBmQeWkl1YCDa8Ge2vTfI4ueRlZUOGIn+sclDuKD74K96RIFuHo/S/VRDgegPe Oe2dxluMhQ6vM0LI7+fSWzgIJxKhyjKilXLWShtvRCsFHKYlkK+bbgxysg1BAST5 Jq4IllmXYt8jbccn32wdo7/FnwF2IooXUVJiUfQesnHf317PZ35MjyUnKWnnujTo f39LDdo7av9lz0Whn8f6lR85A6kDFztZgrIPQGy+mmJEt7nf+lz0kSx/VX/uQN9K RE/rro1J6fQcjiaBThGMgCaGtJbJDl6l7yJxyma6AAwl8scadlv2671yVdt5y5pc F9LC9+f13J1j4hpzo5E0 =23Dt -----END PGP SIGNATURE----- --0BIv9nKhOLc5Ty2i--