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

17.01.2025 23:00, vignesh C пишет:
> On Fri, 17 Jan 2025 at 14:00, Sergey Tatarintsev
> <s(dot)tatarintsev(at)postgrespro(dot)ru> wrote:
>> Hi, hackers!
>>
>> I am looking at subscription creation command:
>>
>> CREATE SUBSCRIPTION sub CONNECTION '...' PUBLICATION pub WITH (origin =
>> none, copy_data = on);
>>
>> For now we log a warning if the publisher has subscribed to the same
>> table from some other publisher.
>> However, in case of publication with publish_via_partition_root option,
>> we will not raise such warinigs
>> because SQL command in check_publications_origin() checks only directly
>> published tables.
> Yes, I agree that we are checking only the directly published tables
> which is why there is no warning in this case. I'm working on a fix to
> change the check_publications_origin to check accordingly.
seems promising. I would like to see this patch
>> For example:
>>
>> CREATE TABLE t(id int) PARTITION BY RANGE(id);
>> CREATE TABLE part1 PARTITION OF t FOR VALUES FROM (0) TO (5);
>> CREATE TABLE part2 PARTITION OF t FOR VALUES FROM (5) TO (10);
>> -- subscribe to part2
>> CREATE SUBSCRIPTION sub_part2 CONNECTION '...' PUBLICATION pub_part2;
>> CREATE PUBLICATION pub_t FOR TABLE t;
>> CREATE PUBLICATION pub_t_via_root FOR TABLE t WITH
>> (publish_via_partition_root);
>>
>> and now this command will raise a warning:
>> CREATE SUBSCRIPTION sub1 CONNECTION '...' PUBLICATION pub_t WITH (origin
>> = none, copy_data = on);
>>
>> but not this:
>> CREATE SUBSCRIPTION sub1 CONNECTION '...' PUBLICATION pub_t_via_root
>> WITH (origin = none, copy_data = on);
>>
>> We also do not take into account cases of foreign partitions:
>> CREATE TABLE t(id int) PARTITION BY RANGE(id);
>> CREATE TABLE part1 PARTITION OF t FOR VALUES FROM (0) TO (5);
>> CREATE FOREIGN TABLE part2 PARTITION OF t FOR VALUES FROM (5) TO (10)
>> SERVER fdw_server;
>> CREATE PUBLICATION pub_t FOR TABLE t;
>>
>> Maybe we should raise WARNING (or even ERROR) in such cases?
> Currently we do not support replication of foreign tables. This is
> mentioned in logical replication restriction sections at [1].
Yes of course, but we must raise an ERROR in such cases
>> I would also note that the (origin = none) will work as expected, but in
>> case of (origin = any)
>> it will lead to inappropriate behavior - we will perform an initial sync
>> of "t", but we unable to
>> replicate further updates for "part2".
> I noticed the same behavior with both origins as none and any. i.e
> initial sync is ok and then replication of foreign table part2 will
> not work which is because of the above restriction that I mentioned.
> Just to be sure that I'm not checking a different scenario, could you
> share the test for this case.

That's right, but i think we must tell user about inappropriate usage.

check_publication_add_relation() checks only publication creation for
foreign tables directly, but not partitioned tables structure.

My test case just shows that we can try to replicate partitioned table
with foreign partitions, but I think we should disallow such cases.

I would also like to show an interesting subscription creation scenario
that I found:

1. subscriber: calls check_publications_origin()

2. publisher: executes the create/attach foreign partition command

3. the subscriber is sure he checked the origin and performing COPY t TO
STDOUT

i.e. between the check and the start of copying the publication has changed

the problem is that check_publications_origin() and COPY t TO STDOUT are
performed in different transactions

>
> [1] - https://www.postgresql.org/docs/devel/logical-replication-restrictions.html
>
> Regards,
> Vignesh