Re: Declarative partitioning - another take

On 2016/10/26 11:41, Amit Kapila wrote:
> On Wed, Oct 26, 2016 at 6:36 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> 1.
>>> @@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate,
>>> {
>>> ..
>>> + else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
>>> + ereport(ERROR,
>>> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>>> + errmsg("cannot copy from partitioned table \"%s\"",
>>> + RelationGetRelationName(rel)),
>>> + errhint("Try the COPY (SELECT ...) TO variant.")));
>>> ..
>>> }
>>>
>>> Why is this restriction? Won't it be useful to allow it for the cases
>>> when user wants to copy the data of all the partitions?
>>
>> Sure, CopyTo() can be be taught to scan leaf partitions when a partitioned
>> table is specified, but I thought this may be fine initially.
>>
>
> Okay, I don't want to add anything to your existing work unless it is
> important. However, I think there should be some agreement on which
> of the restrictions are okay for first version of patch. This can
> avoid such questions in future from other reviewers.

OK, so I assume you don't find this particular restriction problematic in
long term.

>>> 2.
>>> + if (!pg_strcasecmp(stmt->partspec->strategy, "list") &&
>>> + partnatts > 1)
>>> + ereport(ERROR,
>>> + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>>> + errmsg("cannot list partition using more than one column")));
>>>
>>> /cannot list/cannot use list
>>
>> Actually "list partition" works here as a verb, as in "to list partition".
>>
>
> I am not an expert of this matter, so probably some one having better
> grip can comment. Are we using something similar in any other error
> message?

In fact, I changed to the current text after Robert suggested the same [1].

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmoaPxXJ14eDVia514UiuQAXyZGqfbz8Qg3G4a8Rz2gKF7w%40mail.gmail.com