Re: [pgAdmin4][Patch]: View and Materialised View Nodes

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: View and Materialised View Nodes
Date: 2016-05-13 18:14:39
Message-ID: CAM5-9D_28KxDdr_FhWm7wjWwsCejudE2Q1FA4pX8DKFMReYaaA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find updated patch with fixed review comments:

Most of the issues occurred because some code was missing in tables
subnodes patch.
Now I have shared the code related to table subnodes with harshal to
integrate in tables patch.

This patch has dependency on tables patch.

On Tue, May 10, 2016 at 7:46 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi Surinder
>
> On Fri, Apr 29, 2016 at 8:07 PM, Surinder Kumar <surinder.kumar@
> enterprisedb.com> wrote:
>
>> Hi,
>>
>>
>> PFA patch for View and Materialised View Nodes.
>> This patch is dependent on *tables node* and its subnodes patch.
>> Please test this patch once latest tables node patch is submitted.
>>
>> I have merged one other patch:
>> *"Don't show Security group of node if it is under Catalogs"*
>>
>> http://www.postgresql.org/message-id/CAM5-9D8RNZXRG0ygvuRqf2wQAEnye-rqBHSrwcr34dnLCemy3w@mail.gmail.com
>>
>> Below are the fix for the review comments given by Dave:
>>
>> *Views*:
>>
>> - Please add sqlCreateHelp and sqlAlterHelp properties to all nodes.
>> *I have added these in View and Materialised View. *
>>
>> - Some of the SQL templates have inconsistent indenting (i.e. not 4
>> chars), e.g.
>>
>> CREATE OR REPLACE VIEW pem.avail_agents
>> AS
>> ...
>> *Fixed*
>>
>> - When creating any object, the Owner, Schema and Tablespace should
>> have default values.
>> *Fixed*
>>
>> - Property labels should only have the first word capitalised, e.g.
>>
>> Security barrier
>> Check options
>> *Fixed*
>>
>> - The Definition box on the View dialogue should start at a single
>> line and expand as needed - see the Function dialogue
>> *Fixed*
>>
>
> Still reproducible.
>
>>
>>
>> - The Cancel button doesn't work on the View dialog. Please check them
>> all.
>> *In latest code pull, It **seems to be** fixed. not reproducible at my
>> end.*
>>
>> - The Save button doesn't close the View dialog, nor does it add the tree
>> node.
>> *In latest code pull, It **seems to be** fixed. not reproducible at my
>> end.*
>>
>> - Dependencies and Dependents don't show icons, and have very tall
>> rows. This issue likely comes from elsewhere as I'm seeing it on other
>> object types as well.
>> Yes, it is general, previously images were visible. It is regression of
>> some commit. I will check it.
>>
>> - Reverse engineered SQL for a column on a view is shown like:
>>
>> <html><head></head><body>-- Column: id -- ALTER TABLE pem.avail_agents
>> DROP COLUMN id; ALTER TABLE pem.avail_agents ADD COLUMN id
>> integer(4);</body></html>
>>
>> (yes, it's displaying the HTML tags)
>> *I pulled the latest code, but it is not reproducible.*
>>
>> - The ACL property should be called Privileges and be in the Security
>> group (see functions, sequences etc).
>> *Fixed*
>>
>> - The Schema should not be shown in "Properties" view.
>> *Fixed*
>>
>> - Fields in the General section should be in the order: Name, OID,
>> Owner, System xxx? (where appropriate), Comment
>> *I have checked that In view Fields (Name, Owner, Schema & Comment) are
>> in this order.*
>> *I didn't got your point. Can you please give and example, if possible.*
>>
>
> Not Fixed yet.
>
Now it is fixed.

>
>>
>>
>> *Materialised Views:*
>> - Selecting an MV results in:
>>
>> 2016-04-14 09:34:58,863: ERROR pgadmin: Exception on
>> /browser/materialized_view/obj/1/1/24587/27424/107612 [GET]
>> Traceback (most recent call last):
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/app.py",
>> line 1817, in wsgi_app
>> response = self.full_dispatch_request()
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/app.py",
>> line 1477, in full_dispatch_request
>> rv = self.handle_user_exception(e)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/app.py",
>> line 1381, in handle_user_exception
>> reraise(exc_type, exc_value, tb)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/app.py",
>> line 1475, in full_dispatch_request
>> rv = self.dispatch_request()
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/app.py",
>> line 1461, in dispatch_request
>> return self.view_functions[rule.endpoint](**req.view_args)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/views.py",
>> line 84, in view
>> return self.dispatch_request(*args, **kwargs)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line
>> 233, in dispatch_request
>> return method(*args, **kwargs)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_
>> groups/servers/databases/schemas/views/__init__.py",
>> line 185, in wrap
>> return f(*args, **kwargs)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_
>> groups/servers/databases/schemas/views/__init__.py",
>> line 1226, in properties
>> self.conn, result, 'table')
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_
>> groups/servers/databases/schemas/utils.py",
>> line 357, in parse_vacuum_data
>> vacuum_fields = render_template("vacuum_settings/vacuum_fields.json")
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/templating.py",
>> line 127, in render_template
>> return _render(ctx.app.jinja_env.get_or_select_template(template_
>> name_or_list),
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/jinja2/environment.py",
>> line 830, in get_or_select_template
>> return self.get_template(template_name_or_list, parent, globals)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/jinja2/environment.py",
>> line 791, in get_template
>> return self._load_template(name, self.make_globals(globals))
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/jinja2/environment.py",
>> line 765, in _load_template
>> template = self.loader.load(self, name, globals)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/jinja2/loaders.py",
>> line 113, in load
>> source, filename, uptodate = self.get_source(environment, name)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
>> packages/flask/templating.py",
>> line 64, in get_source
>> raise TemplateNotFound(template)
>> TemplateNotFound: vacuum_settings/vacuum_fields.json
>>
>> *It seems this patch*
>>
>> http://www.postgresql.org/message-id/CAM5-9D-4DYgMMHFc5mzDDdK++h7XNvkiA5ojBbXi5sTHmecGCA@mail.gmail.com
>> *wasn't applied properly. Now this patch is sent with tables patch.*
>>
>> - When creating an MV, an error is immediately shown telling me to
>> specify a name. We do not normally show such errors until the field
>> loses focus.
>> *Now its fixed.*
>>
>
> Not fixed. Issue is reproducible.
>
>>
>>
>> - No default owner is shown in create mode.
>> *Fixed.*
>>
>> - "Please enter function definition." is shown as an error on the
>> definition field.
>> *It triggers this error message as it is mandatory.*
>>
>> - The Definition field is missing a label. If relying on the tab title
>> to be the label, the control should fill the tab (as with Security
>> options etc).
>> *Fixed.*
>>
>> - No default tablespace is shown.
>> *Fixed*
>>
>> - If I enable custom auto-vacuum, there is no way to add a row to
>> specify options.
>> *You can enable custom auto vacuum field and add new values in the auto
>> vacuum settings.*
>>
>> - If I specify a comment on an MV, I get "ERROR: "mv_pg_tables" is not a
>> view"
>> *For some reason tablespace name was missing in generated sql.*
>>
>
> Not Fixed. Wrong sql generated "COMMENT ON VIEW ..." instead of
> "COMMENT ON MATERIALIZED VIEW ..."
>
Fixed

>
>>
>> - Save/Cancel buttons not working as expected (like other objects)
>> *In latest code pull, It **seems to be** fixed. not reproducible at my
>> end.*
>>
>
>
> Apart from above below are my review comments
>
> *Views*:-
>
> - As per pgAdmin3 "indexes" node should not be listed under Views.
>
> Done

>
> - SQL not generated when changing the value of "Event" and "Do
> Instead" for Rule node under View node.
>
> Done

>
> - Changing the "Event" in Rule node not working.
>
> Done

>
> - As per pgAdmin3 user can't be able to create columns inside View
> node.
>
> Done

>
> - User can't be able to delete/drop columns and system generated
> Rule's, Trigger's etc..
>
>
Done

>
> - Found one issue when changing value of "Security Barrier" from "Yes"
> to "No" it is not reflected on GUI when we open the dialog again while
> in backend value is updated, but on GUI it is showing "Yes".
>
> Done

>
> - Create -> Trigger menu is missing when any view node is selected.
>
> Done

>
> *Materialized View:-*
>
> - *As per pgAdmin3 user can't be able to create columns inside
> Materialized View node.*
>
> Done

>
> -
> -
> *User can't be able to delete/drop columns and system generated Rule's,
> Trigger's etc. *
>
> I have checked in pgAdmin3 that delete/drop option specific to rules has 2
cases:
1. If rule is system rule, user can't delete/drop it.
2. If not system rule, it can drop dropped.
Done

>
> -
> - Create -> Trigger/Rule/Index menu is missing when any materialized
> view node is selected.
>
> Done

>
> - In pgAdmin3 we have two more refresh options "Refresh data" and
> "Refresh data concurrently" which is missing.
>
> Done

>
> - "Custom Auto Vaccum" for Table and Toast Table not working.
>
> Done

>
>
>>
>> Thanks,
>> Surinder Kumar
>>
>>
>>
>> --
>> 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
>>
>>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

Attachment Content-Type Size
view_and_mat_view_nodes_v1.patch application/octet-stream 211.7 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-13 19:12:56 pgAdmin 4 commit: Improvised the file manager functionalities as per co
Previous Message Surinder Kumar 2016-05-13 17:02:20 [pgAdmin4][Patch]: Menu's canDrop as function is not callable