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

23.01.2025 15:24, Zhijie Hou (Fujitsu) пишет:
> On Thursday, January 23, 2025 4:43 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>> On Thu, 23 Jan 2025 at 12:35, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>>> On Wed, 22 Jan 2025 at 09:00, Zhijie Hou (Fujitsu)
>>> <houzj(dot)fnst(at)fujitsu(dot)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)