Thanks, the patch applied with some fixes. On Wed, Jan 27, 2021 at 11:09 AM Pradip Parkale < pradip.park...@enterprisedb.com> wrote:
> Hi Akshay, > Please find the updated patch. I have fixed almost all issues, some are > not possible. > > On Sat, Jan 16, 2021 at 4:29 PM Akshay Joshi < > akshay.jo...@enterprisedb.com> wrote: > >> 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. >> >> Fixed, > >> >> 1. "All table?" should be "All tables?" and "Table" should be renamed >> to "Tables". >> >> Fixed. > >> >> 1. "-- 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. >> >> Fixed. > >> >> 1. Select any publication and open the properties dialog, Save button >> is enabled by default which should not. >> >> Fixed. > >> >> 1. 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. >> >> Fixed. > >> >> 1. 'Only Table' is not visible in the Properties panel. >> >> There is no entry of 'Only Table?' in the pg_publication table, so it is > not possible to get the value of this. > >> >> 1. On the collection node 'Publications' we should add Tables in the >> properties panel. >> >> Fixed. > >> >> 1. 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. >> >> I have disabled this control. This control will be enabled only when the > user has changed the table data. > >> >> 1. Adding a table in the publication should generate 'ALTER >> PUBLICATION .. ADD TABLE ..' syntax instead of 'ALTER PUBLICATION .. SET >> TABLE..' >> >> Fixed. > >> >> 1. 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. >> >> Fixed. > >> *Subscription:* >> >> 1. Reduce the width of the subscription dialog. >> >> Fixed. I reduced the width to '501px' > >> >> 1. *With *tab, alignment is not proper. Switch control should be >> aligned with others. >> >> Fixed. > >> >> 1. Help messages required for every control in *With* tab. >> >> Fixed. > >> >> 1. 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. >> >> Fixed.' Publication' control will only be enabled when the user > entered the required connection details. > >> >> 1. Publication list from the same database server should be listed by >> default. >> >> This won't be a good idea to display the default publication because if > the user entered the connection details of some other server and select the > default publication without clicking on the 'get_publication' button then > it will be a wrong mapping between connection details and publication. > Logical replication won't happen then. > >> >> 1. 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. >> >> Fixed. Added information message at the right bottom of the page. > >> >> 1. "-- 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. >> >> Done. > >> >> 1. 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. >> >> Fixed. > >> >> 1. 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. >> >> Fixed. > >> >> 1. 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. >> >> Fixed. > >> >> 1. The "Refresh publication" option should be disabled if the >> subscription is disabled, as it should not work with the disabled >> subscription. >> >> Fixed. > >> >> 1. 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);" >> >> Fixed. On removing the slot name I'm adding setting the slot name as > 'None'. > >> >> 1. Provide a space after comma in the current publication list. >> >> Fixed. > >> 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. >> > Removed the service parameter as it is not useful in the connection > string. > > I have also fixed the review comments given in the demo. Subscription > doesn't maintain any 'dependency' and 'dependent'. So I have not added that. > >> >> 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.park...@enterprisedb.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.jo...@enterprisedb.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.park...@enterprisedb.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.park...@enterprisedb.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* >> > > > -- > 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*
