Postgres Pro
Downloads
Home   >   mailing lists

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

From Sanket Mehta
Subject Re: [PATCH] Tables node (pgAdmin4)
Date
Msg-id CA+yw=mPp8CY+jd3FUXeV-KSd+82dztXYWySJVLuL9eoE9hrbfQ@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)  (Harshal Dhumal <harshal.dhumal@enterprisedb.com>)
List pgadmin-hackers
Tree view
Hi Harshal,


Below are my review comments:

 I got below warning when I tried to apply the patch for table node as mentioned below:

Table creation:
  •  Trailing white spaces warnings 
                    $ git apply /projects/patches/pgadmin4/Table/table_14_May_V6.patch
                    /projects/patches/pgadmin4/Table/table_14_May_V6.patch:6008: trailing whitespace.
                              return false; 
                   /projects/patches/pgadmin4/Table/table_14_May_V6.patch:6016: trailing whitespace.
                              return false; 
                   warning: 2 lines add whitespace errors.
  • In Table creation dialog, while adding a new primary key, it does not allow to change the tablespace to empty. (which is not the case in case of tablespace in table)
  • In Table creation dialog, while adding a new column, data type and name field must be mandatory. otherwise while clicking on save it gives below error
                  File "/projects/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py", line 1319, in _parse_format_columns
                   c['cltype'] = self._cltype_formatter(c['cltype'])
                   KeyError: 'cltype
  • In Table creation dialog, While adding a new column, in primary check box is needed to click twice in order to check it. Ideally it should be checked by only one click.
  • In Table creation dialog, While adding a new column, primary key should not be allowed to added unless user has provided name and data type for atleast one column.
    currently if user has clicked on add column button and immediately click on add primary key button, it will add a row in primary key data grid
  • When delete table/drop cascade is apply on any table, i got a javascript error as mentioned below
                 node.js:94 Uncaught TypeError: self.canDrop.apply is not a function
  • Once the above error generated, every time user tries to open a context menu by right clicking on any existing table, that same error comes 
  • In table creation dialog, if table is inherited from another table, if a new primary key is added manually there, the create sql will not have an entry for primary key
  • In table creation dialog, if primary key check box is checked while adding the row, a new row is added in primary key datagrid but unchecking the primary key checkbox from column datagrid, does not removes that row from primary key data grid.
  • In AutoVacuum tab, if user provides any invalid value to any parameter, then a error message should be prompted, only background color change would not tell user to change the value.
  • In table creation dialog, security label are not being added. javascript error is coming as mentioned below:
      {"success": 0, "info": "", "result": null, "data": null, "errormsg": "can't adapt type 'Undefined'"}
  • In Table creation dialog, while adding foreign key, in action tab. if user click on 'x' button in "on update" or "on delete" select2 control, it gives error "Uncaught SyntaxError: Unexpected end of input"
  • In Table creation dialog, while adding a check constraint, "validated" button does not work properly
  • After successfully creation of table, "table name cannot be empty" error is not getting cleared.
  • In Table creation dialog, if user has added an empty column without entering its name or type and trying to add check constraint, it will add an empty constraint
  • In Table creation dialog, while adding an exclude constraint, for "character varying" column type, no operators are being listed
  • In Table creation dialog, while adding an exclude constraint, below mentioned error comes if user removes operator class by clicking 'x' on that control Uncaught TypeError: Cannot read property 'id' of undefined
  • In Table creation dialog, SQL is not getting generated for exclude constraint
  • In Table creation dialog, schema should be prefixed with table name in "of type" control
  • In Table creation dialog, while adding privileges, it always shows default privileges even if user has selected different privileges. (This works fine once user edit the privileges in edit table mode and shows only those privileges which user selects). Ashesh, please confirm the behaviour.
Table edit mode:
  • If in edit mode, any constraint is already having any comment, then remove it. It will not create the SQL for the same.
  • Changing Schema will give server error
Column Creation:
  • Save button is enabled by default
  • Data type validation is not provided. Save button is enabled just after providing column name
  • Length field limitation is not provided. (i.e. for numeric type, length should be allowed greater than 1000)
Exclusion constraint creation:
  • Access method should not be allowed to be empty. (currently by clicking 'x' will remove the selection in it)
Index creation:
  • No error message for name field when empty
  • No error message when column name is not provided while adding a column in index
  • While adding a column if no name is provided, "None" appears in SQL tab which will give error on OK button click
  • when comment is provided while creation, it gives error saying index does not exists. because schema name is not added before it.
Rule creation:
  • Name is empty error does not come till user enters something in definition tab
  • DO INSTEAD button does not make any difference to SQL (it works in edit mode)
Rule edit mode:
  • Add comment in edit mode, check the SQL in sql tab. Now come back to general tab and removes comment and check the sql tab again.
    SQL for comment is still there with empty string as comment
Trigger Creation:
  • SQL is not proper when creating a trigger. "()" should be appended to function name in SQL.
    It gives error while creating a trigger
  • "+" sign is visible in browser tree in front of trigger.  either On expanding trigger, it should show the trigger function name or that "+" sign should not appear
Trigger edit node:
  • On removing comment, nothing happens. No sql is being created. Comment is still there in properties. 


Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Mon, May 16, 2016 at 7:02 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

PFA add-on patch for table and it's child node. (please apply this patch on version 6 patch)

Murtuza and I fixed following issues:

1. SQL formatting
2. Vacuum grid should not be editable in properties mode.
3. Column datatype does not get displayed in the properties and edit mode.
4. Do not allow to add another primary key if one already exist.

And another minor enhancements.


-- 
Harshal Dhumal
Software Engineer 




On Sat, May 14, 2016 at 2:03 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi,


PFA updated patches (version: 6) for table and it's child nodes.

-- 
Harshal Dhumal
Software Engineer 




On Fri, May 13, 2016 at 6:55 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
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.
Fixed
 

- Please maintain one line spacing between SQL queries In the SQL Tab.
TODO
 
- Foreign Key Grid in Table css issue: Grid columns expands on the selection of the cell
Fixed
 
- Check Constraint: Validated? option should be True by default
Not sure about this. I cross checked in pgadmin3.

 
- pg 9.4: Exclude constraint does not render in SQL tab
Fixed
 
- Missing Security validation
Fixed
 
- Vacuum grid should not be editable in properties mode.
TODO (It's editable but one cannot save it on server from here as there is no save button.)
 
- Edit mode, Fill Factor can be allowed to be null.
TODO (This is generic issue in Integer and Numeric controls. This issue is covered in this partial patch)
 
- 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;

Fixed

 

And also render error while clicking on the save button.
ERROR: syntax error at or near "["
LINE 2: INHERIT [;
^
Fixed
 
- in a Reverse Engineering SQL tab, schema_name.tablename should be there, currently only table_name displays.
Fixed

 
- Column SQL is showing below text with HTML

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

I was not able to reproduce exact issue but still I have fixed other issue which I found related to column SQL. Hopefully that will fix this issue as well.
 
- 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.
Fixed.
 

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

TODO ( I cannot fix this blindly as this might introduce another issue(s) in column node. I will need Murtuza's help as he has worked on column node)
 

- Statistics is showing null value even after having value. 
Fixed
 

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

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.dhumal@enterprisedb.com> wrote:
Hi

PFA attached patches for table and it's child nodes with python 2.7 compatibility.





--
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: Akshay Joshi
Date:
Subject: pgAdmin 4 commit: Remove testing code
Next
From: Dave Page
Date:
Subject: pgAdmin 4 commit: Use a 2 part version number. We never used the first