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-22 15:41:55
Message-ID: CANhcyEUZNvMmpaCxhqZHOK3KPAr-xCcwKY3AbcVRm7JaFVtmvw@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 have created a patch to add a test for the patch.

v5-0001 : same as v4-0001
v5-0002: adds the testcase

Thanks and Regards,
Shlok Kyal

Attachment Content-Type Size
v5-0001-Improve-logging-for-data-origin-discrepancies-in-.patch application/octet-stream 4.2 KB
v5-0002-Test-for-Improve-logging-for-data-origin-discrepa.patch application/octet-stream 3.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Kuzmenkov 2025-01-22 15:44:01 Quadratic planning time for ordered paths over partitioned tables
Previous Message Tom Lane 2025-01-22 15:34:16 Re: [PATCH] Fix a tiny typo in the documentation