agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Allow dropping partitioned table without CASCADE
5+ messages / 4 participants
[nested] [flat]
* [PATCH] Allow dropping partitioned table without CASCADE
@ 2017-02-16 06:56 amit <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: amit @ 2017-02-16 06:56 UTC (permalink / raw)
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 18 ------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 5 -----
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/insert.sql | 7 ++-----
9 files changed, 34 insertions(+), 67 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f33aa70da6..27b6556a71 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -730,7 +732,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2245,7 +2247,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2275,7 +2278,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2288,7 +2292,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2322,7 +2327,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10749,7 +10757,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index b0e80a7788..0d1554a73e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3334,10 +3334,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_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, a int not null) partition by range (b);
@@ -3366,5 +3364,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index fc92cd92dd..bfad755e32 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -659,10 +659,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..623aa1db93 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1844,22 +1844,4 @@ explain (costs off) select * from range_list_parted where a >= 30;
(11 rows)
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+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));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..af0d5bfffe 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -220,8 +220,3 @@ DETAIL: Failing row contains (b, 9).
update range_parted set b = b + 1 where b = 10;
-- cleanup
drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 7513769359..0787e785ce 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2194,10 +2194,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2222,5 +2220,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 5f25c436ee..553c084a10 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -593,10 +593,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+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);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
--
2.11.0
--------------5730E3B4E532BCB0A73A0493
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--------------5730E3B4E532BCB0A73A0493--
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH] Allow dropping partitioned table without CASCADE
@ 2017-02-16 06:56 amit <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: amit @ 2017-02-16 06:56 UTC (permalink / raw)
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 18 ------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 5 -----
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/insert.sql | 7 ++-----
9 files changed, 34 insertions(+), 67 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..31b50ad77f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e84af67fb2..ca66158ee3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_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, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..623aa1db93 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1844,22 +1844,4 @@ explain (costs off) select * from range_list_parted where a >= 30;
(11 rows)
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+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));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..af0d5bfffe 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -220,8 +220,3 @@ DETAIL: Failing row contains (b, 9).
update range_parted set b = b + 1 where b = 10;
-- cleanup
drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index a403fd8cb4..fbcc739f41 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+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);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
--
2.11.0
--------------6DF5F0949647EE1703623582
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--------------6DF5F0949647EE1703623582--
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH 3/6] Optimize allocations in bringetbitmap
@ 2021-03-02 18:57 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Tomas Vondra @ 2021-03-02 18:57 UTC (permalink / raw)
The bringetbitmap function allocates memory for various purposes, which
may be quite expensive, depending on the number of scan keys. Instead of
allocating them separately, allocate one bit chunk of memory an carve it
into smaller pieces as needed - all the pieces have the same lifespan,
and it saves quite a bit of CPU and memory overhead.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin.c | 60 ++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 9f2656b8d9..1f82e965f9 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -373,6 +373,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
int *nkeys,
*nnullkeys;
int keyno;
+ char *ptr;
+ Size len;
+ char *tmp PG_USED_FOR_ASSERTS_ONLY;
opaque = (BrinOpaque *) scan->opaque;
bdesc = opaque->bo_bdesc;
@@ -402,15 +405,52 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* We keep null and regular keys separate, so that we can pass just the
* regular keys to the consistent function easily.
*
+ * To reduce the allocation overhead, we allocate one big chunk and then
+ * carve it into smaller arrays ourselves. All the pieces have exactly the
+ * same lifetime, so that's OK.
+ *
* XXX The widest table can have ~1600 attributes, so this may allocate a
* couple kilobytes of memory). We could invent a more compact approach
* (with just space for used attributes) but that would make the matching
* more complicated, so it may not be a win.
*/
- keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
- nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+ len =
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* regular keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts) +
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* NULL keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ ptr = palloc(len);
+ tmp = ptr;
+
+ keys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nullkeys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ nnullkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ for (int i = 0; i < bdesc->bd_tupdesc->natts; i++)
+ {
+ keys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+
+ nullkeys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+ }
+
+ Assert(tmp + len == ptr);
+
+ /* zero the number of keys */
+ memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
+ memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
/*
* Preprocess the scan keys - split them into per-attribute arrays.
@@ -444,9 +484,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
{
FmgrInfo *tmp;
- /* No key/null arrays for this attribute. */
- Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0));
- Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0));
+ /* First time we see this attribute, so no key/null keys. */
+ Assert(nkeys[keyattno - 1] == 0);
+ Assert(nnullkeys[keyattno - 1] == 0);
tmp = index_getprocinfo(idxRel, keyattno,
BRIN_PROCNUM_CONSISTENT);
@@ -457,17 +497,11 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
/* Add key to the proper per-attribute array. */
if (key->sk_flags & SK_ISNULL)
{
- if (!nullkeys[keyattno - 1])
- nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key;
nnullkeys[keyattno - 1]++;
}
else
{
- if (!keys[keyattno - 1])
- keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
keys[keyattno - 1][nkeys[keyattno - 1]] = key;
nkeys[keyattno - 1]++;
}
--
2.26.2
--------------6A37408A2C42B5B56C3B3D49
Content-Type: text/x-patch; charset=UTF-8;
name="0004-BRIN-bloom-indexes-20210303.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment;
filename="0004-BRIN-bloom-indexes-20210303.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assorted style changes with a tiny improvement
@ 2024-10-08 01:53 Michael Paquier <[email protected]>
2024-10-08 12:05 ` Re: Assorted style changes with a tiny improvement Ranier Vilela <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2024-10-08 01:53 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: pgsql-hackers
On Tue, Jul 02, 2024 at 02:39:20PM -0300, Ranier Vilela wrote:
> This is a series of patches to change styles, in assorted sources.
> IMO, this improves a tiny bit and is worth trying.
>
> 1. Avoid dereference iss_SortSupport if it has nulls.
> 2. Avoid dereference plan_node_id if no dsm area.
> 3. Avoid dereference spill partitions if zero ntuples.
> 4. Avoid possible useless palloc call with zero size.
> 5. Avoid redundant variable initialization in foreign.
> 6. Check the cheap test first in ExecMain.
> 7. Check the cheap test first in pathkeys.
> 8. Delay possible expensive bms_is_empty call in sub_select.
> 9. Reduce some scope in execPartition.
> 10. Reduce some scope for TupleDescAttr array dereference.
> 11. Remove useless duplicate test in ruleutils.
> This is already checked at line 4566.
>
> 12. Remove useless duplicate test in string_utils.
> This is already checked at line 982.
You have something here, but not everything is worth changing without
a reason to do so, other than code "correctness". For example,
bms_is_empty() is a NULL comparison, so it does not matter. I don't
see a point in the three dereference patches, either, as these states
are expected AFAIK so it does not matter. If we crash, it's actually
an indication that something has gone wrong. Same comment about the
two remove-useless patches and the two reduce-some-scope.
The point about group_keys_reorder_by_pathkeys() is to be proved; I
doubt it matters. Same for the ExecBuildSlotValueDescription()
business to check for the acl_result before bms_is_member() does not
really matter performance-wise.
The allocation in execTuplesMatchPrepare() is indeed something that
we'd better avoid, that's minimal but memory that can be avoided is
always less error-prone. pg_options_to_table() also, is a bit better
this way. Applied these two, let's move on.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assorted style changes with a tiny improvement
2024-10-08 01:53 Re: Assorted style changes with a tiny improvement Michael Paquier <[email protected]>
@ 2024-10-08 12:05 ` Ranier Vilela <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Ranier Vilela @ 2024-10-08 12:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers
Thanks Michael.
Em seg., 7 de out. de 2024 às 22:53, Michael Paquier <[email protected]>
escreveu:
> On Tue, Jul 02, 2024 at 02:39:20PM -0300, Ranier Vilela wrote:
> > This is a series of patches to change styles, in assorted sources.
> > IMO, this improves a tiny bit and is worth trying.
> >
> > 1. Avoid dereference iss_SortSupport if it has nulls.
> > 2. Avoid dereference plan_node_id if no dsm area.
> > 3. Avoid dereference spill partitions if zero ntuples.
> > 4. Avoid possible useless palloc call with zero size.
> > 5. Avoid redundant variable initialization in foreign.
> > 6. Check the cheap test first in ExecMain.
> > 7. Check the cheap test first in pathkeys.
> > 8. Delay possible expensive bms_is_empty call in sub_select.
> > 9. Reduce some scope in execPartition.
> > 10. Reduce some scope for TupleDescAttr array dereference.
> > 11. Remove useless duplicate test in ruleutils.
> > This is already checked at line 4566.
> >
> > 12. Remove useless duplicate test in string_utils.
> > This is already checked at line 982.
>
> You have something here, but not everything is worth changing without
> a reason to do so, other than code "correctness". For example,
> bms_is_empty() is a NULL comparison, so it does not matter.
Of course, this is a tiny bit of optimization and it is something laborious
for the comitter.
I don't
> see a point in the three dereference patches, either, as these states
> are expected AFAIK so it does not matter. If we crash, it's actually
> an indication that something has gone wrong.
Sorry by sorry for the confusing name patch.
Actually, these are not null pointers being dereferenced.
The point here is to avoid touching memory (cache or RAM) when it is not
strictly necessary.
> Same comment about the
> two remove-useless patches and the two reduce-some-scope.
>
Same motivation here, avoid touching memory (cache or RAM) when it is not
strictly necessary.
> The point about group_keys_reorder_by_pathkeys() is to be proved; I
> doubt it matters.
If *ec_sortref* is nonzero.
We avoid touching memory, an expensive branch and a call to an expensive
function.
> Same for the ExecBuildSlotValueDescription()
> business to check for the acl_result before bms_is_member() does not
> really matter performance-wise.
>
Same here, if *aclresult* is ACLCHECK_OK,
we avoid an expensive subtraction and an expensive function call.
>
> The allocation in execTuplesMatchPrepare() is indeed something that
> we'd better avoid, that's minimal but memory that can be avoided is
> always less error-prone. pg_options_to_table() also, is a bit better
> this way. Applied these two, let's move on.
>
Great.
Thanks for your work.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-10-08 12:05 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-02-16 06:56 [PATCH] Allow dropping partitioned table without CASCADE amit <[email protected]>
2017-02-16 06:56 [PATCH] Allow dropping partitioned table without CASCADE amit <[email protected]>
2021-03-02 18:57 [PATCH 3/6] Optimize allocations in bringetbitmap Tomas Vondra <[email protected]>
2024-10-08 01:53 Re: Assorted style changes with a tiny improvement Michael Paquier <[email protected]>
2024-10-08 12:05 ` Re: Assorted style changes with a tiny improvement Ranier Vilela <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox