| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | Tender Wang <tndrwang(at)gmail(dot)com> |
| Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
| Date: | 2025-12-21 07:42:36 |
| Message-ID: | CALdSSPgT4-epuBeVy3pF61gT1WbwoDXh9uYwqtR7PktFQVQzig@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, 21 Dec 2025, 12:14 Tender Wang, <tndrwang(at)gmail(dot)com> wrote:
>
>
> Alexander Korotkov <aekorotkov(at)gmail(dot)com> 于2025年12月20日周六 19:08写道:
>
>> Hi Tender,
>>
>> On Sat, Dec 20, 2025 at 5:18 AM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
>> > I found this feature merged; thanks for this work.
>> > I tested it and found that one place in the error errcode may need to
>> be changed.
>> > In checkPartition():
>> > ...
>> > if (get_partition_parent(partRelOid, false) != RelationGetRelid(rel))
>> > ereport(ERROR,
>> > errcode(ERRCODE_UNDEFINED_TABLE),
>> > errmsg("relation \"%s\" is not a partition of relation \"%s\"",
>> > ...
>> >
>> > ERRCODE_UNDEFINED_TABLE usually means "table does not exist."
>> > When entering here, the table should exist, otherwise table_open()
>> already reports an error.
>> > I found another two errcode in checkPartition() use
>> ERRCODE_WRONG_OBJECT_TYPE,
>> > In the attached patch, I replace ERRCODE_UNDEFINED_TABLE with
>> ERRCODE_WRONG_OBJECT_TYPE.
>>
>> I agree with you that ERRCODE_UNDEFINED_TABLE is certainly wrong error
>> code because the table actually exists. ERRCODE_WRONG_OBJECT_TYPE is
>> better. For example, we throw it when trying to attach a partition to
>> non-partitioned table. So, the parent table type is wrong. However,
>> are objects in the situation under consideration really have wrong
>> type? The problem is that one table is not partition of another.
>> However, it's possibly that they could be attached without changing of
>> their types. So, I think about
>> ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE. What do you think?
>>
>
> It's ok for me. Please check the v2 patch.
>
> --
> Thanks,
> Tender Wang
>
On Sun, 21 Dec 2025, 12:14 Tender Wang, <tndrwang(at)gmail(dot)com> wrote:
>
>
> Alexander Korotkov <aekorotkov(at)gmail(dot)com> 于2025年12月20日周六 19:08写道:
>
>> Hi Tender,
>>
>> On Sat, Dec 20, 2025 at 5:18 AM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
>> > I found this feature merged; thanks for this work.
>> > I tested it and found that one place in the error errcode may need to
>> be changed.
>> > In checkPartition():
>> > ...
>> > if (get_partition_parent(partRelOid, false) != RelationGetRelid(rel))
>> > ereport(ERROR,
>> > errcode(ERRCODE_UNDEFINED_TABLE),
>> > errmsg("relation \"%s\" is not a partition of relation \"%s\"",
>> > ...
>> >
>> > ERRCODE_UNDEFINED_TABLE usually means "table does not exist."
>> > When entering here, the table should exist, otherwise table_open()
>> already reports an error.
>> > I found another two errcode in checkPartition() use
>> ERRCODE_WRONG_OBJECT_TYPE,
>> > In the attached patch, I replace ERRCODE_UNDEFINED_TABLE with
>> ERRCODE_WRONG_OBJECT_TYPE.
>>
>> I agree with you that ERRCODE_UNDEFINED_TABLE is certainly wrong error
>> code because the table actually exists. ERRCODE_WRONG_OBJECT_TYPE is
>> better. For example, we throw it when trying to attach a partition to
>> non-partitioned table. So, the parent table type is wrong. However,
>> are objects in the situation under consideration really have wrong
>> type? The problem is that one table is not partition of another.
>> However, it's possibly that they could be attached without changing of
>> their types. So, I think about
>> ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE. What do you think?
>>
>
> It's ok for me. Please check the v2 patch.
>
> --
> Thanks,
> Tender Wang
>
Hi! Your v2 looks fine
The only question for me is, should we add any regression test to exercise
this code, or it is not worth the troubles?
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tristan Partin | 2025-12-21 08:19:04 | Re: AIX support |
| Previous Message | Tender Wang | 2025-12-21 07:14:01 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |