Quick Links

Re: [PATCH] Tables node (pgAdmin4)

From:	Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To:	Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
Cc:	pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject:	Re: [PATCH] Tables node (pgAdmin4)
Date:	2016-05-13 13:25:33
Message-ID:	CAFOhELd=_vuqwR0eGLL=KhxGaQCP6n9mcXP1GijcWh1ecCpegw@mail.gmail.com
Views:	Raw Message \| Whole Thread \| Download mbox \| Resend email
Thread:
Lists:	pgadmin-hackers

Hi,

Review Comments:

- Please replace 'can not' with 'cannot' in all the validation messages.
- PG 9.1+ Inheritance issue as below:

CREATE TABLE public.table1
(
)
(
)
INHERITS (a)
WITH (
OIDS = FALSE
)
TABLESPACE pg_default;
ALTER TABLE public.table1
OWNER to postgres;

brackets are coming twice.

- Please maintain one line spacing between SQL queries In the SQL Tab.
- Foreign Key Grid in Table css issue: Grid columns expands on the
selection of the cell
- Check Constraint: Validated? option should be True by default
- pg 9.4: Exclude constraint does not render in SQL tab
- Missing Security validation
- Vacuum grid should not be editable in properties mode.
- Edit mode, Fill Factor can be allowed to be null.
- While dropping inheritance, related table columns drop SQL are also
populated in the SQL Tab

ALTER TABLE public."Tbl"
NO INHERIT b;
ALTER TABLE public."Tbl" DROP COLUMN id;
ALTER TABLE public."Tbl" DROP COLUMN name;

And also render error while clicking on the save button.

ERROR: syntax error at or near "["
LINE 2: INHERIT [;
^

- in a Reverse Engineering SQL tab, schema_name.tablename should be there,
currently only table_name displays.
- Column SQL is showing below text with HTML

<html><head></head><body>-- Column: id -- ALTER TABLE public.a DROP COLUMN
id; ALTER TABLE public.a ADD COLUMN id integer;</body></html>

- The column datatype dependency does not get cleared upon selection of
another datatype.
For example, if I select numeric and gives the length and precision. After
that I change the dat-type then, dependent fields should be get cleared.

- The column datatype does not get displayed in the properties and edit
mode if the length and precision are given while creating a column.

- Statistics is showing null value even after having value.

- if the check constraints are not validated then put proper icon in tree
and also it should be validated in edit mode.

NOTE: I have not checked the Indexes, Triggers and Rules nodes as I do not
have much knowledge about it.

Thanks,
Khushboo

On Fri, May 13, 2016 at 5:24 PM, Harshal Dhumal <
harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:

> 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
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
> --
> 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-05-13 11:54:10 from Harshal Dhumal

Responses

Re: [PATCH] Tables node (pgAdmin4) at 2016-05-13 20:33:42 from Harshal Dhumal

Browse pgadmin-hackers by date

	From	Date	Subject
Next Message	Surinder Kumar	2016-05-13 13:41:42	Re: [pgAdmin4][Patch]: Load/Save file in query tool
Previous Message	Dave Page	2016-05-13 13:11:56	Re: Patch for pgAdmin4 package on Mac OS X