Quick Links

Re: [PATCH] Tables node (pgAdmin4)

From:	Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To:	pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Cc:	Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Subject:	Re: [PATCH] Tables node (pgAdmin4)
Date:	2016-05-07 14:15:34
Message-ID:	CAFiP3vypf7KEmToKsBqn1--qf_PzMV+p-d9bsMj04eoYnj5_tQ@mail.gmail.com
Views:	Raw Message \| Whole Thread \| Download mbox \| Resend email
Thread:
Lists:	pgadmin-hackers

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
>>
>>
>

In response to

Re: [PATCH] Tables node (pgAdmin4) at 2016-04-28 10:06:11 from Khushboo Vashi

Responses

Re: [PATCH] Tables node (pgAdmin4) at 2016-05-09 12:21:48 from Murtuza Zabuawala

Browse pgadmin-hackers by date

	From	Date	Subject
Next Message	eMerzh	2016-05-08 12:39:35	Re: pgAdmin4 - Session file name too large
Previous Message	Harshal Dhumal	2016-05-07 14:13:53	Re: [PATCH] Tables node (pgAdmin4)