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-12 12:32:35
Message-ID: CAFiP3vwppxZfCYdxHOEFRX5x61XojB=zasDcKUNasfV8edo5xQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

PFA updated patches for table and it's child nodes.

Apply order Table node -> Index constrain -> remaining in any order

--
*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.
> *2)* MultiSelect2 rendering issue causing window to hang.
>
>
>
> 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_12_May_V4.patch.zip application/zip 83.6 KB
index_constraint_12_May_V4.patch.zip application/zip 13.1 KB
foreign_key_12_May_V4.patch.zip application/zip 16.0 KB
check_constraint_12_May_V4.patch.zip application/zip 9.3 KB
exclusion_constraint_12_May_V4.patch.zip application/zip 14.5 KB

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Harshal Dhumal 2016-05-12 12:33:11 Re: [PATCH] Tables node (pgAdmin4)
Previous Message Akshay Joshi 2016-05-12 12:14:26 Re: PATCH: Added "Named restore point" functionality (pgAdmin4)