MIME-Version: 1.0
Date: Thu, 14 Apr 2016 10:39:39 +0100
Message-ID: 
 <CA+OCxozN++8sFqJwJZ5QSFBxa7p-47apohOLtSotPBK6N5vUJA@mail.gmail.com>
Subject: Review: Views, Mat. Views, Vaccum controls, Columns, Indexes, Rules,
 Triggers
From: Dave Page <dpage@pgadmin.org>
To: Surinder Kumar <surinder.kumar@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>
Content-Type: text/plain; charset=UTF-8
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

Hi

This review combines the patches from the following threads. As they
are so interdependent, please work on them as one patch moving
forward:

http://www.postgresql.org/message-id/CAM5-9D93fb6hDGWANGhwO5J+LaszZ7rpQm1A1HHg9qjOrOPCiA@mail.gmail.com
http://www.postgresql.org/message-id/CAM5-9D-v+6Jb=DCZgr66muizZAT4AjkcwTGf-rkG+-JF7o=_Eg@mail.gmail.com
http://www.postgresql.org/message-id/CAM5-9D_JXhPDZh63hmSRfYmM_6X4df3iD2Pu9NU_nJ1w8ymKMw@mail.gmail.com

I will combine them into a single card on our internal tracking
system, and assign to Surinder.

Review comments (this is a long list, but much of it is trivial UI/styling etc):

General:

- Please add sqlCreateHelp and sqlAlterHelp properties to all nodes.
- Some of the comments below apply to multiple object types. Please
ensure everything is consistently laid out in the UI, and that
"general" issues that apply to one node are fixed on any others that
might be affected.

Views:

- Some of the SQL templates have inconsistent indenting (i.e. not 4 chars), e.g.

CREATE OR REPLACE VIEW pem.avail_agents
 AS
 ...

- When creating any object, the Owner, Schema and Tablespace should
have default values.

- Property labels should only have the first word capitalised, e.g.

  Security barrier
  Check options

- The Definition box on the View dialogue should start at a single
line and expand as needed - see the Function dialogue

- The Cancel button doesn't work on the View dialog. Please check them all.

- The Save button doesn't close the View dialog, nor does it add the tree node.

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

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

- The ACL property should be called Privileges and be in the Security
group (see functions, sequences etc).

- The Schema should not be shown in "Properties" view.

- Fields in the General section should be in the order: Name, OID,
Owner, System xxx? (where appropriate), Comment

Columns:

- The General properties group should show the standard info - Name,
Position, Inherited, Inherited from table, System Column, Comment

- Please move Primary Key, and Foreign Key to a "Constraints" group.

- The ACL should be shown as a string, not a grid.

- Field ordering is bizarre - Primary Key? appears before Name. Please
check ordering, and ensure group consistency with other objects. Ping
me with any questions.

- Variables should be on their own tab, not the Security tab.

- There's no blank line before COMMENT ON in the Create script, or
before GRANT...

Triggers:

- The Definition tab on the Create dialogue needs to be split up. No
scrolling should be required!

- The Code tab should be hidden on PostgreSQL. On PPAS, it needs to be
clear that either a function or code should be specified.

- The Fires option doesn't offer any values.

- s/TURNCATE/TRUNCATE

- I'm not required to select an event before attempting to create a trigger.

- Creating a trigger executes the CREATE query twice, erroring the second time.

- Renaming and/or commenting on an existing trigger generates "Nothing
changed" SQL.

- Please move events into their own property group and lose the frame
around them.

- Auto-complete for columns doesn't appear to work.

Rules:

- There should be a blank line in the create SQL between the
statements and COMMENT ON

- Let's move the Statements field to the Definition group/tab, and
rename it to Commands (which is used in the PG docs)

- The Save button doesn't close the dialogue, or add the new item to
the treeview.

- The Cancel button doesn't work

- I cannot rename a rule (the name field is disabled)

- Rule commands and conditions should be shown in the Definition panel
of the properties.

- The ordering of properties in the properties list doesn't follow the
normal standard.

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

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

- No default owner is shown in create mode.

- "Please enter function definition." is shown as an error on the
definition field.

- 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).

- No default tablespace is shown.

- If I enable custom auto-vacuum, there is no way to add a row to
specify options.

- If I specify a comment on an MV, I get "ERROR: "mv_pg_tables" is not a view"

- Save/Cancel buttons not working as expected (like other objects)

Indexes:

- Default tablespace not specified.

- Columns should be on a separate tab on the Create dialogue
(scrolling should not be required)

- A default Access Method should be specified.

- The switch controls for Sort order and NULLs first/last should show
both labels in green (as neither option is more important or dangerous
etc. than the other)

- The Create SQL should have a blank line before COMMENT ON

- The collation option should be disabled for data types for which it
is not supported (e.g. name)

- If I specify an opclass on a column, but not a collation, the SQL is
malformed and leads to an error: (schemaname DESC NULLS FIRST,
tablenamebpchar_pattern_ops DESC NULLS LAST)

- Most of the fields in the General section of the Properties list
should be in the Definition section. Please check the ordering is
consistent too.

- Comments on indeexes don't work - the schema name is omitted.

Thanks!

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers