Exclusion constraints on partitioned tables
Hello Hackers,
I'm trying to get things going again on my temporal tables work, and
here is a small patch to move that forward.
It lets you create exclusion constraints on partitioned tables, similar
to today's rules for b-tree primary keys & unique constraints:
just as we permit a PK on a partitioned table when the PK's columns are
a superset of the partition keys, so we could also allow an exclusion
constraint when its columns are a superset of the partition keys.
This patch also requires the matching constraint columns to use equality
comparisons (`(foo WITH =)`), so it is really equivalent to the existing
b-tree rule. Perhaps that is more conservative than necessary, but we
can't permit an arbitrary operator, since some might require testing
rows that fall into other partitions. For example `(foo WITH <>)` would
obviously cause problems.
The exclusion constraint may still include other columns beyond the
partition keys, and those may use equality operators or something else.
This patch is required to support temporal partitioned tables, because
temporal tables use exclusion constraints as their primary key.
Essentially they are `(id WITH =, valid_at with &&)`. Since the primary
key is not a b-tree, partitioning them would be forbidden prior to this
patch. But now you could partition that table on `id`, and we could
still correctly validate the temporal PK without requiring rows from
other partitions.
This patch may be helpful beyond just temporal tables (or for DIY
temporal tables), so it seems worth submitting it separately.
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
Attachments:
v1-0001-Allow-some-exclusion-constraints-on-partitions.patchtext/x-patch; charset=UTF-8; name=v1-0001-Allow-some-exclusion-constraints-on-partitions.patchDownload
From 7daadf9e822509186c9e32794d0e29effdc90edc Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Wed, 23 Nov 2022 14:55:43 -0800
Subject: [PATCH v1] Allow some exclusion constraints on partitions
Previously we only allowed UNIQUE B-tree constraints on partitions
(and only if the constraint included all the partition keys). But we
could allow exclusion constraints with the same restriction. We also
require that those columns be compared for equality, not something like
&&.
---
doc/src/sgml/ddl.sgml | 12 ++--
src/backend/commands/indexcmds.c | 36 +++++++++--
src/backend/parser/parse_utilcmd.c | 6 --
src/test/regress/expected/alter_table.out | 31 +++++++--
src/test/regress/expected/create_table.out | 16 +++--
src/test/regress/expected/indexing.out | 73 ++++++++++++++++++----
src/test/regress/sql/alter_table.sql | 29 +++++++--
src/test/regress/sql/create_table.sql | 13 +++-
src/test/regress/sql/indexing.sql | 57 +++++++++++++++--
9 files changed, 221 insertions(+), 52 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 6e92bbddd2..59be911471 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4206,11 +4206,13 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
- There is no way to create an exclusion constraint spanning the
- whole partitioned table. It is only possible to put such a
- constraint on each leaf partition individually. Again, this
- limitation stems from not being able to enforce cross-partition
- restrictions.
+ Similarly an exclusion constraint must include all the
+ partition key columns. Furthermore the constraint must compare those
+ columns for equality (not e.g. <literal>&&</literal>).
+ Again, this limitation stems from not being able to enforce
+ cross-partition restrictions. The constraint may include additional
+ columns that aren't part of the partition key, and it may compare
+ those with any operators you like.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7dc1aca8fe..28840544b5 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -709,11 +709,6 @@ DefineIndex(Oid relationId,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create index on partitioned table \"%s\" concurrently",
RelationGetRelationName(rel))));
- if (stmt->excludeOpNames)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create exclusion constraints on partitioned table \"%s\"",
- RelationGetRelationName(rel))));
}
/*
@@ -926,7 +921,7 @@ DefineIndex(Oid relationId,
* We could lift this limitation if we had global indexes, but those have
* their own problems, so this is a useful feature combination.
*/
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->primary || stmt->excludeOpNames != NIL))
{
PartitionKey key = RelationGetPartitionKey(rel);
const char *constraint_type;
@@ -983,6 +978,8 @@ DefineIndex(Oid relationId,
*/
if (accessMethodId == BTREE_AM_OID)
eq_strategy = BTEqualStrategyNumber;
+ else if (accessMethodId == GIST_AM_OID)
+ eq_strategy = RTEqualStrategyNumber;
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1020,11 +1017,38 @@ DefineIndex(Oid relationId,
idx_opcintype,
idx_opcintype,
eq_strategy);
+
+ /* For exclusion constraints, fail on columns that don't compare for equality. */
+ if (stmt->excludeOpNames != NIL && indexInfo->ii_ExclusionStrats[j] != BTEqualStrategyNumber && indexInfo->ii_ExclusionStrats[j] != RTEqualStrategyNumber)
+ {
+ Form_pg_attribute att;
+
+ att = TupleDescAttr(RelationGetDescr(rel),
+ key->partattrs[i] - 1);
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".",
+ NameStr(att->attname), get_opname(indexInfo->ii_ExclusionOps[j]))));
+ }
+
if (ptkey_eqop == idx_eqop)
{
found = true;
break;
}
+ else if (eq_strategy == RTEqualStrategyNumber)
+ {
+ /* For exclusion constraints, check BTEqualStrategy too. */
+ idx_eqop = get_opfamily_member(idx_opfamily,
+ idx_opcintype,
+ idx_opcintype,
+ BTEqualStrategyNumber);
+ if (ptkey_eqop == idx_eqop)
+ {
+ found = true;
+ break;
+ }
+ }
}
}
}
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f743cd548c..27124720fa 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -904,12 +904,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
errmsg("exclusion constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- if (cxt->ispartitioned)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("exclusion constraints are not supported on partitioned tables"),
- parser_errposition(cxt->pstate,
- constraint->location)));
cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 600e603bdf..57c3a674c8 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3797,16 +3797,35 @@ Referenced by:
TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id)
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "partitioned" lacks column "b" which is part of the partition key.
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+DROP TABLE partitioned;
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
- ^
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 5eace915a7..02fbd8b433 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -153,14 +153,18 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
ERROR: cannot use "list" partition strategy with more than one column
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 3: EXCLUDE USING gist (a WITH &&)
- ^
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 1bdd430f06..6f8b15c315 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -986,11 +986,32 @@ DETAIL: PRIMARY KEY constraint on table "idxpart" lacks column "a" which is par
-- OK if you use them in some other order
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: create table idxpart (a int, exclude (a with = )) partition ...
- ^
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
ERROR: unsupported PRIMARY KEY constraint with partition key definition
@@ -1047,12 +1068,42 @@ Indexes:
Number of partitions: 0
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: alter table idxpart add exclude (a with =);
- ^
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
create table idxpart (a int, b int, primary key (a, b)) partition by range (a, b);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f58b2f75d5..6ea923d101 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2315,14 +2315,35 @@ ALTER TABLE ataddindex
\d ataddindex
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+DROP TABLE partitioned;
+
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
ALTER TABLE partitioned DROP COLUMN b;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 93ccf77d4a..35ba4f4f2b 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -106,10 +106,17 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-- prevent using prohibited expressions in the key
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 429120e710..198a368a64 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -483,8 +483,27 @@ create table idxpart (a int, b int primary key) partition by range (b, a);
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
@@ -506,9 +525,37 @@ alter table idxpart add unique (b, a); -- this works
\d idxpart
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
--
2.25.1
Paul Jungwirth <pj@illuminatedcomputing.com> writes:
It lets you create exclusion constraints on partitioned tables, similar
to today's rules for b-tree primary keys & unique constraints:
just as we permit a PK on a partitioned table when the PK's columns are
a superset of the partition keys, so we could also allow an exclusion
constraint when its columns are a superset of the partition keys.
OK. AFAICS that works in principle.
This patch also requires the matching constraint columns to use equality
comparisons (`(foo WITH =)`), so it is really equivalent to the existing
b-tree rule.
That's not quite good enough: you'd better enforce that it's the same
equality operator (and same collation, if relevant) as is being used
in the partition key. Remember that we don't have a requirement that
a datatype have only one equality operator; and these days I think
collation can affect equality, too.
Another problem is that while we can safely assume that we know what
BTEqualStrategyNumber means in btree, we can NOT assume that we know
what gist opclass strategy numbers mean: each opclass is free to
define those as it sees fit. The part of your patch that is looking
at RTEqualStrategyNumber seems dangerously broken to me.
It might work better to consider the operator itself and ask if
it's equality in the same btree opfamily that's used by the
partition key. (Hm, do we use btree opfamilies for all types of
partitioning?)
Anyway, I think something can be made of this, but you need to be less
fuzzy about matching the equality semantics of the partition key.
regards, tom lane
On 12/15/22 16:12, Tom Lane wrote:
This patch also requires the matching constraint columns to use equality
comparisons (`(foo WITH =)`), so it is really equivalent to the existing
b-tree rule.That's not quite good enough: you'd better enforce that it's the same
equality operator (and same collation, if relevant) as is being used
in the partition key.
[snip]
It might work better to consider the operator itself and ask if
it's equality in the same btree opfamily that's used by the
partition key.
Thank you for taking a look! Here is a comparison on just the operator
itself.
I included a collation check too, but I'm not sure it's necessary.
Exclusion constraints don't have a collation per se; it comes from the
index, and we choose it just a little above in this function. (I'm not
even sure how to elicit that new error message in a test case.)
I'm not sure what to do about matching the opfamily. In practice an
exclusion constraint will typically use gist, but the partition key will
always use btree/hash. You're saying that the equals operator can be
inconsistent between those access methods? That is surprising to me, but
I admit op classes/families are still sinking in. (Even prior to this
patch, isn't the code for hash-based partitions looking up ptkey_eqop
via the hash opfamily, and then comparing it to idx_eqop looked up via
the btree opfamily?)
If partitions can only support btree-based exclusion constraints, you
still wouldn't be able to partition a temporal table, because those
constraints would always be gist. So I guess what I really want is to
support gist index constraints on partitioned tables.
Regards,
--
Paul ~{:-)
pj@illuminatedcomputing.com
Attachments:
v2-0001-Allow-some-exclusion-constraints-on-partitions.patchtext/x-patch; charset=UTF-8; name=v2-0001-Allow-some-exclusion-constraints-on-partitions.patchDownload
From df25cb63cacd992fc7453fc432488c6046e59479 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Wed, 23 Nov 2022 14:55:43 -0800
Subject: [PATCH v2] Allow some exclusion constraints on partitions
Previously we only allowed UNIQUE B-tree constraints on partitions
(and only if the constraint included all the partition keys). But we
could allow exclusion constraints with the same restriction. We also
require that those columns be compared for equality, not something like
&&.
---
doc/src/sgml/ddl.sgml | 12 ++--
src/backend/commands/indexcmds.c | 42 +++++++++++--
src/backend/parser/parse_utilcmd.c | 6 --
src/test/regress/expected/alter_table.out | 31 +++++++--
src/test/regress/expected/create_table.out | 16 +++--
src/test/regress/expected/indexing.out | 73 ++++++++++++++++++----
src/test/regress/sql/alter_table.sql | 29 +++++++--
src/test/regress/sql/create_table.sql | 13 +++-
src/test/regress/sql/indexing.sql | 57 +++++++++++++++--
9 files changed, 227 insertions(+), 52 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 6e92bbddd2..59be911471 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4206,11 +4206,13 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
- There is no way to create an exclusion constraint spanning the
- whole partitioned table. It is only possible to put such a
- constraint on each leaf partition individually. Again, this
- limitation stems from not being able to enforce cross-partition
- restrictions.
+ Similarly an exclusion constraint must include all the
+ partition key columns. Furthermore the constraint must compare those
+ columns for equality (not e.g. <literal>&&</literal>).
+ Again, this limitation stems from not being able to enforce
+ cross-partition restrictions. The constraint may include additional
+ columns that aren't part of the partition key, and it may compare
+ those with any operators you like.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7dc1aca8fe..a9ecb7df19 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -709,11 +709,6 @@ DefineIndex(Oid relationId,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create index on partitioned table \"%s\" concurrently",
RelationGetRelationName(rel))));
- if (stmt->excludeOpNames)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create exclusion constraints on partitioned table \"%s\"",
- RelationGetRelationName(rel))));
}
/*
@@ -926,7 +921,7 @@ DefineIndex(Oid relationId,
* We could lift this limitation if we had global indexes, but those have
* their own problems, so this is a useful feature combination.
*/
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->primary || stmt->excludeOpNames != NIL))
{
PartitionKey key = RelationGetPartitionKey(rel);
const char *constraint_type;
@@ -983,6 +978,8 @@ DefineIndex(Oid relationId,
*/
if (accessMethodId == BTREE_AM_OID)
eq_strategy = BTEqualStrategyNumber;
+ else if (accessMethodId == GIST_AM_OID)
+ eq_strategy = RTEqualStrategyNumber;
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1020,8 +1017,41 @@ DefineIndex(Oid relationId,
idx_opcintype,
idx_opcintype,
eq_strategy);
+
if (ptkey_eqop == idx_eqop)
{
+ /* For exclusion constraints, fail on columns that don't compare for equality. */
+ if (stmt->excludeOpNames != NIL)
+ {
+ if (idx_eqop != indexInfo->ii_ExclusionOps[j])
+ {
+ Form_pg_attribute att;
+
+ att = TupleDescAttr(RelationGetDescr(rel),
+ key->partattrs[i] - 1);
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".",
+ NameStr(att->attname), get_opname(indexInfo->ii_ExclusionOps[j]))));
+ }
+
+ /*
+ * Require a matching collation too.
+ * This should have been set correctly above, but it doesn't hurt to check.
+ */
+ if (key->partcollation[i] != collationObjectId[j])
+ {
+ Form_pg_attribute att;
+
+ att = TupleDescAttr(RelationGetDescr(rel),
+ key->partattrs[i] - 1);
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using collation \"%s\".",
+ NameStr(att->attname), get_collation_name(collationObjectId[j]))));
+ }
+ }
+
found = true;
break;
}
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f743cd548c..27124720fa 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -904,12 +904,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
errmsg("exclusion constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- if (cxt->ispartitioned)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("exclusion constraints are not supported on partitioned tables"),
- parser_errposition(cxt->pstate,
- constraint->location)));
cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 600e603bdf..57c3a674c8 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3797,16 +3797,35 @@ Referenced by:
TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id)
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "partitioned" lacks column "b" which is part of the partition key.
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+DROP TABLE partitioned;
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
- ^
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 5eace915a7..02fbd8b433 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -153,14 +153,18 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
ERROR: cannot use "list" partition strategy with more than one column
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 3: EXCLUDE USING gist (a WITH &&)
- ^
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 1bdd430f06..6f8b15c315 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -986,11 +986,32 @@ DETAIL: PRIMARY KEY constraint on table "idxpart" lacks column "a" which is par
-- OK if you use them in some other order
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: create table idxpart (a int, exclude (a with = )) partition ...
- ^
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
ERROR: unsupported PRIMARY KEY constraint with partition key definition
@@ -1047,12 +1068,42 @@ Indexes:
Number of partitions: 0
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: alter table idxpart add exclude (a with =);
- ^
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
create table idxpart (a int, b int, primary key (a, b)) partition by range (a, b);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f58b2f75d5..6ea923d101 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2315,14 +2315,35 @@ ALTER TABLE ataddindex
\d ataddindex
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+DROP TABLE partitioned;
+
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
ALTER TABLE partitioned DROP COLUMN b;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 93ccf77d4a..35ba4f4f2b 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -106,10 +106,17 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-- prevent using prohibited expressions in the key
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 429120e710..198a368a64 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -483,8 +483,27 @@ create table idxpart (a int, b int primary key) partition by range (b, a);
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
@@ -506,9 +525,37 @@ alter table idxpart add unique (b, a); -- this works
\d idxpart
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
--
2.25.1
Le vendredi 16 décembre 2022, 06:11:49 CET Paul Jungwirth a écrit :
On 12/15/22 16:12, Tom Lane wrote:
This patch also requires the matching constraint columns to use equality
comparisons (`(foo WITH =)`), so it is really equivalent to the existing
b-tree rule.That's not quite good enough: you'd better enforce that it's the same
equality operator (and same collation, if relevant) as is being used
in the partition key.
[snip]
It might work better to consider the operator itself and ask if
it's equality in the same btree opfamily that's used by the
partition key.Thank you for taking a look! Here is a comparison on just the operator
itself.
I've taken a look at the patch, and I'm not sure why you keep the restriction
on the Gist operator being of the RTEqualStrategyNumber strategy. I don't
think we have any other place where we expect those strategy numbers to
match. For hash it's different, as the hash-equality is the only operator
strategy and as such there is no other way to look at it. Can't we just
enforce partition_operator == exclusion_operator without adding the
RTEqualStrategyNumber for the opfamily into the mix ?
On 1/24/23 06:38, Ronan Dunklau wrote:
I've taken a look at the patch, and I'm not sure why you keep the restriction
on the Gist operator being of the RTEqualStrategyNumber strategy. I don't
think we have any other place where we expect those strategy numbers to
match. For hash it's different, as the hash-equality is the only operator
strategy and as such there is no other way to look at it. Can't we just
enforce partition_operator == exclusion_operator without adding the
RTEqualStrategyNumber for the opfamily into the mix ?
Thank you for taking a look! I did some research on the history of the
code here, and I think I understand Tom's concern about making sure the
index uses the same equality operator as the partition. I was confused
about his remarks about the opfamily, but I agree with you that if the
operator is the same, we should be okay.
I added the code about RTEqualStrategyNumber because that's what we need
to find an equals operator when the index is GiST (except if it's using
an opclass from btree_gist; then it needs to be BTEqual again). But then
I realized that for exclusion constraints we have already figured out
the operator (in RelationGetExclusionInfo) and put it in
indexInfo->ii_ExclusionOps. So we can just compare against that. This
works whether your index uses btree_gist or not.
Here is an updated patch with that change (also rebased).
I also included a more specific error message. If we find a matching
column in the index but with the wrong operator, we should say so, and
not say there is no matching column.
Thanks,
--
Paul ~{:-)
pj@illuminatedcomputing.com
Attachments:
v3-0001-Allow-some-exclusion-constraints-on-partitions.patchtext/x-patch; charset=UTF-8; name=v3-0001-Allow-some-exclusion-constraints-on-partitions.patchDownload
From 928a31433a4b8cad25f74017ca96b843aa30e02d Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Wed, 23 Nov 2022 14:55:43 -0800
Subject: [PATCH v3] Allow some exclusion constraints on partitions
Previously we only allowed UNIQUE B-tree constraints on partitions
(and only if the constraint included all the partition keys). But we
could allow exclusion constraints with the same restriction. We also
require that those columns be compared for equality, not something like
&&.
---
doc/src/sgml/ddl.sgml | 12 ++--
src/backend/commands/indexcmds.c | 66 ++++++++++++-------
src/backend/parser/parse_utilcmd.c | 6 --
src/test/regress/expected/alter_table.out | 31 +++++++--
src/test/regress/expected/create_table.out | 16 +++--
src/test/regress/expected/indexing.out | 73 ++++++++++++++++++----
src/test/regress/sql/alter_table.sql | 29 +++++++--
src/test/regress/sql/create_table.sql | 13 +++-
src/test/regress/sql/indexing.sql | 57 +++++++++++++++--
9 files changed, 236 insertions(+), 67 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 91c036d1cb..9d3c423ffd 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4233,11 +4233,13 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
- There is no way to create an exclusion constraint spanning the
- whole partitioned table. It is only possible to put such a
- constraint on each leaf partition individually. Again, this
- limitation stems from not being able to enforce cross-partition
- restrictions.
+ Similarly an exclusion constraint must include all the
+ partition key columns. Furthermore the constraint must compare those
+ columns for equality (not e.g. <literal>&&</literal>).
+ Again, this limitation stems from not being able to enforce
+ cross-partition restrictions. The constraint may include additional
+ columns that aren't part of the partition key, and it may compare
+ those with any operators you like.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 16ec0b114e..52d2395daa 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -709,11 +709,6 @@ DefineIndex(Oid relationId,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create index on partitioned table \"%s\" concurrently",
RelationGetRelationName(rel))));
- if (stmt->excludeOpNames)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create exclusion constraints on partitioned table \"%s\"",
- RelationGetRelationName(rel))));
}
/*
@@ -918,15 +913,16 @@ DefineIndex(Oid relationId,
index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
- * If this table is partitioned and we're creating a unique index or a
- * primary key, make sure that the partition key is a subset of the
- * index's columns. Otherwise it would be possible to violate uniqueness
- * by putting values that ought to be unique in different partitions.
+ * If this table is partitioned and we're creating a unique index,
+ * primary key, or exclusion constraint, make sure that the partition key
+ * is a subset of the index's columns. Otherwise it would be possible to
+ * violate uniqueness by putting values that ought to be unique in
+ * different partitions.
*
* We could lift this limitation if we had global indexes, but those have
* their own problems, so this is a useful feature combination.
*/
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->primary || stmt->excludeOpNames != NIL))
{
PartitionKey key = RelationGetPartitionKey(rel);
const char *constraint_type;
@@ -979,15 +975,22 @@ DefineIndex(Oid relationId,
* We'll need to be able to identify the equality operators
* associated with index columns, too. We know what to do with
* btree opclasses; if there are ever any other index types that
- * support unique indexes, this logic will need extension.
+ * support unique indexes, this logic will need extension. But
+ * if we have an exclusion constraint, it already knows the
+ * operators, so we don't have to infer them.
*/
- if (accessMethodId == BTREE_AM_OID)
- eq_strategy = BTEqualStrategyNumber;
+ if (stmt->excludeOpNames == NIL)
+ {
+ if (accessMethodId == BTREE_AM_OID)
+ eq_strategy = BTEqualStrategyNumber;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to an index using access method \"%s\"",
+ accessMethodName)));
+ }
else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot match partition key to an index using access method \"%s\"",
- accessMethodName)));
+ eq_strategy = InvalidStrategy;
/*
* It may be possible to support UNIQUE constraints when partition
@@ -1016,15 +1019,36 @@ DefineIndex(Oid relationId,
{
Oid idx_eqop;
- idx_eqop = get_opfamily_member(idx_opfamily,
- idx_opcintype,
- idx_opcintype,
- eq_strategy);
+ if (stmt->excludeOpNames != NIL)
+ idx_eqop = indexInfo->ii_ExclusionOps[j];
+ else
+ idx_eqop = get_opfamily_member(idx_opfamily,
+ idx_opcintype,
+ idx_opcintype,
+ eq_strategy);
+
if (ptkey_eqop == idx_eqop)
{
found = true;
break;
}
+ else
+ {
+ /*
+ * We found a matching column, but the index doesn't use
+ * an equality operator. Instead of failing below with
+ * an error message about a missing column, fail now and
+ * explain that the operator is wrong.
+ */
+ Form_pg_attribute att;
+
+ att = TupleDescAttr(RelationGetDescr(rel),
+ key->partattrs[i] - 1);
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".",
+ NameStr(att->attname), get_opname(indexInfo->ii_ExclusionOps[j]))));
+ }
}
}
}
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f9218f48aa..9a5c5353ae 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -899,12 +899,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
errmsg("exclusion constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- if (cxt->ispartitioned)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("exclusion constraints are not supported on partitioned tables"),
- parser_errposition(cxt->pstate,
- constraint->location)));
cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 27b4d7dc96..1b4007cac9 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3826,16 +3826,35 @@ Referenced by:
TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id)
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "partitioned" lacks column "b" which is part of the partition key.
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+DROP TABLE partitioned;
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
- ^
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 5eace915a7..02fbd8b433 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -153,14 +153,18 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
ERROR: cannot use "list" partition strategy with more than one column
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 3: EXCLUDE USING gist (a WITH &&)
- ^
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 1bdd430f06..6f8b15c315 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -986,11 +986,32 @@ DETAIL: PRIMARY KEY constraint on table "idxpart" lacks column "a" which is par
-- OK if you use them in some other order
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: create table idxpart (a int, exclude (a with = )) partition ...
- ^
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
ERROR: unsupported PRIMARY KEY constraint with partition key definition
@@ -1047,12 +1068,42 @@ Indexes:
Number of partitions: 0
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: alter table idxpart add exclude (a with =);
- ^
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
create table idxpart (a int, b int, primary key (a, b)) partition by range (a, b);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 7dc9e3d632..aa796f38c0 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2333,14 +2333,35 @@ ALTER TABLE ataddindex
\d ataddindex
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- supported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint parts for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH =);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ b int4range
+) PARTITION BY RANGE (a, b);
+ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH -|-);
+DROP TABLE partitioned;
+
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
ALTER TABLE partitioned DROP COLUMN b;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 93ccf77d4a..35ba4f4f2b 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -106,10 +106,17 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
--- unsupported constraint type for partitioned tables
+-- exclusion constraint type for partitioned tables
CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
+ a int4range,
+ EXCLUDE USING gist (a WITH =)
+) PARTITION BY RANGE (a);
+DROP TABLE partitioned;
+
+-- unsupported exclusion constraint operator for partitioned tables
+CREATE TABLE partitioned (
+ a int4range,
+ EXCLUDE USING gist (a WITH -|-)
) PARTITION BY RANGE (a);
-- prevent using prohibited expressions in the key
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 429120e710..198a368a64 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -483,8 +483,27 @@ create table idxpart (a int, b int primary key) partition by range (b, a);
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
@@ -506,9 +525,37 @@ alter table idxpart add unique (b, a); -- this works
\d idxpart
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
+-- Exclusion constraints can be added if a partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
--
2.25.1
Le vendredi 17 mars 2023, 17:03:09 CET Paul Jungwirth a écrit :
I added the code about RTEqualStrategyNumber because that's what we need
to find an equals operator when the index is GiST (except if it's using
an opclass from btree_gist; then it needs to be BTEqual again). But then
I realized that for exclusion constraints we have already figured out
the operator (in RelationGetExclusionInfo) and put it in
indexInfo->ii_ExclusionOps. So we can just compare against that. This
works whether your index uses btree_gist or not.Here is an updated patch with that change (also rebased).
Thanks ! This looks fine to me like this.
I also included a more specific error message. If we find a matching
column in the index but with the wrong operator, we should say so, and
not say there is no matching column.
I agree that's a nicer improvement.
Regards,
--
Ronan Dunklau
On 17.03.23 17:03, Paul Jungwirth wrote:
Thank you for taking a look! I did some research on the history of the
code here, and I think I understand Tom's concern about making sure the
index uses the same equality operator as the partition. I was confused
about his remarks about the opfamily, but I agree with you that if the
operator is the same, we should be okay.I added the code about RTEqualStrategyNumber because that's what we need
to find an equals operator when the index is GiST (except if it's using
an opclass from btree_gist; then it needs to be BTEqual again). But then
I realized that for exclusion constraints we have already figured out
the operator (in RelationGetExclusionInfo) and put it in
indexInfo->ii_ExclusionOps. So we can just compare against that. This
works whether your index uses btree_gist or not.Here is an updated patch with that change (also rebased).
I also included a more specific error message. If we find a matching
column in the index but with the wrong operator, we should say so, and
not say there is no matching column.
This looks all pretty good to me. A few more comments:
It seems to me that many of the test cases added in indexing.sql are
redundant with create_table.sql/alter_table.sql (or vice versa). Is
there a reason for this?
This is not really a problem in your patch, but I think in
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->primary ||
stmt->excludeOpNames != NIL))
the stmt->primary is redundant and should be removed. Right now
"primary" is always a subset of "unique", but presumably a future patch
of yours wants to change that.
Furthermore, I think it would be more elegant in your patch if you wrote
stmt->excludeOpNames without the "== NIL" or "!= NIL", so that it
becomes a peer of stmt->unique. (I understand some people don't like
that style. But it is already used in that file.)
I would consider rearranging some of the conditionals more as a
selection of cases, like "is it a unique constraint?", "else, is it an
exclusion constraint?" -- rather than the current "is it an exclusion
constraint?, "else, various old code". For example, instead of
if (stmt->excludeOpNames != NIL)
idx_eqop = indexInfo->ii_ExclusionOps[j];
else
idx_eqop = get_opfamily_member(..., eq_strategy);
consider
if (stmt->unique)
idx_eqop = get_opfamily_member(..., eq_strategy);
else if (stmt->excludeOpNames)
idx_eqop = indexInfo->ii_ExclusionOps[j];
Assert(idx_eqop);
Also, I would push the code
if (accessMethodId == BTREE_AM_OID)
eq_strategy = BTEqualStrategyNumber;
further down into the loop, so that you don't have to remember in which
cases eq_strategy is assigned or not.
(It's also confusing that the eq_strategy variable is used for two
different things in this function, and that would clean that up.)
Finally, this code
+ att = TupleDescAttr(RelationGetDescr(rel),
+ key->partattrs[i] - 1);
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key
to index on column \"%s\" using non-equal operator \"%s\".",
+ NameStr(att->attname),
get_opname(indexInfo->ii_ExclusionOps[j]))));
could be simplified by using get_attname().
This is all just a bit of polishing. I think it would be good to go
after that.
On Thu, Jul 6, 2023 at 1:03 AM Peter Eisentraut <peter@eisentraut.org> wrote:
This looks all pretty good to me. A few more comments:
Thanks for the feedback! New patch attached here. Responses below:
It seems to me that many of the test cases added in indexing.sql are
redundant with create_table.sql/alter_table.sql (or vice versa). Is
there a reason for this?
Yes, there is some overlap. I think that's just because there was
overlap before, and I didn't want to delete the old tests completely.
But since indexing.sql has a fuller list of tests and is a superset of
the others, this new patch removes the redundant tests from
{create,alter}_table.sql.
Btw speaking of tests, I want to make sure this new feature will still
work when you're using btree_gist and and `EXCLUDE WITH (myint =,
mytsrange &&)` (and not just `(myint4range =, mytsrange &&)`). Some of
my early attempts writing this patch worked w/o btree_gist but not w/
(or vice versa). But as far as I know there's no way to test that in
regress. I wound up writing a private shell script that just does
this:
```
--------
-- test against btree_gist since we can't do that in the postgres
regress test suite:
CREATE EXTENSION btree_gist;
create table partitioned (id int, valid_at tsrange, exclude using gist
(id with =, valid_at with &&)) partition by range (id);
-- should fail with a good error message:
create table partitioned2 (id int, valid_at tsrange, exclude using
gist (id with <>, valid_at with &&)) partition by range (id);
```
Is there some place in the repo to include a test like that? It seems
a little funny to put it in the btree_gist suite, but maybe that's the
right answer.
This is not really a problem in your patch, but I think in
- if (partitioned && (stmt->unique || stmt->primary)) + if (partitioned && (stmt->unique || stmt->primary || stmt->excludeOpNames != NIL))the stmt->primary is redundant and should be removed. Right now
"primary" is always a subset of "unique", but presumably a future patch
of yours wants to change that.
Done! I don't think my temporal work changes that primary ⊆ unique. It
does change that some primary/unique constraints will have non-null
excludeOpNames, which will require small changes here eventually. But
that should be part of the temporal patches, not this one.
Furthermore, I think it would be more elegant in your patch if you wrote
stmt->excludeOpNames without the "== NIL" or "!= NIL", so that it
becomes a peer of stmt->unique. (I understand some people don't like
that style. But it is already used in that file.)
Done.
I would consider rearranging some of the conditionals more as a
selection of cases, like "is it a unique constraint?", "else, is it an
exclusion constraint?" -- rather than the current "is it an exclusion
constraint?, "else, various old code".
Done.
Also, I would push the code
if (accessMethodId == BTREE_AM_OID)
eq_strategy = BTEqualStrategyNumber;further down into the loop, so that you don't have to remember in which
cases eq_strategy is assigned or not.(It's also confusing that the eq_strategy variable is used for two
different things in this function, and that would clean that up.)
Agreed that it's confusing. Done.
Finally, this code
+ att = TupleDescAttr(RelationGetDescr(rel), + key->partattrs[i] - 1); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".", + NameStr(att->attname), get_opname(indexInfo->ii_ExclusionOps[j]))));could be simplified by using get_attname().
Okay, done. I changed the similar error message just below too.
This is all just a bit of polishing. I think it would be good to go
after that.
Thanks!
--
Paul ~{:-)
pj@illuminatedcomputing.com
Attachments:
v4-0001-Allow-some-exclusion-constraints-on-partitions.patchapplication/x-patch; name=v4-0001-Allow-some-exclusion-constraints-on-partitions.patchDownload
From ca3a54d78dce9b2d553f37e769ccc65fbf579f42 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Wed, 23 Nov 2022 14:55:43 -0800
Subject: [PATCH v4] Allow some exclusion constraints on partitions
Previously we only allowed UNIQUE B-tree constraints on partitions
(and only if the constraint included all the partition keys). But we
could allow exclusion constraints with the same restriction. We also
require that those columns be compared for equality, not something like
&&.
---
doc/src/sgml/ddl.sgml | 12 ++--
src/backend/commands/indexcmds.c | 63 ++++++++++---------
src/backend/parser/parse_utilcmd.c | 6 --
src/test/regress/expected/alter_table.out | 7 +--
src/test/regress/expected/create_table.out | 8 ---
src/test/regress/expected/indexing.out | 73 ++++++++++++++++++----
src/test/regress/sql/alter_table.sql | 5 +-
src/test/regress/sql/create_table.sql | 6 --
src/test/regress/sql/indexing.sql | 57 +++++++++++++++--
9 files changed, 158 insertions(+), 79 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index e32f8253d0..92cadd2fd2 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4233,11 +4233,13 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
- There is no way to create an exclusion constraint spanning the
- whole partitioned table. It is only possible to put such a
- constraint on each leaf partition individually. Again, this
- limitation stems from not being able to enforce cross-partition
- restrictions.
+ Similarly an exclusion constraint must include all the
+ partition key columns. Furthermore the constraint must compare those
+ columns for equality (not e.g. <literal>&&</literal>).
+ Again, this limitation stems from not being able to enforce
+ cross-partition restrictions. The constraint may include additional
+ columns that aren't part of the partition key, and it may compare
+ those with any operators you like.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 403f5fc143..5e537dc97a 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -713,11 +713,6 @@ DefineIndex(Oid relationId,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create index on partitioned table \"%s\" concurrently",
RelationGetRelationName(rel))));
- if (stmt->excludeOpNames)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create exclusion constraints on partitioned table \"%s\"",
- RelationGetRelationName(rel))));
}
/*
@@ -924,15 +919,16 @@ DefineIndex(Oid relationId,
index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
- * If this table is partitioned and we're creating a unique index or a
- * primary key, make sure that the partition key is a subset of the
- * index's columns. Otherwise it would be possible to violate uniqueness
- * by putting values that ought to be unique in different partitions.
+ * If this table is partitioned and we're creating a unique index,
+ * primary key, or exclusion constraint, make sure that the partition key
+ * is a subset of the index's columns. Otherwise it would be possible to
+ * violate uniqueness by putting values that ought to be unique in
+ * different partitions.
*
* We could lift this limitation if we had global indexes, but those have
* their own problems, so this is a useful feature combination.
*/
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->excludeOpNames))
{
PartitionKey key = RelationGetPartitionKey(rel);
const char *constraint_type;
@@ -942,7 +938,7 @@ DefineIndex(Oid relationId,
constraint_type = "PRIMARY KEY";
else if (stmt->unique)
constraint_type = "UNIQUE";
- else if (stmt->excludeOpNames != NIL)
+ else if (stmt->excludeOpNames)
constraint_type = "EXCLUDE";
else
{
@@ -985,11 +981,11 @@ DefineIndex(Oid relationId,
* We'll need to be able to identify the equality operators
* associated with index columns, too. We know what to do with
* btree opclasses; if there are ever any other index types that
- * support unique indexes, this logic will need extension.
+ * support unique indexes, this logic will need extension. But
+ * if we have an exclusion constraint, it already knows the
+ * operators, so we don't have to infer them.
*/
- if (accessMethodId == BTREE_AM_OID)
- eq_strategy = BTEqualStrategyNumber;
- else
+ if (stmt->unique && accessMethodId != BTREE_AM_OID)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot match partition key to an index using access method \"%s\"",
@@ -1020,34 +1016,45 @@ DefineIndex(Oid relationId,
&idx_opfamily,
&idx_opcintype))
{
- Oid idx_eqop;
+ Oid idx_eqop = InvalidOid;
+
+ if (stmt->unique)
+ idx_eqop = get_opfamily_member(idx_opfamily,
+ idx_opcintype,
+ idx_opcintype,
+ BTEqualStrategyNumber);
+ else if (stmt->excludeOpNames)
+ idx_eqop = indexInfo->ii_ExclusionOps[j];
+ Assert(idx_eqop);
- idx_eqop = get_opfamily_member(idx_opfamily,
- idx_opcintype,
- idx_opcintype,
- eq_strategy);
if (ptkey_eqop == idx_eqop)
{
found = true;
break;
}
+ else if (stmt->excludeOpNames)
+ /*
+ * We found a match, but it's not an equality operator.
+ * Instead of failing below with an error message about
+ * a missing column, fail now and explain that the
+ * operator is wrong.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".",
+ get_attname(relationId, key->partattrs[i], false),
+ get_opname(indexInfo->ii_ExclusionOps[j]))));
}
}
}
if (!found)
- {
- Form_pg_attribute att;
-
- att = TupleDescAttr(RelationGetDescr(rel),
- key->partattrs[i] - 1);
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("unique constraint on partitioned table must include all partitioning columns"),
errdetail("%s constraint on table \"%s\" lacks column \"%s\" which is part of the partition key.",
constraint_type, RelationGetRelationName(rel),
- NameStr(att->attname))));
- }
+ get_attname(relationId, key->partattrs[i], false))));
}
}
@@ -1102,7 +1109,7 @@ DefineIndex(Oid relationId,
constraint_type = "PRIMARY KEY";
else if (stmt->unique)
constraint_type = "UNIQUE";
- else if (stmt->excludeOpNames != NIL)
+ else if (stmt->excludeOpNames)
constraint_type = "EXCLUDE";
else
{
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index d67580fc77..e48e9e99d3 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -900,12 +900,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
errmsg("exclusion constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- if (cxt->ispartitioned)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("exclusion constraints are not supported on partitioned tables"),
- parser_errposition(cxt->pstate,
- constraint->location)));
cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 3b708c7976..8315c30ea2 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3834,16 +3834,11 @@ Referenced by:
TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id)
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
- ^
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 5eace915a7..92478f0344 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -153,14 +153,6 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
ERROR: cannot use "list" partition strategy with more than one column
--- unsupported constraint type for partitioned tables
-CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
-) PARTITION BY RANGE (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 3: EXCLUDE USING gist (a WITH &&)
- ^
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 3e5645c2ab..551ca44540 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -986,11 +986,32 @@ DETAIL: PRIMARY KEY constraint on table "idxpart" lacks column "a" which is par
-- OK if you use them in some other order
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: create table idxpart (a int, exclude (a with = )) partition ...
- ^
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
ERROR: unsupported PRIMARY KEY constraint with partition key definition
@@ -1047,12 +1068,42 @@ Indexes:
Number of partitions: 0
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: alter table idxpart add exclude (a with =);
- ^
+-- Exclusion constraints can be added if partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
create table idxpart (a int, b int, primary key (a, b)) partition by range (a, b);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 58ea20ac3d..ff8c498419 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2342,14 +2342,11 @@ ALTER TABLE ataddindex
\d ataddindex
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
ALTER TABLE partitioned DROP COLUMN b;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 93ccf77d4a..82ada47661 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -106,12 +106,6 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
--- unsupported constraint type for partitioned tables
-CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
-) PARTITION BY RANGE (a);
-
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index d6e5a06d95..b69c41832c 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -483,8 +483,27 @@ create table idxpart (a int, b int primary key) partition by range (b, a);
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
@@ -506,9 +525,37 @@ alter table idxpart add unique (b, a); -- this works
\d idxpart
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
+-- Exclusion constraints can be added if partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
--
2.32.0 (Apple Git-132)
On 09.07.23 03:21, Paul A Jungwirth wrote:
It seems to me that many of the test cases added in indexing.sql are
redundant with create_table.sql/alter_table.sql (or vice versa). Is
there a reason for this?Yes, there is some overlap. I think that's just because there was
overlap before, and I didn't want to delete the old tests completely.
But since indexing.sql has a fuller list of tests and is a superset of
the others, this new patch removes the redundant tests from
{create,alter}_table.sql.
This looks better.
Btw speaking of tests, I want to make sure this new feature will still
work when you're using btree_gist and and `EXCLUDE WITH (myint =,
mytsrange &&)` (and not just `(myint4range =, mytsrange &&)`). Some of
my early attempts writing this patch worked w/o btree_gist but not w/
(or vice versa). But as far as I know there's no way to test that in
regress. I wound up writing a private shell script that just does
this:
Is there some place in the repo to include a test like that? It seems
a little funny to put it in the btree_gist suite, but maybe that's the
right answer.
I'm not sure what value we would get from testing this with btree_gist,
but if we wanted to do that, then adding a new test file to the
btree_gist sql/ directory would seem reasonable to me.
(I would make the test a little bit bigger than you had shown, like
insert a few values.)
If you want to do that, please send another patch. Otherwise, I'm ok to
commit this one.
On Mon, Jul 10, 2023 at 7:05 AM Peter Eisentraut <peter@eisentraut.org> wrote:
I'm not sure what value we would get from testing this with btree_gist,
but if we wanted to do that, then adding a new test file to the
btree_gist sql/ directory would seem reasonable to me.(I would make the test a little bit bigger than you had shown, like
insert a few values.)If you want to do that, please send another patch. Otherwise, I'm ok to
commit this one.
I can get you a patch tonight or tomorrow. I think it's worth it since
btree_gist uses different strategy numbers than ordinary gist.
Thanks!
Paul
On Mon, Jul 10, 2023 at 8:06 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:
On Mon, Jul 10, 2023 at 7:05 AM Peter Eisentraut <peter@eisentraut.org> wrote:
I'm not sure what value we would get from testing this with btree_gist,
but if we wanted to do that, then adding a new test file to the
btree_gist sql/ directory would seem reasonable to me.(I would make the test a little bit bigger than you had shown, like
insert a few values.)If you want to do that, please send another patch. Otherwise, I'm ok to
commit this one.I can get you a patch tonight or tomorrow. I think it's worth it since
btree_gist uses different strategy numbers than ordinary gist.
Patch attached.
Regards,
Paul
Attachments:
v5-0001-Allow-some-exclusion-constraints-on-partitions.patchapplication/octet-stream; name=v5-0001-Allow-some-exclusion-constraints-on-partitions.patchDownload
From a426b7d75f8e1b8867978ec5afc5415353588e0b Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Wed, 23 Nov 2022 14:55:43 -0800
Subject: [PATCH v5] Allow some exclusion constraints on partitions
Previously we only allowed UNIQUE B-tree constraints on partitions
(and only if the constraint included all the partition keys). But we
could allow exclusion constraints with the same restriction. We also
require that those columns be compared for equality, not something like
&&.
---
contrib/btree_gist/Makefile | 2 +-
contrib/btree_gist/expected/partitions.out | 82 ++++++++++++++++++++++
contrib/btree_gist/meson.build | 1 +
contrib/btree_gist/sql/partitions.sql | 39 ++++++++++
doc/src/sgml/ddl.sgml | 12 ++--
src/backend/commands/indexcmds.c | 63 +++++++++--------
src/backend/parser/parse_utilcmd.c | 6 --
src/test/regress/expected/alter_table.out | 7 +-
src/test/regress/expected/create_table.out | 8 ---
src/test/regress/expected/indexing.out | 73 ++++++++++++++++---
src/test/regress/sql/alter_table.sql | 5 +-
src/test/regress/sql/create_table.sql | 6 --
src/test/regress/sql/indexing.sql | 57 +++++++++++++--
13 files changed, 281 insertions(+), 80 deletions(-)
create mode 100644 contrib/btree_gist/expected/partitions.out
create mode 100644 contrib/btree_gist/sql/partitions.sql
diff --git a/contrib/btree_gist/Makefile b/contrib/btree_gist/Makefile
index 48997c75f6..073dcc745c 100644
--- a/contrib/btree_gist/Makefile
+++ b/contrib/btree_gist/Makefile
@@ -38,7 +38,7 @@ PGFILEDESC = "btree_gist - B-tree equivalent GiST operator classes"
REGRESS = init int2 int4 int8 float4 float8 cash oid timestamp timestamptz \
time timetz date interval macaddr macaddr8 inet cidr text varchar char \
- bytea bit varbit numeric uuid not_equal enum bool
+ bytea bit varbit numeric uuid not_equal enum bool partitions
SHLIB_LINK += $(filter -lm, $(LIBS))
diff --git a/contrib/btree_gist/expected/partitions.out b/contrib/btree_gist/expected/partitions.out
new file mode 100644
index 0000000000..2dc6d0c760
--- /dev/null
+++ b/contrib/btree_gist/expected/partitions.out
@@ -0,0 +1,82 @@
+-- Make sure we can create an exclusion constraint
+-- across a partitioned table.
+-- That code looks at strategy numbers that can differ in regular gist vs btree_gist,
+-- so we want to make sure it works here too.
+create table parttmp (
+ id int,
+ valid_at daterange,
+ exclude using gist (id with =, valid_at with &&)
+) partition by range (id);
+create table parttmp_1_to_10 partition of parttmp for values from (1) to (10);
+create table parttmp_11_to_20 partition of parttmp for values from (11) to (20);
+insert into parttmp (id, valid_at) values
+ (1, '[2000-01-01, 2000-02-01)'),
+ (1, '[2000-02-01, 2000-03-01)'),
+ (2, '[2000-01-01, 2000-02-01)'),
+ (11, '[2000-01-01, 2000-02-01)'),
+ (11, '[2000-02-01, 2000-03-01)'),
+ (12, '[2000-01-01, 2000-02-01)');
+select * from parttmp order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 1 | [01-01-2000,02-01-2000)
+ 1 | [02-01-2000,03-01-2000)
+ 2 | [01-01-2000,02-01-2000)
+ 11 | [01-01-2000,02-01-2000)
+ 11 | [02-01-2000,03-01-2000)
+ 12 | [01-01-2000,02-01-2000)
+(6 rows)
+
+select * from parttmp_1_to_10 order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 1 | [01-01-2000,02-01-2000)
+ 1 | [02-01-2000,03-01-2000)
+ 2 | [01-01-2000,02-01-2000)
+(3 rows)
+
+select * from parttmp_11_to_20 order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 11 | [01-01-2000,02-01-2000)
+ 11 | [02-01-2000,03-01-2000)
+ 12 | [01-01-2000,02-01-2000)
+(3 rows)
+
+update parttmp set valid_at = valid_at * '[2000-01-15,2000-02-15)' where id = 1;
+select * from parttmp order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 1 | [01-15-2000,02-01-2000)
+ 1 | [02-01-2000,02-15-2000)
+ 2 | [01-01-2000,02-01-2000)
+ 11 | [01-01-2000,02-01-2000)
+ 11 | [02-01-2000,03-01-2000)
+ 12 | [01-01-2000,02-01-2000)
+(6 rows)
+
+select * from parttmp_1_to_10 order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 1 | [01-15-2000,02-01-2000)
+ 1 | [02-01-2000,02-15-2000)
+ 2 | [01-01-2000,02-01-2000)
+(3 rows)
+
+select * from parttmp_11_to_20 order by id, valid_at;
+ id | valid_at
+----+-------------------------
+ 11 | [01-01-2000,02-01-2000)
+ 11 | [02-01-2000,03-01-2000)
+ 12 | [01-01-2000,02-01-2000)
+(3 rows)
+
+-- make sure the excluson constraint excludes:
+insert into parttmp (id, valid_at) values
+ (2, '[2000-01-15, 2000-02-01)');
+ERROR: conflicting key value violates exclusion constraint "parttmp_1_to_10_id_valid_at_excl"
+DETAIL: Key (id, valid_at)=(2, [01-15-2000,02-01-2000)) conflicts with existing key (id, valid_at)=(2, [01-01-2000,02-01-2000)).
+drop table parttmp;
+-- should fail with a good error message:
+create table parttmp (id int, valid_at daterange, exclude using gist (id with <>, valid_at with &&)) partition by range (id);
+ERROR: cannot match partition key to index on column "id" using non-equal operator "<>".
diff --git a/contrib/btree_gist/meson.build b/contrib/btree_gist/meson.build
index 5811026301..087c5b8d4b 100644
--- a/contrib/btree_gist/meson.build
+++ b/contrib/btree_gist/meson.build
@@ -88,6 +88,7 @@ tests += {
'not_equal',
'enum',
'bool',
+ 'partitions',
],
},
}
diff --git a/contrib/btree_gist/sql/partitions.sql b/contrib/btree_gist/sql/partitions.sql
new file mode 100644
index 0000000000..6265c10121
--- /dev/null
+++ b/contrib/btree_gist/sql/partitions.sql
@@ -0,0 +1,39 @@
+-- Make sure we can create an exclusion constraint
+-- across a partitioned table.
+-- That code looks at strategy numbers that can differ in regular gist vs btree_gist,
+-- so we want to make sure it works here too.
+create table parttmp (
+ id int,
+ valid_at daterange,
+ exclude using gist (id with =, valid_at with &&)
+) partition by range (id);
+
+create table parttmp_1_to_10 partition of parttmp for values from (1) to (10);
+create table parttmp_11_to_20 partition of parttmp for values from (11) to (20);
+
+insert into parttmp (id, valid_at) values
+ (1, '[2000-01-01, 2000-02-01)'),
+ (1, '[2000-02-01, 2000-03-01)'),
+ (2, '[2000-01-01, 2000-02-01)'),
+ (11, '[2000-01-01, 2000-02-01)'),
+ (11, '[2000-02-01, 2000-03-01)'),
+ (12, '[2000-01-01, 2000-02-01)');
+
+select * from parttmp order by id, valid_at;
+select * from parttmp_1_to_10 order by id, valid_at;
+select * from parttmp_11_to_20 order by id, valid_at;
+
+update parttmp set valid_at = valid_at * '[2000-01-15,2000-02-15)' where id = 1;
+
+select * from parttmp order by id, valid_at;
+select * from parttmp_1_to_10 order by id, valid_at;
+select * from parttmp_11_to_20 order by id, valid_at;
+
+-- make sure the excluson constraint excludes:
+insert into parttmp (id, valid_at) values
+ (2, '[2000-01-15, 2000-02-01)');
+
+drop table parttmp;
+
+-- should fail with a good error message:
+create table parttmp (id int, valid_at daterange, exclude using gist (id with <>, valid_at with &&)) partition by range (id);
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 4317995965..58aaa691c6 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4216,11 +4216,13 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
- There is no way to create an exclusion constraint spanning the
- whole partitioned table. It is only possible to put such a
- constraint on each leaf partition individually. Again, this
- limitation stems from not being able to enforce cross-partition
- restrictions.
+ Similarly an exclusion constraint must include all the
+ partition key columns. Furthermore the constraint must compare those
+ columns for equality (not e.g. <literal>&&</literal>).
+ Again, this limitation stems from not being able to enforce
+ cross-partition restrictions. The constraint may include additional
+ columns that aren't part of the partition key, and it may compare
+ those with any operators you like.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 02250ae74b..d319f59a4e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -712,11 +712,6 @@ DefineIndex(Oid relationId,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create index on partitioned table \"%s\" concurrently",
RelationGetRelationName(rel))));
- if (stmt->excludeOpNames)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create exclusion constraints on partitioned table \"%s\"",
- RelationGetRelationName(rel))));
}
/*
@@ -923,15 +918,16 @@ DefineIndex(Oid relationId,
index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
- * If this table is partitioned and we're creating a unique index or a
- * primary key, make sure that the partition key is a subset of the
- * index's columns. Otherwise it would be possible to violate uniqueness
- * by putting values that ought to be unique in different partitions.
+ * If this table is partitioned and we're creating a unique index,
+ * primary key, or exclusion constraint, make sure that the partition key
+ * is a subset of the index's columns. Otherwise it would be possible to
+ * violate uniqueness by putting values that ought to be unique in
+ * different partitions.
*
* We could lift this limitation if we had global indexes, but those have
* their own problems, so this is a useful feature combination.
*/
- if (partitioned && (stmt->unique || stmt->primary))
+ if (partitioned && (stmt->unique || stmt->excludeOpNames))
{
PartitionKey key = RelationGetPartitionKey(rel);
const char *constraint_type;
@@ -941,7 +937,7 @@ DefineIndex(Oid relationId,
constraint_type = "PRIMARY KEY";
else if (stmt->unique)
constraint_type = "UNIQUE";
- else if (stmt->excludeOpNames != NIL)
+ else if (stmt->excludeOpNames)
constraint_type = "EXCLUDE";
else
{
@@ -984,11 +980,11 @@ DefineIndex(Oid relationId,
* We'll need to be able to identify the equality operators
* associated with index columns, too. We know what to do with
* btree opclasses; if there are ever any other index types that
- * support unique indexes, this logic will need extension.
+ * support unique indexes, this logic will need extension. But
+ * if we have an exclusion constraint, it already knows the
+ * operators, so we don't have to infer them.
*/
- if (accessMethodId == BTREE_AM_OID)
- eq_strategy = BTEqualStrategyNumber;
- else
+ if (stmt->unique && accessMethodId != BTREE_AM_OID)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot match partition key to an index using access method \"%s\"",
@@ -1019,34 +1015,45 @@ DefineIndex(Oid relationId,
&idx_opfamily,
&idx_opcintype))
{
- Oid idx_eqop;
+ Oid idx_eqop = InvalidOid;
+
+ if (stmt->unique)
+ idx_eqop = get_opfamily_member(idx_opfamily,
+ idx_opcintype,
+ idx_opcintype,
+ BTEqualStrategyNumber);
+ else if (stmt->excludeOpNames)
+ idx_eqop = indexInfo->ii_ExclusionOps[j];
+ Assert(idx_eqop);
- idx_eqop = get_opfamily_member(idx_opfamily,
- idx_opcintype,
- idx_opcintype,
- eq_strategy);
if (ptkey_eqop == idx_eqop)
{
found = true;
break;
}
+ else if (stmt->excludeOpNames)
+ /*
+ * We found a match, but it's not an equality operator.
+ * Instead of failing below with an error message about
+ * a missing column, fail now and explain that the
+ * operator is wrong.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot match partition key to index on column \"%s\" using non-equal operator \"%s\".",
+ get_attname(relationId, key->partattrs[i], false),
+ get_opname(indexInfo->ii_ExclusionOps[j]))));
}
}
}
if (!found)
- {
- Form_pg_attribute att;
-
- att = TupleDescAttr(RelationGetDescr(rel),
- key->partattrs[i] - 1);
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("unique constraint on partitioned table must include all partitioning columns"),
errdetail("%s constraint on table \"%s\" lacks column \"%s\" which is part of the partition key.",
constraint_type, RelationGetRelationName(rel),
- NameStr(att->attname))));
- }
+ get_attname(relationId, key->partattrs[i], false))));
}
}
@@ -1101,7 +1108,7 @@ DefineIndex(Oid relationId,
constraint_type = "PRIMARY KEY";
else if (stmt->unique)
constraint_type = "UNIQUE";
- else if (stmt->excludeOpNames != NIL)
+ else if (stmt->excludeOpNames)
constraint_type = "EXCLUDE";
else
{
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index d67580fc77..e48e9e99d3 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -900,12 +900,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
errmsg("exclusion constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- if (cxt->ispartitioned)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("exclusion constraints are not supported on partitioned tables"),
- parser_errposition(cxt->pstate,
- constraint->location)));
cxt->ixconstraints = lappend(cxt->ixconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 05351cb1a4..cd814ff321 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3834,16 +3834,11 @@ Referenced by:
TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id)
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
- ^
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 1c3ef2b05a..2a0902ece2 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -153,14 +153,6 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
ERROR: cannot use "list" partition strategy with more than one column
--- unsupported constraint type for partitioned tables
-CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
-) PARTITION BY RANGE (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 3: EXCLUDE USING gist (a WITH &&)
- ^
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 3e5645c2ab..551ca44540 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -986,11 +986,32 @@ DETAIL: PRIMARY KEY constraint on table "idxpart" lacks column "a" which is par
-- OK if you use them in some other order
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: create table idxpart (a int, exclude (a with = )) partition ...
- ^
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
ERROR: unsupported PRIMARY KEY constraint with partition key definition
@@ -1047,12 +1068,42 @@ Indexes:
Number of partitions: 0
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
-ERROR: exclusion constraints are not supported on partitioned tables
-LINE 1: alter table idxpart add exclude (a with =);
- ^
+-- Exclusion constraints can be added if partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "b" which is part of the partition key.
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+ERROR: cannot match partition key to index on column "a" using non-equal operator "-|-".
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+ERROR: unique constraint on partitioned table must include all partitioning columns
+DETAIL: EXCLUDE constraint on table "idxpart" lacks column "a" which is part of the partition key.
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
create table idxpart (a int, b int, primary key (a, b)) partition by range (a, b);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 58ea20ac3d..ff8c498419 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2342,14 +2342,11 @@ ALTER TABLE ataddindex
\d ataddindex
DROP TABLE ataddindex;
--- unsupported constraint types for partitioned tables
+-- cannot drop column that is part of the partition key
CREATE TABLE partitioned (
a int,
b int
) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
-
--- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
ALTER TABLE partitioned DROP COLUMN b;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 93ccf77d4a..82ada47661 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -106,12 +106,6 @@ CREATE TABLE partitioned (
a2 int
) PARTITION BY LIST (a1, a2); -- fail
--- unsupported constraint type for partitioned tables
-CREATE TABLE partitioned (
- a int,
- EXCLUDE USING gist (a WITH &&)
-) PARTITION BY RANGE (a);
-
-- prevent using prohibited expressions in the key
CREATE FUNCTION retset (a int) RETURNS SETOF int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
CREATE TABLE partitioned (
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index d6e5a06d95..b69c41832c 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -483,8 +483,27 @@ create table idxpart (a int, b int primary key) partition by range (b, a);
create table idxpart (a int, b int, c text, primary key (a, b, c)) partition by range (b, c, a);
drop table idxpart;
--- not other types of index-based constraints
-create table idxpart (a int, exclude (a with = )) partition by range (a);
+-- OK to add an exclusion constraint if partitioning by its equal column
+create table idxpart (a int4range, exclude USING GIST (a with = )) partition by range (a);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a, b);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with =)) partition by range (a);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with = )) partition by range (a, b);
+-- Not OK with just -|-
+create table idxpart (a int4range, exclude USING GIST (a with -|- )) partition by range (a);
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range, exclude USING GIST (a with =, b with &&)) partition by range (a);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (b with =, c with &&)) partition by range (a);
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range, exclude USING GIST (a with =, b with =, c with &&)) partition by range (a, b);
+drop table idxpart;
-- no expressions in partition key for PK/UNIQUE
create table idxpart (a int primary key, b int) partition by range ((b + a));
@@ -506,9 +525,37 @@ alter table idxpart add unique (b, a); -- this works
\d idxpart
drop table idxpart;
--- Exclusion constraints cannot be added
-create table idxpart (a int, b int) partition by range (a);
-alter table idxpart add exclude (a with =);
+-- Exclusion constraints can be added if partitioning by their equal column
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- OK more than one equal column
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- OK with more than one equal column: constraint is a proper superset of partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with =);
+drop table idxpart;
+-- Not OK more than one equal column: partition keys are a proper superset of constraint
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =);
+drop table idxpart;
+-- Not OK with just -|-
+create table idxpart (a int4range, b int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with -|-);
+drop table idxpart;
+-- OK with equals and &&, and equals is the partition key
+create table idxpart (a int4range, b int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (a with =, b with &&);
+drop table idxpart;
+-- Not OK with equals and &&, and equals is not the partition key
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a);
+alter table idxpart add exclude USING GIST (b with =, c with &&);
+drop table idxpart;
+-- OK more than one equal column and a && column
+create table idxpart (a int4range, b int4range, c int4range) partition by range (a, b);
+alter table idxpart add exclude USING GIST (a with =, b with =, c with &&);
drop table idxpart;
-- When (sub)partitions are created, they also contain the constraint
--
2.32.0 (Apple Git-132)
On 11.07.23 07:52, Paul A Jungwirth wrote:
On Mon, Jul 10, 2023 at 8:06 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:On Mon, Jul 10, 2023 at 7:05 AM Peter Eisentraut <peter@eisentraut.org> wrote:
I'm not sure what value we would get from testing this with btree_gist,
but if we wanted to do that, then adding a new test file to the
btree_gist sql/ directory would seem reasonable to me.(I would make the test a little bit bigger than you had shown, like
insert a few values.)If you want to do that, please send another patch. Otherwise, I'm ok to
commit this one.I can get you a patch tonight or tomorrow. I think it's worth it since
btree_gist uses different strategy numbers than ordinary gist.Patch attached.
Looks good, committed.
I had some second thoughts about the use of get_attname(). It seems the
previous code used the dominant style of extracting the attribute name
from the open relation handle, so I kept it that way. That's also more
efficient than going via the syscache.