On 2018/07/12 2:33, Alvaro Herrera wrote:
> On 2018-Jul-11, Amit Langote wrote:
>
>> 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.
>
> 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?
> But I worry that Tom was using domain constraints as just an example of
> a more general problem that we can get into.
>
>
>>> 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.
>
> 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.
> 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?
Thanks,
Amit