Quick Links

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

From:	Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To:	Surinder Kumar <surinder(dot)kumar(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-16 08:26:17
Message-ID:	CANxoLDd-HE-wfgcfQhGFXG3cU4Sf1qosKxO9P84XDQLaNdq7zw@mail.gmail.com
Views:	Raw Message \| Whole Thread \| Download mbox \| Resend email
Thread:
Lists:	pgadmin-hackers

Below are my review comments:

- Definition box on the View dialogue not expanded as needed. It should
expand on resize.
- "Do Instead" on Rule node under View/M-View node not working. Unable
to generate proper SQL.
- I am still able to drop columns under view/mview node.
- "Save" button is enable by default when user opens Materialised View
dialog.

On Fri, May 13, 2016 at 11:44 PM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

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

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Re: [pgAdmin4][Patch]: View and Materialised View Nodes at 2016-05-13 18:14:39 from Surinder Kumar

Responses

Re: [pgAdmin4][Patch]: View and Materialised View Nodes at 2016-05-17 06:22:35 from Surinder Kumar

Browse pgadmin-hackers by date

	From	Date	Subject
Next Message	Ashesh Vashi	2016-05-16 12:59:47	pgAdmin 4 commit: Check for valid object existence before checking its
Previous Message	Surinder Kumar	2016-05-16 08:16:43	Re: [pgAdmin4][Patch]: Load/Save file in query tool