Re: [PATCH] Tables node (pgAdmin4)

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