Re: MERGE/SPLIT PARTITIONS issues/questions

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: MERGE/SPLIT PARTITIONS issues/questions
Date: 2026-08-11 21:36:34
Message-ID: CAPpHfdvR-0=4ZFreeQpm2-JdyTw7Bge+vtYwKY1UKcvy+MW55w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Jian,
Zsolt,

Thank you both for your valuable catches. Attached is v3 addressing
the points raised.

On Fri, Aug 7, 2026 at 6:42 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> On Fri, Aug 7, 2026 at 6:59 AM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> > > Copying the value of generated column "as is" can produce data that differs from
> > > what the generated expression would compute if any merged partition's generation
> > > expression differs from the partitioned table's.
> >
> > I think this would be probably fine, as we can get the same effect by
> > replacing a function used by the expression, a preexisting condition
> > for many existing cases. But I do agree that requiring the same
> > expression is a better approach.
> >
> > Also, not directly related to this patch, but now that I looked into
> > this, I can still use tableoids for check constraints with a text
> > cast:
> >
> > CREATE TABLE t (i int) PARTITION BY RANGE (i);
> > CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
> > CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
> > ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> 'tp_0_2');
> > INSERT INTO t VALUES (0),(1);
> > ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; --
> > SUCCESS, but should ERROR instead?
>
> Interesting!
>
> Before we call MergePartitionsMoveRows, we did RestrictSearchPath(),
> which will set GUC search_path
> to "pg_catalog, pg_temp" temporally, and text_regclass will consider
> search_path when resolve object name.
>
> On the other hand, if we unconditionally validate all the partitioned
> table's inherited CHECK constraints, it may fail
> and the resulting message isn't helpful.
> The error message below shows what happens when evaluating all CHECK
> constraints during MERGE PARTITIONS.
>
> DROP TABLE IF EXISTS t;
> CREATE TABLE t (i int, b text default 't') PARTITION BY RANGE (i);
> CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
> CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
> ALTER TABLE t ADD CONSTRAINT cc CHECK (b::regclass::text in ('t',
> 'tp_0_1', 'tp_0_2', 'tp_1_2'));
> INSERT INTO t VALUES (0);
> INSERT INTO t VALUES (0, 'tp_0_1'), (1, 'tp_1_2'), (1, 'public.tp_1_2');
> ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
> ERROR: relation "t" does not exist

I see that virtual generated columns also can lead to the problems.
The revised 0003 rejects the dependency regardless of the generated
column kind, in a new checkPartitionSystemColumnRefs() called before
the new partition is created.

I confirm that CHECK constraints depending on a system column are also
problematic. The revised 0003 rejects CHECK constraints referencing a
system column as well, for the same reason as generated columns.
Since nothing needs re-verification anymore, the machinery that did it
is removed: buildPartitionCheckExprStates(),
checkPartitionRowConstraints(), the AlteredTableInfo.constraints
population, and the work queue entry and arguments that existed only
to carry them.

0002 also refuses to create the new partition in a schema whose FOR
TABLES IN SCHEMA publications differ from those of the source
partitions, since that would silently add the relocated rows to, or
remove them from, such a publication. The check triggers only when a
schema publication is actually involved, so a cross-schema MERGE/SPLIT
is still allowed otherwise; publications FOR ALL TABLES, or covering
the partitioned table itself, keep covering the new partitions and are
unaffected.

Agreed that the previous wording recommended something that runs into
your data-loss scenario. The paragraph now just states the facts: if
changes are published for the partitioned table itself, subscribers
are unaffected and may keep their own partition layout; otherwise the
new partition is not part of the subscription until it is refreshed,
and changes made in the meantime are not applied – so refreshing
without copying its data would silently lose them.

On the rewrite event trigger: MERGE/SPLIT doesn't fire table_rewrite,
and I don't think it should. table_rewrite reports a single table
that keeps its identity while getting a new relfilenode. MERGE turns
N partitions into one new relation and SPLIT one into N, dropping the
originals, so there is no single "table being rewritten" to report.
The commands are still visible to ddl_command_start/ddl_command_end as
an ALTER TABLE.

------
Regards,
Alexander Korotkov
Supabase

Attachment Content-Type Size
v3-0001-Don-t-logically-decode-MERGE-SPLIT-PARTITION-row-.patch application/x-patch 11.2 KB
v3-0002-Peserve-replica-identity-and-publications-in-MERG.patch application/x-patch 24.5 KB
v3-0003-Don-t-recalculate-generated-columns-during-MERGE-.patch application/x-patch 52.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2026-08-11 21:43:19 Re: problems with toast.* reloptions
Previous Message Andres Freund 2026-08-11 20:57:35 Re: [PATCH] bufmgr: tighten LWLock:BufferMapping on InvalidateBuffer

Browse pgsql-bugs by date

  From Date Subject
Previous Message Zsolt Parragi 2026-08-11 19:32:12 COPY TO regression with psql -c