Re: [pgAdmin][RM5912]: Added support for Logical Replication.

Hi Pradip

Following are the GUI related review comments:

*Publication:*

1. The truncate option is not visible for PG 10 but the SQL query
showing it.
2. "All table?" should be "All tables?" and "Table" should be renamed to
"Tables".
3. "-- Publication : test;
-- DROP PUBLICATION test;" Follow the syntax(Spacing, no space between
colon) as we did for other nodes. Check the SQL tab for any other node.
4. Select any publication and open the properties dialog, Save button is
enabled by default which should not.
5. Create publication with (insert, update, delete, and truncate). Open
the properties dialog and Set the value of Insert, Update and Delete to
"No" check the SQL tab no ALTER query is there. ALTER query should be there.
6. 'Only Table' is not visible in the Properties panel.
7. On the collection node 'Publications' we should add Tables in the
properties panel.
8. Create a publication using one table and set 'Only Table' to No.
Select a newly created publication and set 'Only Table' to Yes, no SQL
query is generated.
9. Adding a table in the publication should generate 'ALTER PUBLICATION
.. ADD TABLE ..' syntax instead of 'ALTER PUBLICATION .. SET TABLE..'
10. Create a publication with more than one table. Remove one table from
publication it is creating two ALTER statements one for DROP Table and
another one is to SET TABLE which I think not required.

*Subscription:*

1. Reduce the width of the subscription dialog.
2. *With *tab, alignment is not proper. Switch control should be aligned
with others.
3. Help messages required for every control in *With* tab.
4. Open Create subscription dialog, specify the name, on the connection
tab click the button to get the publication without specifying any details.
One popup comes with msg 'host'. I would suggest to *disabled* that
button until all the required fields will be populated or entered by the
user.
5. Publication list from the same database server should be listed by
default.
6. Specify all the required connection details on the connection tab and
click on the 'get publication' button. It fetches the publication but no
message for the user that action is successfully completed. For the user,
it seems nothing is happening and the user will click on that button
continuously. *Suggestion: *Show some message 'Publications fetched
successfully.' or something similar.
7. "-- Subscription : my_sub; -- DROP SUBSCRIPTION my_sub;" Follow the
syntax(Spacing, no space between colon) as we did for other nodes. Check
the SQL tab for any other node.
8. Remove the unnecessary spaces from the following SQL:
- CONNECTION ' host=127.0.0.1 port=5436 user=postgres
dbname=postgres '
- with (enabled = False, slot_name = my_sub1, synchronous_commit =
'off');
- (connect = True, enabled = True, copy_data = True, create_slot =
True, synchronous_commit = 'off'); Use small true/false instead of
True/False.
9. Create a subscription. Open the properties dialog and click on the
"get publication" button. It throws an error message with the string
'password', please provide a valid error message. Check for all such error
messages.
10. Consider the above(Point 9) scenario and provide a password and then
click on the "get publication" button it throws an error "*could not
translate host name "127.0.0.1 " to address: nodename nor servname
provided, or not known*". There is a space after the IP address, even if
we remove that space or provide a different IP address the same error
occurs.
11. The "Refresh publication" option should be disabled if the
subscription is disabled, as it should not work with the disabled
subscription.
12. No SQL query created when we remove the Slot name for the existing
subscription. How to handle:
- If the subscription is enabled then add validation "Slot name can
not be empty".
- If the subscription is disabled then on removing the slot name
should create "ALTER SUBSCRIPTION <name> SET (slot_name = NONE);"
13. Provide a space after comma in the current publication list.

Please explain how the service file parameter will work with the "CREATE
SUBSCRIPTION .." syntax as per documentation we have to provide the
connection info string. Not able to test it.

Created #6153 <https://redmine.postgresql.org/issues/6153> to add
publication and subscription in Schema Diff.

On Thu, Jan 14, 2021 at 6:58 PM Pradip Parkale <
pradip(dot)parkale(at)enterprisedb(dot)com> wrote:

> Hi Akshay,
> Please find the latest patch. I missed some files in my previous patch.
>
> On Thu, Jan 14, 2021 at 5:49 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Pradip
>>
>> The patch is applied, but not working. Please check and send the patch
>> again.
>>
>> On Wed, Jan 13, 2021 at 9:08 AM Pradip Parkale <
>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Akshay,
>>>
>>> Please ignore my previous patch. Please find my updated patch.
>>>
>>>
>>> On Mon, Jan 11, 2021 at 5:07 PM Pradip Parkale <
>>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the attached patch for logical replication support.
>>>>
>>>> --
>>>> Thanks & Regards,
>>>> Pradip Parkale
>>>> Software Engineer | EnterpriseDB Corporation
>>>>
>>>
>>>
>>> --
>>> Thanks & Regards,
>>> Pradip Parkale
>>> Software Engineer | EnterpriseDB Corporation
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> Thanks & Regards,
> Pradip Parkale
> Software Engineer | EnterpriseDB Corporation
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*