On Thu, 23 Jan 2025 at 17:54, Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Thursday, January 23, 2025 4:43 PM Shlok Kyal <shlok.kyal....@gmail.com> > wrote: > > > > On Thu, 23 Jan 2025 at 12:35, Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > > > > > On Wed, 22 Jan 2025 at 09:00, Zhijie Hou (Fujitsu) > > > <houzj.f...@fujitsu.com> wrote: > > > > > > > > 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. > > > > > I have created a patch which applies to both PG17 and PG 16. The > > v6-0002 is the test patch. It applies to all the branches (HEAD, PG17, > > PG16) correctly. > > Thanks for the patch. I think the testcases could be improved. > > It's not clear why a separate schema is created for tables. I assume it was > initially intended to test TABLES IN SCHEMA but was later modified. If the > separate schema is still necessary, could you please add comments to clarify > its purpose? > Besides, the new table name 'ts' seems a bit unconventional. It would be > better > to align with the naming style of existing test cases for consistency and > clarity. I think the schema is not required. I have removed it.
> Also, Sergey had suggested adding an more test to verify scenarios where the > table's ancestors are subscribed. It appears this hasn't been added yet. I > think it would be better to add it. I have added the test in the latest patch. Thanks and Regards, Shlok Kyal
v7-0001-Improve-logging-for-data-origin-discrepancies-in-.patch
Description: Binary data
v7-PG_17-PG_16-0001-Improve-logging-for-data-origin-discr.patch
Description: Binary data
v7-0002-Test-for-Improve-logging-for-data-origin-discrepa.patch
Description: Binary data
