On 2017/05/08 12:42, Stephen Frost wrote: > Amit, > > * Amit Langote (langote_amit...@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. Thanks, Amit
>From a27689d445d88462dc499f04ed6779cb686a9588 Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Tue, 18 Apr 2017 10:59:35 +0900 Subject: [PATCH] Improve test coverage of partition constraints Better exercise the code manipulating partition constraints a bit differently from the traditional inheritance children. --- src/bin/pg_dump/t/002_pg_dump.pl | 10 +++++--- src/test/regress/expected/create_table.out | 41 ++++++++++++++++++++++-------- src/test/regress/sql/create_table.sql | 21 ++++++++++++--- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index ce0c9ef54d..08ae7eb83b 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -4791,12 +4791,16 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog catch_all => 'CREATE ... commands', 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\');', + 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 measurement_y2006m2 PARTITION OF dump_test.measurement\E\n + \QCREATE TABLE 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.out index dda0d7ee5d..79603166d1 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -618,16 +618,42 @@ CREATE TABLE part_b PARTITION OF parted ( ) 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) + 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 -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass AND conname = 'check_a'; +SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; conislocal | coninhcount ------------+------------- f | 1 -(1 row) + 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); @@ -643,9 +669,6 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); a | text | | | b | integer | | not null | 1 Partition of: parted FOR VALUES IN ('b') -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 @@ -656,8 +679,6 @@ Check constraints: b | integer | | not null | 0 Partition of: parted FOR VALUES IN ('c') Partition key: RANGE (b) -Check constraints: - "check_a" CHECK (length(a) > 0) Number of partitions: 1 (Use \d+ to list them.) -- Show partition count in the parent's describe output @@ -671,8 +692,6 @@ Number of partitions: 1 (Use \d+ to list them.) 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.) -- cleanup diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index caf5ddb58b..14a8583c85 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -579,11 +579,26 @@ CREATE TABLE part_b PARTITION OF parted ( ) 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) + 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 -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass AND conname = 'check_a'; +SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; + +-- 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); -- 2.11.0
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers