| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Skipping schema changes in publication |
| Date: | 2026-02-02 06:12:21 |
| Message-ID: | CAJpy0uCDJg9BbnQVRNskwCA9dfH-GbU1wdQxG1Bdiry-xJDrHg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Feb 2, 2026 at 11:27 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Mon, Feb 2, 2026 at 4:32 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Fri, Jan 30, 2026 at 1:46 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > Hi Vignesh.
> > >
> > > Some review comments for v38-0001.
> > >
> > > ======
> > >
> > > 1. General. Patch structure
> > >
> > > This patch set structure seems muddled to me; I think patch 0001
> > > requires proper handling for EXCEPT (partitions), otherwise, it cannot
> > > be independently committed.
> > >
> > > It's my understanding that the goal is to try approach #1, but if that
> > > proves too difficult, then the fallback would be approach #3. Yet
> > > AFAICT this patch 0001 is neither -- I have no idea anymore what patch
> > > 0001 does for partitions; IIUC it looks like just old partition logic
> > > from a few versions back (???).
> > >
> > > Personally, I felt it would be better to combine 0001 + 0002 (approach
> > > #3), then 0002 would be a patch that *replaces* approach #3 logic with
> > > approach #1 logic. That way, 0001 is self-contained, and 0002 is an
> > > evolution of the feature.
> > >
> >
> > I agree that the current patch structure is hard to follow and doesn’t
> > add much value, since patch001 isn’t aligned with a specific approach.
> > How about this patch structure:
> >
> > patch001: Implement EXCEPT syntax and CREATE PUBLICATION changes. This
> > includes all changes required to correctly populate
> > pg_publication_tables and pg_publication_rel.
> >
> > Expectation for patch001:
> > When this patch is applied on its own, CREATE PUBLICATION command with
> > the EXCEPT clause should work, and the appropriate entries should
> > appear in pg_publication_tables/pg_publication_rel with the except
> > flag set correctly. No publishing expected in this patch.
> >
> > patch002: Enable publishing and subscription support.
> > All changes required to make subscriptions work should go here, including:
> > --table sync and other subscriber-side changes
> > --pgoutput logic to determine which tables are published
> >
> > Expectation for patch002:
> > Changes should be published and replicated correctly to the subscriber.
> >
> > patch003: pg_dump, tests, documentation, etc.
> >
> > All of the above changes/patches are intended to support Approach001.
> >
> > Approach003 has a limitation in that it allows 'only' ROOT table to be
> > specified in the EXCEPT clause. If tab_root is listed in EXCEPT and a
> > user later attempts to attach it as a partition of another table
> > using:
> > 'ALTER TABLE root ATTACH PARTITION tab_root', we would need to block
> > the ATTACH PARTITION command. Given this, we should first try to
> > implement Approach001 and evaluate its feasibility. If it turns out to
> > be impractical,
> > we can fall back to Approach003 or consider other alternatives. For
> > now, the Approach003 patch can remain on hold.
> >
>
> Hi Shveta,
>
> Your split proposal sounds like an improvement.
>
> Personally, I feel each patch should be *self-contained*, not only
> because then it could safely be pushed independently, but also because
> I normally work 1 patch at a time
> - apply 0001 / review 0001
> - apply 0002 / review 0002
> - etc
>
> Tests:
> IMO if patch 0001 does something then it should also include the
> necessary tests for that "something". Then patch 0002 adds something
> more so the 0002 tests should evolve to add a few more tests to what
> were there for 0001 etc.
> IOW, I don't really want to be forced to apply everything in order to
> run the tests for 0001.
+1. we should be adding tests in the concerned patch
>
> Docs:
> Again personally I prefer everything *self-contained* for 0001 would
> have docs for 001 etc, so I can focus on just one improvement at a
> time, but I'm OK with docs being kept separated if others prefer it
> that way.
I think splitting the doccould be challenging. We can start by keeping
it in patch003, and once we have better clarity, we can consider
splitting the doc if needed.
OTOH, each patch’s commit message should clearly state its purpose, so
the EXCEPT concept can be understood without referring to the doc
coming in later patch.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-02-02 06:25:01 | walsender: Assert MyReplicationSlot is set before use |
| Previous Message | Peter Smith | 2026-02-02 05:57:02 | Re: Skipping schema changes in publication |