Hello
> Sergei, can we enlist you to submit a patch for this? Namely reduce the
> log level to DEBUG1 and add a TAP test in src/test/modules/alter_table/
> that verifies that the message is or isn't emitted, as appropriate.
I created this patch.
I test message existence. Also I check message "verifying table" (generated on
DEBUG1 from ATRewriteTable). So with manually damaged logic in
NotNullImpliedByRelConstraints or ConstraintImpliedByRelConstraint "make check"
may works but fails on new test during "make check-world". As we want.
regards, Sergei
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 60d6d7be1b..ab3fc91fae 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -20,6 +20,7 @@ SUBDIRS = \
test_rls_hooks \
test_shm_mq \
unsafe_tests \
- worker_spi
+ worker_spi \
+ alter_table
$(recurse)
diff --git a/src/test/modules/alter_table/.gitignore b/src/test/modules/alter_table/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/alter_table/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/alter_table/Makefile b/src/test/modules/alter_table/Makefile
new file mode 100644
index 0000000000..17bd882428
--- /dev/null
+++ b/src/test/modules/alter_table/Makefile
@@ -0,0 +1,17 @@
+# src/test/modules/alter_table/Makefile
+
+MODULE = alter_table
+PGFILEDESC = "alter_table - regression testing for some ALTER TABLE commands"
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/alter_table
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/alter_table/t/001_constraint_validation.pl b/src/test/modules/alter_table/t/001_constraint_validation.pl
new file mode 100644
index 0000000000..3c5b65e796
--- /dev/null
+++ b/src/test/modules/alter_table/t/001_constraint_validation.pl
@@ -0,0 +1,213 @@
+# copy of some alter_table test to prove table verification optimisations
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 42;
+use Data::Dumper;
+
+# Initialize master node
+my $node = get_new_node('master');
+$node->init();
+$node->append_conf('postgresql.conf', 'client_min_messages = DEBUG1');
+$node->start;
+
+sub run_sql_command
+{
+ my $stderr;
+ $node->psql(
+ 'postgres',
+ shift,
+ stderr => \$stderr,
+ on_error_die => 1,
+ on_error_stop => 1
+ );
+ return $stderr
+}
+
+sub is_table_verified
+{
+ return index(shift, 'DEBUG: verifying table') != -1;
+}
+
+my $output;
+
+note "test alter table set not null";
+
+run_sql_command('create table atacc1 (test_a int, test_b int);');
+run_sql_command('insert into atacc1 values (1, 2);');
+
+$output = run_sql_command('alter table atacc1 alter test_a set not null;');
+ok( is_table_verified($output), 'column test_a without constraint will scan table');
+
+run_sql_command('alter table atacc1 alter test_a drop not null;');
+run_sql_command('alter table atacc1 add constraint atacc1_constr_a_valid check(test_a is not null);');
+
+# normal run will verify table data
+$output = run_sql_command('alter table atacc1 alter test_a set not null;');
+ok(!is_table_verified($output), 'with constraint will not scan table');
+ok( $output =~ m/existing constraints on column "atacc1"."test_a" are sufficient to prove that it does not contain nulls/ , 'test_a proved by constraints');
+
+run_sql_command('alter table atacc1 alter test_a drop not null;');
+
+# we have check only for test_a column, so we need verify table for test_b
+$output = run_sql_command('alter table atacc1 alter test_b set not null, alter test_a set not null;');
+ok( is_table_verified($output), 'table was scanned');
+# we may miss debug message for test_a constraint because we need verify table due test_b
+ok( ! ($output =~ m/existing constraints on column "atacc1"."test_b" are sufficient to prove that it does not contain nulls/ ) , 'test_b not proved by wrong constraints');
+
+run_sql_command('alter table atacc1 alter test_a drop not null, alter test_b drop not null;');
+# both column has check constraints
+run_sql_command('alter table atacc1 add constraint atacc1_constr_b_valid check(test_b is not null);');
+$output = run_sql_command('alter table atacc1 alter test_b set not null, alter test_a set not null;');
+ok(!is_table_verified($output), 'table was not scanned for both columns');
+ok( $output =~ m/existing constraints on column "atacc1"."test_a" are sufficient to prove that it does not contain nulls/ , 'test_a proved by constraints');
+ok( $output =~ m/existing constraints on column "atacc1"."test_b" are sufficient to prove that it does not contain nulls/ , 'test_b proved by constraints');
+run_sql_command('drop table atacc1;');
+
+note "test alter table attach partition";
+
+run_sql_command('CREATE TABLE list_parted2 (
+ a int,
+ b char
+) PARTITION BY LIST (a);');
+run_sql_command('CREATE TABLE part_3_4 (
+ LIKE list_parted2,
+ CONSTRAINT check_a CHECK (a IN (3))
+);');
+
+# need NOT NULL to skip table scan
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);');
+ok( is_table_verified($output), 'table part_3_4 scanned');
+
+run_sql_command('ALTER TABLE list_parted2 DETACH PARTITION part_3_4;');
+run_sql_command('ALTER TABLE part_3_4 ALTER a SET NOT NULL;');
+
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);');
+ok(!is_table_verified($output), 'table part_3_4 not scanned');
+ok ($output =~ m/partition constraint for table "part_3_4" is implied by existing constraints/ , 'part_3_4 verified by existing constraints');
+
+#~ run_sql_command('CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT;');
+#~ run_sql_command('ALTER TABLE list_parted2_def ADD CONSTRAINT check_a CHECK (a IN (5, 6));');
+
+# test attach default parition
+run_sql_command('CREATE TABLE list_parted2_def (
+ LIKE list_parted2,
+ CONSTRAINT check_a CHECK (a IN (5, 6))
+);');
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION list_parted2_def default;');
+ok(!is_table_verified($output), 'table list_parted2_def not scanned');
+ok ($output =~ m/partition constraint for table "list_parted2_def" is implied by existing constraints/ , 'list_parted2_def verified by existing constraints');
+
+$output = run_sql_command('CREATE TABLE part_55_66 PARTITION OF list_parted2 FOR VALUES IN (55, 66);');
+ok(!is_table_verified($output), 'table list_parted2_def not scanned');
+ok ($output =~ m/updated partition constraint for default partition "list_parted2_def" is implied by existing constraints/, 'updated partition constraint for default partition list_parted2_def');
+
+# test attach another partitioned table
+run_sql_command('CREATE TABLE part_5 (
+ LIKE list_parted2
+) PARTITION BY LIST (b);');
+run_sql_command('CREATE TABLE part_5_a PARTITION OF part_5 FOR VALUES IN (\'a\');');
+run_sql_command('ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5);');
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);');
+ok(!($output =~ m/verifying table "part_5"/), 'table part_5 not scanned');
+ok($output =~ m/verifying table "list_parted2_def"/, 'list_parted2_def scanned');
+ok ($output =~ m/partition constraint for table "part_5" is implied by existing constraints/, 'part_5 verified by existing constraints');
+
+run_sql_command('ALTER TABLE list_parted2 DETACH PARTITION part_5;');
+run_sql_command('ALTER TABLE part_5 DROP CONSTRAINT check_a;');
+# scan should again be skipped, even though NOT NULL is now a column property
+run_sql_command('ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;');
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);');
+ok(!($output =~ m/verifying table "part_5"/), 'table part_5 not scanned');
+ok($output =~ m/verifying table "list_parted2_def"/, 'list_parted2_def scanned');
+ok ($output =~ m/partition constraint for table "part_5" is implied by existing constraints/, 'part_5 verified by existing constraints');
+
+# Check the case where attnos of the partitioning columns in the table being
+# attached differs from the parent. It should not affect the constraint-
+# checking logic that allows to skip the scan.
+run_sql_command('CREATE TABLE part_6 (
+ c int,
+ LIKE list_parted2,
+ CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 6)
+);');
+run_sql_command('ALTER TABLE part_6 DROP c;');
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);');
+ok(!($output =~ m/verifying table "part_6"/), 'table part_6 not scanned');
+ok($output =~ m/verifying table "list_parted2_def"/, 'list_parted2_def scanned');
+ok ($output =~ m/partition constraint for table "part_6" is implied by existing constraints/, 'part_6 verified by existing constraints');
+
+# Similar to above, but the table being attached is a partitioned table
+# whose partition has still different attnos for the root partitioning
+# columns.
+run_sql_command('CREATE TABLE part_7 (
+ LIKE list_parted2,
+ CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) PARTITION BY LIST (b);');
+run_sql_command('CREATE TABLE part_7_a_null (
+ c int,
+ d int,
+ e int,
+ LIKE list_parted2, -- a will have attnum = 4
+ CONSTRAINT check_b CHECK (b IS NULL OR b = \'a\'),
+ CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+);');
+run_sql_command('ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;');
+$output = run_sql_command('ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN (\'a\', null);');
+ok(!is_table_verified($output), 'table not scanned');
+ok ($output =~ m/partition constraint for table "part_7_a_null" is implied by existing constraints/, 'part_7_a_null verified by existing constraints');
+$output = run_sql_command('ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);');
+ok(!is_table_verified($output), 'tables not scanned');
+ok ($output =~ m/partition constraint for table "part_7" is implied by existing constraints/, 'part_7 verified by existing constraints');
+ok ($output =~ m/updated partition constraint for default partition "list_parted2_def" is implied by existing constraints/, 'updated partition constraint for default partition list_parted2_def');
+
+run_sql_command('CREATE TABLE range_parted (
+ a int,
+ b int
+) PARTITION BY RANGE (a, b);');
+run_sql_command('CREATE TABLE range_part1 (
+ a int NOT NULL CHECK (a = 1),
+ b int NOT NULL
+);');
+$output = run_sql_command('ALTER TABLE range_parted ATTACH PARTITION range_part1 FOR VALUES FROM (1, 1) TO (1, 10);');
+ok( is_table_verified($output), 'table range_part1 scanned');
+ok(! ($output =~ m/partition constraint for table "range_part1" is implied by existing constraints/) , 'range_part1 not verified by existing constraints');
+
+run_sql_command('CREATE TABLE range_part2 (
+ a int NOT NULL CHECK (a = 1),
+ b int NOT NULL CHECK (b >= 10 and b < 18)
+);');
+$output = run_sql_command('ALTER TABLE range_parted ATTACH PARTITION range_part2 FOR VALUES FROM (1, 10) TO (1, 20);');
+ok(!is_table_verified($output), 'table range_part2 not scanned');
+ok( $output =~ m/partition constraint for table "range_part2" is implied by existing constraints/ , 'range_part2 verified by existing constraints');
+
+# If a partitioned table being created or an existing table being attached
+# as a partition does not have a constraint that would allow validation scan
+# to be skipped, but an individual partition does, then the partition's
+# validation scan is skipped.
+run_sql_command('CREATE TABLE quuux (a int, b text) PARTITION BY LIST (a);');
+run_sql_command('CREATE TABLE quuux_default PARTITION OF quuux DEFAULT PARTITION BY LIST (b);');
+run_sql_command('CREATE TABLE quuux_default1 PARTITION OF quuux_default (
+ CONSTRAINT check_1 CHECK (a IS NOT NULL AND a = 1)
+) FOR VALUES IN (\'b\');');
+run_sql_command('CREATE TABLE quuux1 (a int, b text);');
+$output = run_sql_command('ALTER TABLE quuux ATTACH PARTITION quuux1 FOR VALUES IN (1);');
+ok(is_table_verified($output), 'quuux1 table scanned');
+ok( !($output =~ m/partition constraint for table "quuux1" is implied by existing constraints/) , 'quuux1 verified by existing constraints');
+
+run_sql_command('CREATE TABLE quuux2 (a int, b text);');
+$output = run_sql_command('ALTER TABLE quuux ATTACH PARTITION quuux2 FOR VALUES IN (2);');
+ok(!($output =~ m/verifying table "quuux_default1"/), 'quuux_default1 not scanned');
+ok($output =~ m/verifying table "quuux2"/, 'quuux2 scanned');
+ok( $output =~ m/updated partition constraint for default partition "quuux_default1" is implied by existing constraints/ , 'updated partition constraint for default partition quuux_default1');
+run_sql_command('DROP TABLE quuux1, quuux2;');
+# should validate for quuux1, but not for quuux2
+$output = run_sql_command('CREATE TABLE quuux1 PARTITION OF quuux FOR VALUES IN (1);');
+ok(!is_table_verified($output), 'tables not scanned');
+ok( !($output =~ m/partition constraint for table "quuux1" is implied by existing constraints/) , 'quuux1 verified by existing constraints');
+$output = run_sql_command('CREATE TABLE quuux2 PARTITION OF quuux FOR VALUES IN (2);');
+ok(!is_table_verified($output), 'tables not scanned');
+ok( $output =~ m/updated partition constraint for default partition "quuux_default1" is implied by existing constraints/ , 'updated partition constraint for default partition quuux_default1');
+run_sql_command('DROP TABLE quuux;');
+
+$node->stop('fast');
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cceefbdd49..30f8d536bf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15402,11 +15402,11 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
if (PartConstraintImpliedByRelConstraint(scanrel, partConstraint))
{
if (!validate_default)
- ereport(INFO,
+ ereport(DEBUG1,
(errmsg("partition constraint for table \"%s\" is implied by existing constraints",
RelationGetRelationName(scanrel))));
else
- ereport(INFO,
+ ereport(DEBUG1,
(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
RelationGetRelationName(scanrel))));
return;
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 46d03f3b9b..318d8ecae9 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -1253,7 +1253,7 @@ check_default_partition_contents(Relation parent, Relation default_rel,
*/
if (PartConstraintImpliedByRelConstraint(default_rel, def_part_constraints))
{
- ereport(INFO,
+ ereport(DEBUG1,
(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
RelationGetRelationName(default_rel))));
return;
@@ -1304,7 +1304,7 @@ check_default_partition_contents(Relation parent, Relation default_rel,
if (PartConstraintImpliedByRelConstraint(part_rel,
def_part_constraints))
{
- ereport(INFO,
+ ereport(DEBUG1,
(errmsg("updated partition constraint for default partition \"%s\" is implied by existing constraints",
RelationGetRelationName(part_rel))));
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e5407bbf0f..23d4265555 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3633,11 +3633,9 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
ALTER TABLE list_parted2 DETACH PARTITION part_3_4;
ALTER TABLE part_3_4 ALTER a SET NOT NULL;
ALTER TABLE list_parted2 ATTACH PARTITION part_3_4 FOR VALUES IN (3, 4);
-INFO: partition constraint for table "part_3_4" is implied by existing constraints
-- check if default partition scan skipped
ALTER TABLE list_parted2_def ADD CONSTRAINT check_a CHECK (a IN (5, 6));
CREATE TABLE part_55_66 PARTITION OF list_parted2 FOR VALUES IN (55, 66);
-INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
-- check validation when attaching range partitions
CREATE TABLE range_parted (
a int,
@@ -3662,7 +3660,6 @@ CREATE TABLE part2 (
b int NOT NULL CHECK (b >= 10 AND b < 18)
);
ALTER TABLE range_parted ATTACH PARTITION part2 FOR VALUES FROM (1, 10) TO (1, 20);
-INFO: partition constraint for table "part2" is implied by existing constraints
-- Create default partition
CREATE TABLE partr_def1 PARTITION OF range_parted DEFAULT;
-- Only one default partition is allowed, hence, following should give error
@@ -3690,13 +3687,11 @@ ERROR: partition constraint is violated by some row
DELETE FROM part_5_a WHERE a NOT IN (3);
ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5);
ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
-INFO: partition constraint for table "part_5" is implied by existing constraints
ALTER TABLE list_parted2 DETACH PARTITION part_5;
ALTER TABLE part_5 DROP CONSTRAINT check_a;
-- scan should again be skipped, even though NOT NULL is now a column property
ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
-INFO: partition constraint for table "part_5" is implied by existing constraints
-- Check the case where attnos of the partitioning columns in the table being
-- attached differs from the parent. It should not affect the constraint-
-- checking logic that allows to skip the scan.
@@ -3707,7 +3702,6 @@ CREATE TABLE part_6 (
);
ALTER TABLE part_6 DROP c;
ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
-INFO: partition constraint for table "part_6" is implied by existing constraints
-- Similar to above, but the table being attached is a partitioned table
-- whose partition has still different attnos for the root partitioning
-- columns.
@@ -3725,10 +3719,7 @@ CREATE TABLE part_7_a_null (
);
ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;
ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null);
-INFO: partition constraint for table "part_7_a_null" is implied by existing constraints
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
-INFO: partition constraint for table "part_7" is implied by existing constraints
-INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
-- Same example, but check this time that the constraint correctly detects
-- violating rows
ALTER TABLE list_parted2 DETACH PARTITION part_7;
@@ -3742,7 +3733,6 @@ SELECT tableoid::regclass, a, b FROM part_7 order by a;
(2 rows)
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
-INFO: updated partition constraint for default partition "list_parted2_def" is implied by existing constraints
ERROR: partition constraint is violated by some row
-- check that leaf partitions of default partition are scanned when
-- attaching a partitioned table.
@@ -3779,12 +3769,10 @@ CREATE TABLE quuux1 (a int, b text);
ALTER TABLE quuux ATTACH PARTITION quuux1 FOR VALUES IN (1); -- validate!
CREATE TABLE quuux2 (a int, b text);
ALTER TABLE quuux ATTACH PARTITION quuux2 FOR VALUES IN (2); -- skip validation
-INFO: updated partition constraint for default partition "quuux_default1" is implied by existing constraints
DROP TABLE quuux1, quuux2;
-- should validate for quuux1, but not for quuux2
CREATE TABLE quuux1 PARTITION OF quuux FOR VALUES IN (1);
CREATE TABLE quuux2 PARTITION OF quuux FOR VALUES IN (2);
-INFO: updated partition constraint for default partition "quuux_default1" is implied by existing constraints
DROP TABLE quuux;
-- check validation when attaching hash partitions
-- Use hand-rolled hash functions and operator class to get predictable result
@@ -4019,7 +4007,6 @@ delete from defpart_attach_test_d where a = 1;
alter table defpart_attach_test_d add check (a > 1);
-- should be attached successfully and without needing to be scanned
alter table defpart_attach_test attach partition defpart_attach_test_d default;
-INFO: partition constraint for table "defpart_attach_test_d" is implied by existing constraints
-- check that attaching a partition correctly reports any rows in the default
-- partition that should not be there for the new partition to be attached
-- successfully
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 3c302713dc..ce2484fd4e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -1163,7 +1163,6 @@ alter table defcheck_def drop c;
alter table defcheck attach partition defcheck_def default;
alter table defcheck_def add check (b <= 0 and b is not null);
create table defcheck_1 partition of defcheck for values in (1, null);
-INFO: updated partition constraint for default partition "defcheck_def" is implied by existing constraints
-- test that complex default partition constraints are enforced correctly
insert into defcheck_def values (0, 0);
create table defcheck_0 partition of defcheck for values in (0);
