Dear Shubham, > The attached patches contain the suggested changes.
Thanks for updating the patch. I reviewed only 0001 because they would be committed separately. Few comments: 01. ``` + For every non-template database on the source server, create one + subscription on the target server in the database with the same name. ``` It is quite confusing for me; We do not have to describe users that this command checks databases of the source server. How about something like: "Create one subscription per all non-template databases on the target server." 02. ``` +# run pg_createsubscriber with '--database' and '--all' without '--dry-run' +# and verify the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--database' => $db1, + '--all', + ], + qr/--database cannot be used with --all/, + 'fail if --database is used with --all'); + +# run pg_createsubscriber with '--publication' and '--all' and verify +# the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--all', + '--publication' => 'pub1', + ], + qr/--publication cannot be used with --all/, + 'fail if --publication is used with --all'); ``` You seemed to move most of validation checks to 0002. Can you tell me a reason why they are remained? 03. ``` +# Verify that the required logical replication objects are created. The +# expected count 3 refers to postgres, $db1 and $db2 databases. ``` Hmm, but since we did a dry-run, any objects are not created, right? Best regards, Hayato Kuroda FUJITSU LIMITED
