| 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>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com> |
| Subject: | Re: [PATCH] Tables node (pgAdmin4) |
| Date: | 2016-04-28 10:06:11 |
| Message-ID: | CAFOhELea6nCtX_T78o_3mNKJC0_emirXKGm3nq8d6mvSZV57pg@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgadmin-hackers |
Hi,
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.
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;
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.
4. Unlogged setting does not honor the change of value.
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.
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
)
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
)
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
9. Please check all the Grid table columns. It should not be expanded while
editing directly into the grid. For ref: Check constraint grid
10. Constraint Nodes are not covered yet due to validation issue on which
Harshal is working.
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.
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 (
);
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;
14. In check constraint, change "Don't Validate" to Validated? Please refer
Domain Constraint for the same.
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>
16. While updating table columns from column node. Below SQL generating an
error.
ALTER TABLE pem.khushboo1
ALTER COLUMN col2 numeric(1, 1);
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
18. Table Node : Exclusion constraint : Grid validates DESC instead of
operator.
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
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) {
21. Job Trigger : Validation missing, so user can't get an idea what is
missing while checking the SQL tab
22. For the reverse Engineering SQL tab, the constraints should be start
with Schema.Table.Constraint. Please follow same path for all the nodes.
23. Indexes : Comments can not be updated. Please check the attached
screen-shot for reference.
24. Spelling mistake of 'Definition' in Indexes.
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 |
|---|---|---|
| Screen Shot 2016-04-28 at 2.42.28 pm.png | image/png | 200.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Neel Patel | 2016-04-28 13:09:08 | Re: [pgAdmin4][Patch]: File Manager & Backform FileControl |
| Previous Message | Dave Page | 2016-04-28 08:52:59 | Re: [PATCH] Tables node (pgAdmin4) |
