Re: [PATCH] Tables node (pgAdmin4)

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Tables node (pgAdmin4)
Date: 2016-05-13 11:54:10
Message-ID: CAFiP3vxO9tej3WXYeLjZk6MgQYR0C-hp9UUWmog1E7iDvvqFmQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

PFA attached patches for table and it's child nodes with python 2.7
compatibility.

--
*Harshal Dhumal*
*Software Engineer *

EenterpriseDB <http://www.enterprisedb.com>

On Thu, May 12, 2016 at 6:55 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi,
>
> I have started reviewing the patch but the basic functionalities are
> breaking on python 2.7.
> Please test the patch on Python 2.7, fix the issues and resend the patch.
>
> Thanks,
> Khushboo
>
> On Thu, May 12, 2016 at 6:03 PM, Harshal Dhumal <
> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>>
>> --
>> *Harshal Dhumal*
>> *Software Engineer *
>>
>>
>>
>> EenterpriseDB <http://www.enterprisedb.com>
>>
>> On Tue, May 10, 2016 at 6:37 PM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Harshal,
>>>
>>> Pending issues to be fixed which I tried but not able to fix in
>>> Constraints node,
>>>
>> *1)* Adding Primary key in create table mode causes "too much recursion"
>>> error & Column collection validation error.
>>>
>> Fixed.
>>
>>
>>> *2)* MultiSelect2 rendering issue causing window to hang.
>>>
>> Fixed.
>>
>>
>>>
>>>
>>>
>>>
>>> PFA updated patch for table node,
>>> - Added help file names in js.
>>> - Added Deps for primary key cell in create table node
>>> - Corrected validation error messages
>>> - Formatted SQL templates properly
>>> - Added support for View in triggers node
>>>
>>>
>>> Regards,
>>> Murtuza
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Mon, May 9, 2016 at 5:51 PM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Harshal,
>>>>
>>>> Please find comments as below for constraints node,
>>>>
>>>> 1) Not able to create Primary key due to 'Please provide primary key'
>>>> validation error
>>>> 2) Primary key dialog do not close after save.
>>>> 3) Error "too much recursion" when creating Forgien key from New table.
>>>> 4) Error "too much recursion" when creating Check constraint from New
>>>> table.
>>>> 5) Remove console.log from JS (Unique constraint)
>>>> 6) Unique & Exclude constraint are also not working in create mode, No
>>>> SQL is generated in create mode
>>>> 7) If there are no columns on table select2 shows columns of previously
>>>> fetched objects columns.
>>>>
>>>>
>>>>
>>>> Also attaching new updated patch, which will fixes below issues,
>>>> Fixed:
>>>> =====
>>>> 1) Do not show Foreign tables under tables node
>>>> 2) In trigger node changed select2 control options as per new format.
>>>> 3) Removed unwanted templates from trigger node
>>>> 4) clean up some unwanted code from trigger node
>>>> 5) Fixed Create sql template in index node
>>>>
>>>>
>>>> Regards,
>>>> Murtuza
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>> On Sat, May 7, 2016 at 7:45 PM, Harshal Dhumal <
>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find below responses.
>>>>>
>>>>>>
>>>>>> Please find the review comments so far:
>>>>>>
>>>>>> 1. On the Table Collection node, The fields in the grid should be
>>>>>> Name, Owner and Comments. OID is not required. Please follow same for the
>>>>>> other Nodes like Index, Constraints etc.
>>>>>>
>>>>>
>>>>> Fixed
>>>>>
>>>>>> 2. While Updating the Table, Add any column as well as Inherits any
>>>>>> table, then the check the SQL tab.
>>>>>> ALTER TABLE SQL should be come in the new line
>>>>>>
>>>>>> *Current SQL Tab:*
>>>>>>
>>>>>> ALTER TABLE pem.agent_heartbeat
>>>>>> INHERIT pem.alert_history;ALTER TABLE pem.agent_heartbeat
>>>>>> ADD COLUMN test bigint;
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>> 3. While Creating/updating table, if the Schema is other than
>>>>>> selected one, then after saving the table, it is not falling under the same
>>>>>> schema. And also in update mode it gives an error.
>>>>>>
>>>>> TODO
>>>>>
>>>>>
>>>>>> 4. Unlogged setting does not honor the change of value.
>>>>>>
>>>>> Not reproducible.
>>>>>
>>>>>
>>>>>> 5. Please Check SQL tab for all the Nodes as most of them having
>>>>>> problem of No blank lines/More than one Blank Lines/Blank Lines at the end
>>>>>> etc.
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>> 6. Creating Table with auto_vacuum and updating only one field then
>>>>>> wrong SQL is generated.
>>>>>> WITH (
>>>>>> OIDS = TRUE,
>>>>>> FILLFACTOR = 12,
>>>>>> autovacuum_enabled = TRUE,
>>>>>> ,
>>>>>> autovacuum_vacuum_cost_delay = 21
>>>>>> )
>>>>>>
>>>>>> Fixed.
>>>>>
>>>>>
>>>>>> 7. Same as toast
>>>>>> WITH (
>>>>>> OIDS = TRUE,
>>>>>> FILLFACTOR = 12,
>>>>>> autovacuum_enabled = TRUE,
>>>>>> toast.autovacuum_enabled = TRUE,
>>>>>> autovacuum_analyze_scale_factor = 1,
>>>>>> autovacuum_analyze_threshold = 2,
>>>>>> autovacuum_freeze_max_age = 2,
>>>>>> ,
>>>>>> toast.autovacuum_vacuum_cost_limit = 2,
>>>>>> toast.autovacuum_freeze_min_age = 4
>>>>>> )
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>>
>>>>>> 8. Sometimes while creating table and checking sql table, below error
>>>>>> is coming
>>>>>>
>>>>>> File
>>>>>> "/home/khushboo/Projects/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
>>>>>> line 1060, in properties
>>>>>> data = res['rows'][0]
>>>>>> IndexError: list index out of range
>>>>>>
>>>>> TODO. (Need exact steps to reproduce.)
>>>>>
>>>>>
>>>>>>
>>>>>> 9. Please check all the Grid table columns. It should not be expanded
>>>>>> while editing directly into the grid. For ref: Check constraint grid
>>>>>>
>>>>> TODO
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> 10. Constraint Nodes are not covered yet due to validation issue on
>>>>>> which Harshal is working.
>>>>>>
>>>>> Done.
>>>>>
>>>>>
>>>>>>
>>>>>> 11. While creating the table if auto-vacuume has been enabled by
>>>>>> user, then it should stay enabled in Edit mode also. Currently it is not.
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>
>>>>>> 12 .If I just enable 'custom auto activated' and don't update
>>>>>> anything then SQL tab is generating below SQL which is wrong.
>>>>>>
>>>>>> ALTER TABLE pem.khushboo1 SET (
>>>>>>
>>>>>> );
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>>
>>>>>> 13. IF I Change privileges from pem_agent to agent1 then the SQL i
>>>>>> like below, which is not corrent
>>>>>>
>>>>>> REVOKE ALL ON TABLE pem.khushboo1 FROM agent1;
>>>>>> GRANT SELECT ON TABLE pem.khushboo1 TO agent1;
>>>>>>
>>>>>
>>>>> Not reproducible Or please provide steps to reproduce.
>>>>>
>>>>>
>>>>>
>>>>>> 14. In check constraint, change "Don't Validate" to Validated? Please
>>>>>> refer Domain Constraint for the same.
>>>>>>
>>>>> Fixed.
>>>>>
>>>>>
>>>>>>
>>>>>> 15. SQL for the Column is coming as below, which is not correct.
>>>>>>
>>>>>> <html><head></head><body>-- Column: col3 -- ALTER TABLE pem.khushboo1
>>>>>> DROP COLUMN col3; ALTER TABLE pem.khushboo1 ADD COLUMN col3 integer NOT
>>>>>> NULL;</body></html>
>>>>>>
>>>>>
>>>>> Not reproducible Or please provide steps to reproduce.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> 16. While updating table columns from column node. Below SQL
>>>>>> generating an error.
>>>>>>
>>>>>> ALTER TABLE pem.khushboo1
>>>>>> ALTER COLUMN col2 numeric(1, 1);
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>>
>>>>>> 17. After deleting any column, the properties of the another column
>>>>>> of the same table doesn't show up. Gives below error.
>>>>>>
>>>>>> TypeError: self.canDrop.apply is not a function
>>>>>>
>>>>>> ...lf.canDrop) ? function() { return self.canDrop.apply(self,
>>>>>> arguments); } : fals
>>>>>>
>>>>>
>>>>> This issue is already raised.
>>>>>
>>>>>
>>>>>> 18. Table Node : Exclusion constraint : Grid validates DESC instead
>>>>>> of operator.
>>>>>>
>>>>>
>>>>> Not reproducible.
>>>>>
>>>>>
>>>>>>
>>>>>> 19. Please check validation of the Exclusion control, as some JS
>>>>>> error is coming and due to this, we can not close the dialogue.
>>>>>>
>>>>>> The select2('destroy') method was called on an element that is
>>>>>> not using Select2.
>>>>>>
>>>>>>
>>>>>> ...this.$dropdown.on(d.join("
>>>>>> "),function(a){a.stopPropagation()})},a}),b.define("s...
>>>>>>
>>>>>> select2....min.js (line 3)
>>>>>> TypeError: c is undefined
>>>>>>
>>>>>
>>>>> This is already fixed.
>>>>>
>>>>>
>>>>>>
>>>>>> 20. While updating the comments field of the Index node, it throws
>>>>>> below error:
>>>>>>
>>>>>> TypeError: obj is null
>>>>>>
>>>>>> } else if ((obj.sessChanged && obj.sessChanged()) || isNew) {
>>>>>>
>>>>>
>>>>> This is already fixed.
>>>>>
>>>>>
>>>>>
>>>>>> 21. Job Trigger : Validation missing, so user can't get an idea what
>>>>>> is missing while checking the SQL tab
>>>>>>
>>>>>
>>>>> Fixed.
>>>>>
>>>>>
>>>>>> 22. For the reverse Engineering SQL tab, the constraints should be
>>>>>> start with Schema.Table.Constraint. Please follow same path for all the
>>>>>> nodes.
>>>>>>
>>>>>
>>>>> I cross checked with pgadmin3. Reverse Engineering SQL looks good to
>>>>> me. Please let me know what is missing?
>>>>>
>>>>>
>>>>>> 23. Indexes : Comments can not be updated. Please check the attached
>>>>>> screen-shot for reference.
>>>>>>
>>>>> Fixed
>>>>>
>>>>>
>>>>>> 24. Spelling mistake of 'Definition' in Indexes.
>>>>>>
>>>>>
>>>>> Fixed
>>>>>
>>>>>>
>>>>>> NOTE: I haven't check Constraints properly due to validation issue.
>>>>>> Also I have checked only functional flow, I will review the code today
>>>>>> evening or tomorrow.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Apr 27, 2016 at 2:52 PM, Harshal Dhumal <
>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA attached patches for table node and all table child nodes.
>>>>>>>
>>>>>>> This patch includes below nodes,
>>>>>>>
>>>>>>> 1) Table node *-- Initial patch by
>>>>>>> Murtuza, constraints compatibility by Harshal. *
>>>>>>> 2) Column node *-- by Murtuza. *
>>>>>>> 3) Index node *-- by Murtuza. *
>>>>>>> 4) Trigger node *-- by Murtuzz. *
>>>>>>> 6) Rules node
>>>>>>> *-- by Surinder.*
>>>>>>> 7) Constraints nodes:
>>>>>>> i] Index Constraint *-- Initial patch by
>>>>>>> Harshal, Integration with table node by **Murtuza.*
>>>>>>> ii] Foreign key *-- Initial patch and
>>>>>>> Integration with table node by Harshal**.*
>>>>>>> iii] Check constraint *-- Initial patch and
>>>>>>> Integration with table node by Harshal**.*
>>>>>>> iv] Exclusion constraint *-- Initial patch and
>>>>>>> Integration with table node by Harshal**.*
>>>>>>>
>>>>>>> Please apply patches in following order as all of them depends on
>>>>>>> each other.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Order: Table Node ----> Index constraint ---> remaining patches in
>>>>>>> any order.*
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Harshal Dhumal*
>>>>>>> *Software Engineer *
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> EenterpriseDB <http://www.enterprisedb.com>
>>>>>>>
>>>>>>> On Mon, Apr 18, 2016 at 7:04 PM, Murtuza Zabuawala <
>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find initial patch for tables node.
>>>>>>>>
>>>>>>>> This patch includes below nodes,
>>>>>>>>
>>>>>>>> 1) Tables node
>>>>>>>> 2) Columns node
>>>>>>>> 3) Index node
>>>>>>>> 4) Trigger node
>>>>>>>> 5) Constraints node (Primary key & Unique constraints only) *--
>>>>>>>> From: Harshal*
>>>>>>>> 6) Roles node
>>>>>>>> *-- From: Surinder*
>>>>>>>>
>>>>>>>> This patch also includes "VacuumSettings control" required by table
>>>>>>>> node.
>>>>>>>>
>>>>>>>> Please apply Fieldset Control UI patch sent earlier.
>>>>>>>>
>>>>>>>>
>>>>>>>> *Please note that constraint node is still partial, It has Primary
>>>>>>>> Key & Unique constraint working & integrated in tables node.*
>>>>>>>>
>>>>>>>> 1) I have used initial patch of index constraints node from
>>>>>>>> Harshal & further extend it it to work with table node.
>>>>>>>> [ Harshal will integrate rest of constraints in tables node, he is
>>>>>>>> working on it.]
>>>>>>>>
>>>>>>>> 2) I have also used initial patches of rules node and
>>>>>>>> VacuumSettings control from Surinder & further extend them it to work with
>>>>>>>> table node.
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Regards,
>>>>>>>> Murtuza Zabuawala
>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Sent via pgadmin-hackers mailing list (
>>>>>>>> pgadmin-hackers(at)postgresql(dot)org)
>>>>>>>> To make changes to your subscription:
>>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgadmin-hackers mailing list (
>>>>>>> pgadmin-hackers(at)postgresql(dot)org)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Attachment Content-Type Size
table_13_May_V5.patch.zip application/zip 83.7 KB
index_constraint_13_May_V5.patch.zip application/zip 13.1 KB
check_constraint_13_May_V5.patch.zip application/zip 9.4 KB
exclusion_constraint_13_May_V5.patch.zip application/zip 14.5 KB
foreign_key_13_May_V5.patch.zip application/zip 16.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-13 12:09:14 Re: PATCH: Initiale backup utility [pgAdmin4]
Previous Message Surinder Kumar 2016-05-13 11:03:55 Re: [pgAdmin4][Patch]: File Manager & Backform FileControl