On 2017/03/07 14:04, Tom Lane wrote:
> Amit Langote <langote_amit...@lab.ntt.co.jp> writes:
>> Also, I found out that alter_table.sql mistakenly forgot to drop
>> partitioned table "p1". Patch 0002 takes care of that.
>
> While that might or might not have been intentional, I think it's an
> astoundingly bad idea to not leave any partitioned tables behind in
> the final state of the regression database. Doing so would likely
> have meant that this particular bug evaded detection for much longer
> than it did. Moreover, it would mean that the pg_upgrade test would
> have exactly no coverage of partitioned cases.
That's true. Should have been apparent to me.
> Therefore, there should definitely be a partitioned table, hopefully with
> a less generic name than "p1", in the final regression DB state. Whether
> this particular one from alter_table.sql is a good candidate, I dunno.
> But let's not drop it without adding a better-thought-out replacement.
OK, let's drop p1 in alter_table.sql. I think a partitioned table created
in insert.sql is a good candidate to keep around after having it renamed,
which patch 0003 does.
Thanks,
Amit
>From 0fe3bcea9424133b8711bdcc23b1432cde4fc394 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 7 Mar 2017 10:26:16 +0900
Subject: [PATCH 1/3] Fix relcache ref leak in acquire_inherited_sample_rows
Add tests for vacuum, analyze, truncate on partitioned table, as
3c3bb9933 should have.
---
src/backend/commands/analyze.c | 7 +++++--
src/test/regress/expected/truncate.out | 6 ++++++
src/test/regress/expected/vacuum.out | 9 +++++++++
src/test/regress/sql/truncate.sql | 7 +++++++
src/test/regress/sql/vacuum.sql | 10 ++++++++++
5 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index a70c760341..b91df986c5 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1360,11 +1360,14 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
else
{
/*
- * ignore, but release the lock on it. could be a partitioned
- * table.
+ * ignore, but release the lock on it. don't try to unlock the
+ * passed-in relation
*/
+ Assert(childrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
if (childrel != onerel)
heap_close(childrel, AccessShareLock);
+ else
+ heap_close(childrel, NoLock);
continue;
}
diff --git a/src/test/regress/expected/truncate.out b/src/test/regress/expected/truncate.out
index 5c5277e0f1..81612d8c88 100644
--- a/src/test/regress/expected/truncate.out
+++ b/src/test/regress/expected/truncate.out
@@ -420,3 +420,9 @@ SELECT nextval('truncate_a_id1'); -- fail, seq should have been dropped
ERROR: relation "truncate_a_id1" does not exist
LINE 1: SELECT nextval('truncate_a_id1');
^
+-- partitioned table
+CREATE TABLE truncparted (a int, b char) PARTITION BY LIST (a);
+CREATE TABLE truncparted1 PARTITION OF truncparted FOR VALUES IN (1);
+INSERT INTO truncparted VALUES (1, 'a');
+TRUNCATE truncparted;
+DROP TABLE truncparted;
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 9b604be4b6..6f68663087 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -82,3 +82,12 @@ VACUUM FULL vactst;
VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
DROP TABLE vaccluster;
DROP TABLE vactst;
+-- partitioned table
+CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
+CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
+INSERT INTO vacparted VALUES (1, 'a');
+UPDATE vacparted SET b = 'b';
+VACUUM (ANALYZE) vacparted;
+VACUUM (FULL) vacparted;
+VACUUM (FREEZE) vacparted;
+DROP TABLE vacparted;
diff --git a/src/test/regress/sql/truncate.sql b/src/test/regress/sql/truncate.sql
index a3d6f5368f..d61eea1a42 100644
--- a/src/test/regress/sql/truncate.sql
+++ b/src/test/regress/sql/truncate.sql
@@ -215,3 +215,10 @@ SELECT * FROM truncate_a;
DROP TABLE truncate_a;
SELECT nextval('truncate_a_id1'); -- fail, seq should have been dropped
+
+-- partitioned table
+CREATE TABLE truncparted (a int, b char) PARTITION BY LIST (a);
+CREATE TABLE truncparted1 PARTITION OF truncparted FOR VALUES IN (1);
+INSERT INTO truncparted VALUES (1, 'a');
+TRUNCATE truncparted;
+DROP TABLE truncparted;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 7b819f654d..7c5fb04917 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -64,3 +64,13 @@ VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
DROP TABLE vaccluster;
DROP TABLE vactst;
+
+-- partitioned table
+CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
+CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
+INSERT INTO vacparted VALUES (1, 'a');
+UPDATE vacparted SET b = 'b';
+VACUUM (ANALYZE) vacparted;
+VACUUM (FULL) vacparted;
+VACUUM (FREEZE) vacparted;
+DROP TABLE vacparted;
--
2.11.0
>From 19a2e5d76a048bf7f761376471947366bffbd6c4 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 7 Mar 2017 13:45:10 +0900
Subject: [PATCH 2/3] Drop a table mistakenly left behind by alter_table.sql
---
src/test/regress/expected/alter_table.out | 1 +
src/test/regress/sql/alter_table.sql | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c15bbdcbd1..2227f2d977 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3371,3 +3371,4 @@ alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
-- cleanup
drop table p;
+drop table p1;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 37f327bf6d..8cd6786a90 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2227,3 +2227,4 @@ alter table p attach partition p1 for values from (1, 2) to (1, 10);
-- cleanup
drop table p;
+drop table p1;
--
2.11.0
>From 45a4b1260f1e9bcb129c6b063af388eb0cea350d Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 7 Mar 2017 15:35:01 +0900
Subject: [PATCH 3/3] Leave a partitioned table in the regression database
And its partitions. Rename from 'p%' to 'mlparted%'.
---
src/test/regress/expected/insert.out | 104 ++++++++++++++---------------
src/test/regress/expected/sanity_check.out | 9 ++-
src/test/regress/sql/insert.sql | 69 +++++++++----------
src/test/regress/sql/sanity_check.sql | 2 +-
4 files changed, 93 insertions(+), 91 deletions(-)
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index d16880fa2d..116854e142 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -316,75 +316,75 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
-create table p (a int, b int) partition by range (a, b);
-create table p1 (b int not null, a int not null) partition by range ((b+0));
-create table p11 (like p1);
-alter table p11 drop a;
-alter table p11 add a int;
-alter table p11 drop a;
-alter table p11 add a int not null;
--- attnum for key attribute 'a' is different in p, p1, and p11
+create table mlparted (a int, b int) partition by range (a, b);
+create table mlparted1 (b int not null, a int not null) partition by range ((b+0));
+create table mlparted11 (like mlparted1);
+alter table mlparted11 drop a;
+alter table mlparted11 add a int;
+alter table mlparted11 drop a;
+alter table mlparted11 add a int not null;
+-- attnum for key attribute 'a' is different in mlparted, mlparted1, and mlparted11
select attrelid::regclass, attname, attnum
from pg_attribute
where attname = 'a'
- and (attrelid = 'p'::regclass
- or attrelid = 'p1'::regclass
- or attrelid = 'p11'::regclass)
+ and (attrelid = 'mlparted'::regclass
+ or attrelid = 'mlparted1'::regclass
+ or attrelid = 'mlparted11'::regclass)
order by attrelid::regclass::text;
- attrelid | attname | attnum
-----------+---------+--------
- p | a | 1
- p1 | a | 2
- p11 | a | 4
+ attrelid | attname | attnum
+------------+---------+--------
+ mlparted | a | 1
+ mlparted1 | a | 2
+ mlparted11 | a | 4
(3 rows)
-alter table p1 attach partition p11 for values from (2) to (5);
-alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- check that "(1, 2)" is correctly routed to p11.
-insert into p values (1, 2);
-select tableoid::regclass, * from p;
- tableoid | a | b
-----------+---+---
- p11 | 1 | 2
+alter table mlparted1 attach partition mlparted11 for values from (2) to (5);
+alter table mlparted attach partition mlparted1 for values from (1, 2) to (1, 10);
+-- check that "(1, 2)" is correctly routed to mlparted11.
+insert into mlparted values (1, 2);
+select tableoid::regclass, * from mlparted;
+ tableoid | a | b
+------------+---+---
+ mlparted11 | 1 | 2
(1 row)
--- check that proper message is shown after failure to route through p1
-insert into p (a, b) values (1, 5);
-ERROR: no partition of relation "p1" found for row
+-- check that proper message is shown after failure to route through mlparted1
+insert into mlparted (a, b) values (1, 5);
+ERROR: no partition of relation "mlparted1" found for row
DETAIL: Partition key of the failing row contains ((b + 0)) = (5).
-truncate p;
-alter table p add constraint check_b check (b = 3);
--- check that correct input row is shown when constraint check_b fails on p11
+truncate mlparted;
+alter table mlparted add constraint check_b check (b = 3);
+-- check that correct input row is shown when constraint check_b fails on mlparted11
-- after "(1, 2)" is routed to it
-insert into p values (1, 2);
-ERROR: new row for relation "p11" violates check constraint "check_b"
+insert into mlparted values (1, 2);
+ERROR: new row for relation "mlparted11" violates check constraint "check_b"
DETAIL: Failing row contains (1, 2).
-- check that inserting into an internal partition successfully results in
-- checking its partition constraint before inserting into the leaf partition
-- selected by tuple-routing
-insert into p1 (a, b) values (2, 3);
-ERROR: new row for relation "p11" violates partition constraint
+insert into mlparted1 (a, b) values (2, 3);
+ERROR: new row for relation "mlparted11" violates partition constraint
DETAIL: Failing row contains (3, 2).
-- check that RETURNING works correctly with tuple-routing
-alter table p drop constraint check_b;
-create table p12 partition of p1 for values from (5) to (10);
-create table p2 (b int not null, a int not null);
-alter table p attach partition p2 for values from (1, 10) to (1, 20);
-create table p3 partition of p for values from (1, 20) to (1, 30);
-create table p4 (like p);
-alter table p4 drop a;
-alter table p4 add a int not null;
-alter table p attach partition p4 for values from (1, 30) to (1, 40);
+alter table mlparted drop constraint check_b;
+create table mlparted12 partition of mlparted1 for values from (5) to (10);
+create table mlparted2 (b int not null, a int not null);
+alter table mlparted attach partition mlparted2 for values from (1, 10) to (1, 20);
+create table mlparted3 partition of mlparted for values from (1, 20) to (1, 30);
+create table mlparted4 (like mlparted);
+alter table mlparted4 drop a;
+alter table mlparted4 add a int not null;
+alter table mlparted attach partition mlparted4 for values from (1, 30) to (1, 40);
with ins (a, b, c) as
- (insert into p (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *)
+ (insert into mlparted (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *)
select a, b, min(c), max(c) from ins group by a, b order by 1;
- a | b | min | max
------+---+-----+-----
- p11 | 1 | 2 | 4
- p12 | 1 | 5 | 9
- p2 | 1 | 10 | 19
- p3 | 1 | 20 | 29
- p4 | 1 | 30 | 39
+ a | b | min | max
+------------+---+-----+-----
+ mlparted11 | 1 | 2 | 4
+ mlparted12 | 1 | 5 | 9
+ mlparted2 | 1 | 10 | 19
+ mlparted3 | 1 | 20 | 29
+ mlparted4 | 1 | 30 | 39
(5 rows)
-- check that message shown after failure to find a partition shows the
@@ -413,5 +413,3 @@ revoke all on key_desc from someone_else;
revoke all on key_desc_1 from someone_else;
drop role someone_else;
drop table key_desc, key_desc_1;
--- cleanup
-drop table p;
diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index 0af013f8a2..bdbb39180f 100644
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -9,7 +9,7 @@ VACUUM;
\a\t
SELECT relname, relhasindex
FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = relnamespace
- WHERE relkind = 'r' AND (nspname ~ '^pg_temp_') IS NOT TRUE
+ WHERE relkind IN ('r', 'P') AND (nspname ~ '^pg_temp_') IS NOT TRUE
ORDER BY relname;
a|f
a_star|f
@@ -70,6 +70,13 @@ line_tbl|f
log_table|f
lseg_tbl|f
main_table|f
+mlparted|f
+mlparted1|f
+mlparted11|f
+mlparted12|f
+mlparted2|f
+mlparted3|f
+mlparted4|f
money_data|f
num_data|f
num_exp_add|t
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index b0b552bd99..c56c3c22f8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -189,55 +189,55 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
-create table p (a int, b int) partition by range (a, b);
-create table p1 (b int not null, a int not null) partition by range ((b+0));
-create table p11 (like p1);
-alter table p11 drop a;
-alter table p11 add a int;
-alter table p11 drop a;
-alter table p11 add a int not null;
--- attnum for key attribute 'a' is different in p, p1, and p11
+create table mlparted (a int, b int) partition by range (a, b);
+create table mlparted1 (b int not null, a int not null) partition by range ((b+0));
+create table mlparted11 (like mlparted1);
+alter table mlparted11 drop a;
+alter table mlparted11 add a int;
+alter table mlparted11 drop a;
+alter table mlparted11 add a int not null;
+-- attnum for key attribute 'a' is different in mlparted, mlparted1, and mlparted11
select attrelid::regclass, attname, attnum
from pg_attribute
where attname = 'a'
- and (attrelid = 'p'::regclass
- or attrelid = 'p1'::regclass
- or attrelid = 'p11'::regclass)
+ and (attrelid = 'mlparted'::regclass
+ or attrelid = 'mlparted1'::regclass
+ or attrelid = 'mlparted11'::regclass)
order by attrelid::regclass::text;
-alter table p1 attach partition p11 for values from (2) to (5);
-alter table p attach partition p1 for values from (1, 2) to (1, 10);
+alter table mlparted1 attach partition mlparted11 for values from (2) to (5);
+alter table mlparted attach partition mlparted1 for values from (1, 2) to (1, 10);
--- check that "(1, 2)" is correctly routed to p11.
-insert into p values (1, 2);
-select tableoid::regclass, * from p;
+-- check that "(1, 2)" is correctly routed to mlparted11.
+insert into mlparted values (1, 2);
+select tableoid::regclass, * from mlparted;
--- check that proper message is shown after failure to route through p1
-insert into p (a, b) values (1, 5);
+-- check that proper message is shown after failure to route through mlparted1
+insert into mlparted (a, b) values (1, 5);
-truncate p;
-alter table p add constraint check_b check (b = 3);
--- check that correct input row is shown when constraint check_b fails on p11
+truncate mlparted;
+alter table mlparted add constraint check_b check (b = 3);
+-- check that correct input row is shown when constraint check_b fails on mlparted11
-- after "(1, 2)" is routed to it
-insert into p values (1, 2);
+insert into mlparted values (1, 2);
-- check that inserting into an internal partition successfully results in
-- checking its partition constraint before inserting into the leaf partition
-- selected by tuple-routing
-insert into p1 (a, b) values (2, 3);
+insert into mlparted1 (a, b) values (2, 3);
-- check that RETURNING works correctly with tuple-routing
-alter table p drop constraint check_b;
-create table p12 partition of p1 for values from (5) to (10);
-create table p2 (b int not null, a int not null);
-alter table p attach partition p2 for values from (1, 10) to (1, 20);
-create table p3 partition of p for values from (1, 20) to (1, 30);
-create table p4 (like p);
-alter table p4 drop a;
-alter table p4 add a int not null;
-alter table p attach partition p4 for values from (1, 30) to (1, 40);
+alter table mlparted drop constraint check_b;
+create table mlparted12 partition of mlparted1 for values from (5) to (10);
+create table mlparted2 (b int not null, a int not null);
+alter table mlparted attach partition mlparted2 for values from (1, 10) to (1, 20);
+create table mlparted3 partition of mlparted for values from (1, 20) to (1, 30);
+create table mlparted4 (like mlparted);
+alter table mlparted4 drop a;
+alter table mlparted4 add a int not null;
+alter table mlparted attach partition mlparted4 for values from (1, 30) to (1, 40);
with ins (a, b, c) as
- (insert into p (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *)
+ (insert into mlparted (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *)
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- check that message shown after failure to find a partition shows the
@@ -266,6 +266,3 @@ revoke all on key_desc from someone_else;
revoke all on key_desc_1 from someone_else;
drop role someone_else;
drop table key_desc, key_desc_1;
-
--- cleanup
-drop table p;
diff --git a/src/test/regress/sql/sanity_check.sql b/src/test/regress/sql/sanity_check.sql
index 0da838eced..fa3a90ff11 100644
--- a/src/test/regress/sql/sanity_check.sql
+++ b/src/test/regress/sql/sanity_check.sql
@@ -12,7 +12,7 @@ VACUUM;
SELECT relname, relhasindex
FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = relnamespace
- WHERE relkind = 'r' AND (nspname ~ '^pg_temp_') IS NOT TRUE
+ WHERE relkind IN ('r', 'P') AND (nspname ~ '^pg_temp_') IS NOT TRUE
ORDER BY relname;
-- restore normal output mode
--
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
