Re: [PATCH] Tables node (pgAdmin4) - Mailing list pgadmin-hackers

From Khushboo Vashi
Subject Re: [PATCH] Tables node (pgAdmin4)
Date
Msg-id CAFOhELdg9-TB=fdpGDLNkjYve9vBxYttE_PzmahsrNMQv4nn4Q@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Tables node (pgAdmin4)  (Harshal Dhumal <harshal.dhumal@enterprisedb.com>)
Responses Re: [PATCH] Tables node (pgAdmin4)
List pgadmin-hackers
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.dhumal@enterprisedb.com> wrote:
Hi,


-- 
Harshal Dhumal
Software Engineer 




On Tue, May 10, 2016 at 6:37 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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.zabuawala@enterprisedb.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.dhumal@enterprisedb.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.dhumal@enterprisedb.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 




On Mon, Apr 18, 2016 at 7:04 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers







pgadmin-hackers by date:

Previous
From: Harshal Dhumal
Date:
Subject: Re: [PATCH] Tables node (pgAdmin4)
Next
From: Akshay Joshi
Date:
Subject: Re: variable scope issue in server js [pgadmin4]