RE: row filtering for logical replication
От | houzj.fnst@fujitsu.com |
---|---|
Тема | RE: row filtering for logical replication |
Дата | |
Msg-id | OS0PR01MB5716BFE16C6F617CF2685B4494209@OS0PR01MB5716.jpnprd01.prod.outlook.com обсуждение исходный текст |
Ответ на | Re: row filtering for logical replication (Peter Smith <smithpb2250@gmail.com>) |
Ответы |
RE: row filtering for logical replication
("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
|
Список | pgsql-hackers |
On Tuesday, January 25, 2022 1:55 PM Peter Smith <smithpb2250@gmail.com> wrote: > > A couple more comments for the v69-0001 TAP tests. > Thanks for the comments! > > 1. src/test/subscription/t/027_row_filter.pl > > +# The subscription of the ALL TABLES IN SCHEMA publication means > there should be > +# no filtering on the tablesync COPY, so all expect all 5 will be present. > +$result = $node_subscriber->safe_psql('postgres', "SELECT count(x) > FROM schema_rf_x.tab_rf_x"); > +is($result, qq(5), 'check initial data copy from table tab_rf_x > should not be filtered'); > + > +# Similarly, normal filtering after the initial phase will also have > not effect. > +# Expected: > +# tab_rf_x : 5 initial rows + 2 new rows = 7 rows > +# tab_rf_partition : 1 initial row + 1 new row = 2 rows > +$node_publisher->safe_psql('postgres', "INSERT INTO > schema_rf_x.tab_rf_x (x) VALUES (-99), (99)"); > +$node_publisher->safe_psql('postgres', "INSERT INTO > schema_rf_x.tab_rf_partitioned (x) VALUES (5), (25)"); > +$node_publisher->wait_for_catchup($appname); > +$result = $node_subscriber->safe_psql('postgres', "SELECT count(x) > FROM schema_rf_x.tab_rf_x"); > +is($result, qq(7), 'check table tab_rf_x should not be filtered'); > +$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM > public.tab_rf_partition"); > +is($result, qq(20 > +25), 'check table tab_rf_partition should be filtered'); > > That comment ("Similarly, normal filtering after the initial phase will also have > not effect.") seems no good: > - it is too vague for the tab_rf_x tablesync > - it seems completely wrong for the tab_rf_partition table (because that filter is > working fine) > > I'm not sure exactly what the comment should say, but possibly something like > this (??): > > BEFORE: > Similarly, normal filtering after the initial phase will also have not effect. > AFTER: > Similarly, the table filter for tab_rf_x (after the initial phase) has no effect when > combined with the ALL TABLES IN SCHEMA. Meanwhile, the filter for the > tab_rf_partition does work because that partition belongs to a different > schema (and publish_via_partition_root = false). > Thanks, I think your change looks good. Changed. > > 2. src/test/subscription/t/027_row_filter.pl > > Here is a 2nd place with the same broken comment: > > +# The subscription of the FOR ALL TABLES publication means there should > +be no # filtering on the tablesync COPY, so all expect all 5 will be present. > +my $result = $node_subscriber->safe_psql('postgres', "SELECT count(x) > FROM tab_rf_x"); > +is($result, qq(5), 'check initial data copy from table tab_rf_x > should not be filtered'); > + > +# Similarly, normal filtering after the initial phase will also have > not effect. > +# Expected: 5 initial rows + 2 new rows = 7 rows > +$node_publisher->safe_psql('postgres', "INSERT INTO tab_rf_x (x) > VALUES (-99), (99)"); > +$node_publisher->wait_for_catchup($appname); > +$result = $node_subscriber->safe_psql('postgres', "SELECT count(x) > FROM tab_rf_x"); > +is($result, qq(7), 'check table tab_rf_x should not be filtered'); > > Here I also think the comment maybe should just say something like: > > BEFORE: > Similarly, normal filtering after the initial phase will also have not effect. > AFTER: > Similarly, the table filter for tab_rf_x (after the initial phase) has no effect when > combined with the ALL TABLES IN SCHEMA. Changed. Attach the V71 patch set which addressed the above comments. The patch also includes the changes: - Changed the function RelationBuildPublicationDesc's signature to be void and pass the PublicationDesc* from stack instead of palloc-ing it. [1] - Removed the Push/Pop ActiveSnapshot related code. IIRC, these functions are needed when we execute functions which will execute SQL(via SPI functions) to access the database. I think we don't need the ActiveSnapshot for now as we only support built-in immutable in the row filter which should only use the argument values passed to the function. - Adjusted some comments in pgoutput.c. [1] https://www.postgresql.org/message-id/CAHut%2BPsRTtXoYQiRqxwvyrcmkDMm-kR4GkvD9-nAqNrk4A3aCQ%40mail.gmail.com Best regards, Hou zj
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: "tanghy.fnst@fujitsu.com"Дата:
Сообщение: RE: Support tab completion for upper character inputs in psql