23.01.2025 15:24, Zhijie Hou (Fujitsu) пишет:
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.
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.
Best Regards,
Hou zj
Hi!
That's right, separate schema was used to test "CREATE PUBLICATION FOR
TABLES IN SCHEMA".
My first patch with test cases contains 3 scenarios:
CREATE PUBLICATION pub_a FOR TABLE ts.t;
CREATE PUBLICATION pub_a_via_root FOR TABLE ts.t WITH
(publish_via_partition_root);
CREATE PUBLICATION pub_a_schema FOR TABLES IN SCHEMA ts WITH
(publish_via_partition_root);
(but not scenario where the table's ancestors are subscribed)
