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