Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: 2025-09-15 17:47:15
Message-ID: CAPpHfdsh_jPZ6-2JtWoFeZravVQYY4cYCcnKXQW6KxEx+F5vxg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 15, 2025 at 11:03 AM Alexander Korotkov
<aekorotkov(at)gmail(dot)com> wrote:
>
> On Mon, Sep 1, 2025 at 2:04 PM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> wrote:
> > Hi!
> > Thank you for the notes and patch!
>
> Some additional notes from me.
>
> 1) src/backend/parser/parse_utilcmd.c includes are not alphabetically ordered here
> +#include "partitioning/partdesc.h"
> +#include "partitioning/partbounds.h"
>
> 2) There is unicode dash in the comment of ATExecMergePartitions() here. I suggest we should stick to ascii.
>
> + /*
> + * Check ownership of merged partitions — partitions with different
> + * owners cannot be merged. Also, collect the OIDs of these partitions
> + * during the check.
> + */
>
> 3) Regarding 17bcf4f545, I see btnamecmp() is collation-aware. Should we also specify COLLATE "C" every time we do "ORDER BY relname"?
>
> 4) This comment sounds misleading. Probably it should say "are contained".
>
> +/*
> + * check_parent_values_in_new_partitions
> + *
> + * (function for BY LIST partitioning)
> + *
> + * Checks that all values of split partition (with Oid partOid) contains in new
> + * partitions.
> + *
>
> 5) Given what latter items say, I think the 1. should say "The DEFAULT partition must be at most one."
>
> /*
> * check_partitions_for_split
> *
> * Checks new partitions for SPLIT PARTITIONS command:
> * 1. DEFAULT partition should be one.
>
> 6) Regarding the isolation tests. I see we are exercising INSERTs and intra-partition UPDATEs. Should we also try some cross-partition UPDATEs?

Additionally, I've made a numerous and small fixes for grammar to the
docs directly to the patchset.

------
Regards,
Alexander Korotkov
Supabase

Attachment Content-Type Size
v55-0001-Implement-ALTER-TABLE-.-MERGE-PARTITIONS-.-comma.patch application/octet-stream 167.1 KB
v55-0002-Implement-ALTER-TABLE-.-SPLIT-PARTITION-.-comman.patch application/octet-stream 228.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Burd 2025-09-15 18:00:18 Re: [PATCH] Add tests for Bitmapset
Previous Message Sophie Alpert 2025-09-15 17:41:27 Re: Fix missing EvalPlanQual recheck for TID scans