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 ~{:-) p...@illuminatedcomputing.com
From 928a31433a4b8cad25f74017ca96b843aa30e02d Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" <p...@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