On 2018/07/26 1:41, Alvaro Herrera wrote:
> On 2018-Jul-12, Amit Langote wrote:
>> On 2018/07/12 2:33, Alvaro Herrera wrote:
>>> Yeah, any domain constraints added before won't be a problem. Another
>>> angle on this problem is to verify partition bounds against the domain
>>> constraint being added; if they all pass, there's no reason to reject
>>> the constraint.
>>
>> That's an idea. It's not clear to me how easy it is to get hold of all
>> the partition bounds that contain domain values. How do we get from the
>> domain on which a constraint is being added to the relpartbound which
>> would contain the partition bound value of the domain?
>
> Well, we already get all table columns using a domain when the domain
> gets a new constraint; I suppose it's just a matter of verifying for
> each column whether it's part of the partition key, and then drill down
> from there. (I'm not really familiar with that part of the catalog.)
Possibly doable, but maybe leave it for another day.
>>> Actually, here's another problem: why are we letting a column on a
>>> domain become partition key, if you'll never be able to create a
>>> partition? It seems that for pg11 the right reaction is to check
>>> the partition key and reject this case.
>>
>> Yeah, that seems much safer, and given how things stand today, no users
>> would complain if we do this.
>
> Agreed.
>
>>> I'm not sure of this implementation -- I couldn't find any case where we
>>> reject the deletion on the function called from doDeletion (and we don't
>>> have a preliminary phase on which to check for this, either). Maybe we
>>> need a pg_depend entry to prevent this, though it seems a little silly.
>>> Maybe we should add a preliminary verification phase in
>>> deleteObjectsInList(), where we invoke object-type-specific checks.
>>
>> Doing pre-check based fix had crossed my mind, but I didn't try hard to
>> pursue it. If we decide to just prevent domain partition keys, we don't
>> need to try too hard now to come up with a nice implementation for this,
>> right?
>
> Absolutely.
OK, attached is a patch for that.
Thanks,
Amit
From 042aa582f717ceb695a1ab60517c2e9f7d04704b Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 26 Jul 2018 11:07:51 +0900
Subject: [PATCH v1] Disallow domain type partition key
---
src/backend/commands/tablecmds.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index eb2d33dd86..871c831cd2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13693,6 +13693,12 @@ ComputePartitionAttrs(Relation rel, List *partParams,
AttrNumber *partattrs,
atttype = attform->atttypid;
attcollation = attform->attcollation;
ReleaseSysCache(atttuple);
+
+ /* Prevent using domains as a partition key. */
+ if (get_typtype(atttype) == TYPTYPE_DOMAIN)
+ ereport(ERROR,
+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot use
domain type column as partition key")));
}
else
{
@@ -13703,6 +13709,12 @@ ComputePartitionAttrs(Relation rel, List *partParams,
AttrNumber *partattrs,
atttype = exprType(expr);
attcollation = exprCollation(expr);
+ /* Prevent using domains as a partition key. */
+ if (get_typtype(atttype) == TYPTYPE_DOMAIN)
+ ereport(ERROR,
+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot use
domain type expression as partition key")));
+
/*
* Strip any top-level COLLATE clause. This ensures
that we treat
* "x COLLATE y" and "(x COLLATE y)" alike.
--
2.11.0
