Re: [PATCH] Tables node (pgAdmin4) - Mailing list pgadmin-hackers
From | Khushboo Vashi |
---|---|
Subject | Re: [PATCH] Tables node (pgAdmin4) |
Date | |
Msg-id | CAFOhELd=_vuqwR0eGLL=KhxGaQCP6n9mcXP1GijcWh1ecCpegw@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)
|
List | 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.table1OWNER 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.dhumal@enterprisedb.com> wrote:
Hi
PFA attached patches for table and it's child nodes with python 2.7 compatibility.On Thu, May 12, 2016 at 6:55 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:KhushbooThanks,Please test the patch on Python 2.7, fix the issues and resend the patch.Hi,I have started reviewing the patch but the basic functionalities are breaking on python 2.7.On Thu, May 12, 2016 at 6:03 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:Hi,On Tue, May 10, 2016 at 6:37 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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 nodeRegards,Murtuza--Regards,On Mon, May 9, 2016 at 5:51 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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 error2) 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 mode7) 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 node2) In trigger node changed select2 control options as per new format.3) Removed unwanted templates from trigger node4) clean up some unwanted code from trigger node5) Fixed Create sql template in index nodeRegards,Murtuza--Regards,On Sat, May 7, 2016 at 7:45 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.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.Fixed2. 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 lineCurrent SQL Tab:
ALTER TABLE pem.agent_heartbeat
INHERIT pem.alert_history;ALTER TABLE pem.agent_heartbeat
ADD COLUMN test bigint;Fixed3. 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.TODO4. 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.Fixed6. 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
)Fixed8. 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 rangeTODO. (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 gridTODO10. 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.Fixed12 .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); } : falsThis 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 undefinedThis 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 tabFixed.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.Fixed24. Spelling mistake of 'Definition' in Indexes.FixedNOTE: 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,KhushbooOn Wed, Apr 27, 2016 at 2:52 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:ii] Foreign key -- Initial patch and Integration with table node by Harshal.i] Index Constraint -- Initial patch by Harshal, Integration with table node by Murtuza.7) Constraints nodes: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.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.
On Mon, Apr 18, 2016 at 7:04 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:--Hi,Please find initial patch for tables node.This patch includes below nodes,1) Tables node2) Columns node3) Index node4) Trigger node5) Constraints node (Primary key & Unique constraints only) -- From: Harshal6) Roles node -- From: SurinderThis 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,
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
--
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: