Re: create subscription with (origin = none, copy_data = on)

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Sergey Tatarintsev <s(dot)tatarintsev(at)postgrespro(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: create subscription with (origin = none, copy_data = on)
Date: 2025-01-23 07:05:46
Message-ID: CANhcyEULzgCkjpYTAqnm55J8qaAprnpSs790u7uYZPomUa8tYg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 22 Jan 2025 at 09:00, Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tuesday, January 21, 2025 1:31 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> Hi,
>
> >
> > On Mon, 20 Jan 2025 at 17:31, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Sat, Jan 18, 2025 at 10:31 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > >
> > > > Attached patch has the fix for this issue which includes the
> > > > partition tables also for the publication now and throws a warning
> > > > appropriately.
> > > >
> > >
> > > The corresponding query (see "To find which tables might potentially
> > > include non-local origins .." on [1]) on the create_subscription doc
> > > page.
> >
> > > BTW, the proposed fix is not backpatcheable as it changes the catalog
> > > which requires catversion bump. However, as this is a WARNING case, if
> > > we can't find a fix that can't be backpatched, we can fix it in
> > > HEAD-only.
> >
> > I could not find a way to fix the back version without changing the catalog
> > version.
> >
> > The attached v3 version has the changes for the same.
>
> Thanks for the patch.
>
> I agree that covering the partitioned table case when checking the non-local
> origin data on publisher is an improvement. But I think adding or extending the
> SQL functions may not be the appropriate way to fix because the new functions
> cannot be used in older PG version and is also not backpatchable.
>
> I am thinking it would be better to use the existing pg_partition_ancestors()
> and pg_partition_tree() to verify the same, which can be used in all supported
> PG versions and is also backpatchable.
>
> And here is another version which fixed the issue like that. I have not added
> tests for it, but I think it's doable to write the something like the testcases
> provided by Sergey. This patch does not fix the foreign tabel as that seems to
> be a separate issue which can be fixed independtly.
>
> Hi Sergey, if you have the time, could you please verify whether this patch
> resolves the partition issue you reported? I've confirmed that it passes the
> partitioned tests in the scripts, but I would appreciate your confirmation for
> the same.

Hi Hou-san,

I tested the patch, and it is working fine on HEAD.
I also tried to apply the patches to back branches PG17 and PG 16. But
the patch does not apply.

This 'origin' option was added in PG 16. So, this patch will not be
required for PG 15 and back branches.

Thanks and Regards,
Shlok Kyal

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2025-01-23 07:15:48 Re: generic plans and "initial" pruning
Previous Message Michael Paquier 2025-01-23 07:05:19 Re: pg_createsubscriber TAP test wrapping makes command options hard to read.