From: Stephen Frost <sfrost@snowman.net>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Noah Misch <noah@leadboat.com>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: pg_dump emits ALTER TABLE ONLY partitioned_table
Date: Fri, 7 Dec 2018 19:17:36 -0500
Message-ID: <20181208001735.GT3415@tamriel.snowman.net> (raw)
In-Reply-To: <ff0b4014-28d0-7937-59c9-075aad27e976@lab.ntt.co.jp>
References: <20170427141726.GF21223@tamriel.snowman.net>
<696c2d04-80aa-57e2-c1c5-6df25e2bbed5@lab.ntt.co.jp>
<20170503030520.GR21223@tamriel.snowman.net>
<CA+HiwqFn=iGV_sg53CtX+kqmNGm7vdT7sbgzTNOfJ5w=O4H5zg@mail.gmail.com>
<20170504214702.GP21223@tamriel.snowman.net>
<20170506031749.GD843225@rfd.leadboat.com>
<CAOuzzgpmP8uDmqh9ekwR2VUakm_9=B1X6vyyZK+xazwK+s1Hug@mail.gmail.com>
<3e2aa1e9-e621-9ad5-b24d-e24dbc9650ba@lab.ntt.co.jp>
<20170508034245.GY21223@tamriel.snowman.net>
<ff0b4014-28d0-7937-59c9-075aad27e976@lab.ntt.co.jp>
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 sorry
> >> that I couldn't reply promptly during the last week due to vacation.
> >
> > 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.
>
> 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 over.
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
Attachments:
[text/x-diff] addl-partition-tests-v1_master.patch (8.0K, ../20181208001735.GT3415@tamriel.snowman.net/2-addl-partition-tests-v1_master.patch)
download | inline diff:diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.plnew file mode 100644index 2afd950..8f4bb55
*** a/src/bin/pg_dump/t/002_pg_dump.pl
--- b/src/bin/pg_dump/t/002_pg_dump.pl
*************** my %tests = (
*** 2246,2257 ****
create_order => 91,
create_sql =>
'CREATE TABLE dump_test_second_schema.measurement_y2006m2
! PARTITION OF dump_test.measurement FOR VALUES
! FROM (\'2006-02-01\') TO (\'2006-03-01\');',
regexp => qr/^
\Q-- Name: measurement_y2006m2;\E.*\n
\Q--\E\n\n
! \QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION OF dump_test.measurement\E\n
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
/xm,
like => {
--- 2246,2261 ----
create_order => 91,
create_sql =>
'CREATE TABLE dump_test_second_schema.measurement_y2006m2
! PARTITION OF dump_test.measurement (
! unitsales DEFAULT 0 CHECK (unitsales >= 0)
! )
! FOR VALUES FROM (\'2006-02-01\') TO (\'2006-03-01\');',
regexp => qr/^
\Q-- Name: measurement_y2006m2;\E.*\n
\Q--\E\n\n
! \QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION OF dump_test.measurement (\E\n
! \s+\QCONSTRAINT measurement_y2006m2_unitsales_check CHECK ((unitsales >= 0))\E\n
! \)\n
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
/xm,
like => {
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.outnew file mode 100644index 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 >= 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 = 'part_b'::regclass AND conname = 'check_a';
conislocal | coninhcount
------------+-------------
f | 1
! (1 row)
-- 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 >= 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 = 'part_b'::regclass ORDER BY conislocal, coninhcount;
conislocal | coninhcount
------------+-------------
f | 1
! t | 0
! (2 rows)
!
! -- Once check_b is added to the parent, it should be made non-local for part_b
! ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0);
! NOTICE: merging constraint "check_b" with inherited definition
! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
! conislocal | coninhcount
! ------------+-------------
! f | 1
! f | 1
! (2 rows)
!
! -- 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, unlike
! -- traditional inheritance where they will be left behind, because they would
! -- be local constraints.
! ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b;
! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
! conislocal | coninhcount
! ------------+-------------
! (0 rows)
-- 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 | |
Partition of: parted FOR VALUES IN ('b')
Partition constraint: ((a IS NOT NULL) AND (a = 'b'::text))
- Check constraints:- "check_a" CHECK (length(a) > 0)- "part_b_b_check" CHECK (b >= 0)
-- 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 = '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)
-- 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 | |
Partition of: part_c FOR VALUES FROM (1) TO (10)
Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10))
- Check constraints:- "check_a" CHECK (length(a) > 0)
-- 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 | | |
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.)
\d hash_parted
--- 817,822 ----diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sqlnew file mode 100644index 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');
CREATE TABLE part_b PARTITION OF parted (
! b NOT NULL DEFAULT 1 CHECK (b >= 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 = 'part_b'::regclass AND conname = 'check_a';
-- 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');
CREATE TABLE part_b PARTITION OF parted (
! b NOT NULL DEFAULT 1,
! CONSTRAINT check_a CHECK (length(a) > 0),
! CONSTRAINT check_b CHECK (b >= 0)
) FOR VALUES IN ('b');
! -- conislocal should be false for any merged constraints, true otherwise
! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY conislocal, coninhcount;
!
! -- Once check_b is added to the parent, it should be made non-local for part_b
! ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0);
! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
!
! -- 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;
!
! -- And dropping it from parted should leave no trace of them on part_b, unlike
! -- traditional inheritance where they will be left behind, because they would
! -- be local constraints.
! ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b;
! SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass;
-- specify PARTITION BY for a partition
CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c);
[application/pgp-signature] signature.asc (818B, ../20181208001735.GT3415@tamriel.snowman.net/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, Langote_Amit_f8@lab.ntt.co.jp, noah@leadboat.com, amitlangote09@gmail.com, robertmhaas@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: pg_dump emits ALTER TABLE ONLY partitioned_table
In-Reply-To: <20181208001735.GT3415@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox