On 2018/07/11 13:12, Alvaro Herrera wrote:
> On 2018-Jul-11, Amit Langote wrote:
>
>> What's the solution here then? Prevent domains as partition key?
>
> Maybe if a domain is used in a partition key somewhere, prevent
> constraints from being added?
Maybe, but I guess you mean only prevent adding such a constraint
after-the-fact.
If a domain has a constraint before creating any partitions based on the
domain, then partition creation command would check that the partition
bound values satisfy those constraints.
> Another thing worth considering: are you prevented from dropping a
> domain that's used in a partition key? If not, you get an ugly message
> when you later try to drop the table.
Yeah, I was about to write a message about that. I think we need to teach
RemoveAttributeById, which dependency.c calls when dropping the
referencing domain with cascade option, to abort if the attribute passed
to it belongs to the partition key of the input relation.
I tried that in the attached, but not sure about the order of messages
that appear in the output of DROP DOMAIN .. CASCADE. It contains a NOTICE
message followed by an ERROR message.
Thanks,
Amit
From aca92efe08a45c7645720bf7c47ee5ca221f0a80 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 11 Jul 2018 10:26:55 +0900
Subject: [PATCH v1] Prevent RemoveAttributeById from dropping partition key
dependency.c is currently be able to drop whatever is referencing
the partition key column, because RemoveAttributeById lacks guards
checks to see if it's actually dropping a partition key.
---
src/backend/catalog/heap.c | 29 +++++++++++++++++++++++++++++
src/test/regress/expected/create_table.out | 10 ++++++++++
src/test/regress/sql/create_table.sql | 8 ++++++++
3 files changed, 47 insertions(+)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index d223ba8537..b69b9d9436 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1585,6 +1585,35 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
else
{
/* Dropping user attributes is lots harder */
+ bool is_expr;
+
+ /*
+ * Prevent dropping partition key attribute, making whatever
that's
+ * trying to do this fail.
+ */
+ if (has_partition_attrs(rel,
+ bms_make_singleton(attnum -
FirstLowInvalidHeapAttributeNumber),
+ &is_expr))
+ {
+ if (!is_expr)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot drop column
\"%s\" of table \"%s\"",
+
NameStr(attStruct->attname),
+
RelationGetRelationName(rel)),
+ errdetail("Column \"%s\" is
named in the partition key of \"%s\"",
+
NameStr(attStruct->attname),
+
RelationGetRelationName(rel))));
+ else
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot drop column
\"%s\" of table \"%s\"",
+
NameStr(attStruct->attname),
+
RelationGetRelationName(rel)),
+ errdetail("Column \"%s\" is
referenced in the partition key expression of \"%s\"",
+
NameStr(attStruct->attname),
+
RelationGetRelationName(rel))));
+ }
/* Mark the attribute as dropped */
attStruct->attisdropped = true;
diff --git a/src/test/regress/expected/create_table.out
b/src/test/regress/expected/create_table.out
index 8fdbca1345..ba32d781d1 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -909,3 +909,13 @@ ERROR: cannot create a temporary relation as partition of
permanent relation "p
create temp table temp_part partition of temp_parted default; -- ok
drop table perm_parted cascade;
drop table temp_parted cascade;
+-- test domain partition key behavior
+create domain ct_overint as int;
+create table ct_domainpartkey (a ct_overint) partition by range (a);
+-- errors because partition key column cannot be dropped
+drop domain ct_overint cascade;
+NOTICE: drop cascades to column a of table ct_domainpartkey
+ERROR: cannot drop column "a" of table "ct_domainpartkey"
+DETAIL: Column "a" is named in the partition key of "ct_domainpartkey"
+drop table ct_domainpartkey;
+drop domain ct_overint;
diff --git a/src/test/regress/sql/create_table.sql
b/src/test/regress/sql/create_table.sql
index 78944950fe..5c6c37cdaa 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -735,3 +735,11 @@ create temp table temp_part partition of perm_parted
default; -- error
create temp table temp_part partition of temp_parted default; -- ok
drop table perm_parted cascade;
drop table temp_parted cascade;
+
+-- test domain partition key behavior
+create domain ct_overint as int;
+create table ct_domainpartkey (a ct_overint) partition by range (a);
+-- errors because partition key column cannot be dropped
+drop domain ct_overint cascade;
+drop table ct_domainpartkey;
+drop domain ct_overint;
--
2.11.0
