Re: Stalled post to pgadmin-hackers - Mailing list pgadmin-hackers

On 18-May-2016, at 4:01 pm, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:

Hi,

PFA patch with resolved review comments.

On Tue, May 17, 2016 at 6:46 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Surinder,

Please find my comments as below for latest v2. patch,

1) In CREATE mode, Removing Owner & Schema fields causes SQL to break

CREATE OR REPLACE VIEW "<Response 159 bytes [500 INTERNAL SERVER ERROR]>"."test view" AS

select * from table1;

ALTER TABLE "<Response 159 bytes [500 INTERNAL SERVER ERROR]>"."test view"

    OWNER TO pem_agent;

Steps to re-produce,

- enter name

- enter definition

- remove owner & schema from select2 list

go to SQL tab

Fixed. 

2) Renaming view do not reflect on browser tree (We need to refresh manually)

Steps to re-produce,

- enter new name

Click on Save

Fixed. 

3) Please maintain one line spacing between SQL queries In the SQL Tab.

SQL generated (current patch):

--------------

CREATE OR REPLACE VIEW pem.dsfasfa

WITH (

    check_option=local,

    security_barrier=true

) AS

select * from agent;

ALTER TABLE pem.dsfasfa

    OWNER TO postgres;

COMMENT ON VIEW pem.dsfasfa

    IS 'my comments...';

SECURITY LABEL FOR pro ON VIEW dsfasfa IS 'lbl1';GRANT ALL ON TABLE pem.dsfasfa TO pem_admin;

Fixed. 

4) EDIT Mode, Changing schema generates wrong SQL, eg: Schema changed to "Public" generated below sql,

ALTER VIEW pem."Test_View"

    SET SCHEMA "2200";

Fixed. 

5) EDIT Mode, If i remove check option, it do not generate SQL for that operation

Fixed. 

6) EDIT Mode, If i modified definition, it creates new view but what about my current setting like privileges, security labels, security barrier etc

Those should be preserved.

Fixed. 

7) Security barrier is not being displayed on properties node

Fixed. 

8) Rule node, Edit mode, I am not allowed to rename Rule, but as per postgres doc we can only rename it rest options should be disabled in edit mode.

http://www.postgresql.org/docs/9.5/static/sql-alterrule.html

I will send separate patch for it once done.

9) Column node, 'Primary key' switch should be hidden under view node, refer column node visible condition for table node

I will send a add-on patch for this issue. 

10) Same issue in MATERIALIZED VIEW as mentioned in Issue-1

Fixed. 

11) Same issue in MATERIALIZED VIEW as mentioned in Issue-3

Fixed. 

12) VaccumSetting options are not Sync with Table node, as per Postgres Docs it should use same settings as CREATE TABLE storage settings options.

In Table: When we only set

- Custom auto-vacuum? (table)

- Custom auto-vacuum? (toast table)

it generates SQL like below but in MATERIALIZED VIEW it does not,

WITH (

    autovacuum_enabled = FALSE,

    toast.autovacuum_enabled = FALSE

)

Fixed. 

13) MATERIALIZED VIEW, Edit mode, remove one Privilege from MV and see there is no sql generated in SQL tab

Fixed. 

14) MATERIALIZED VIEW, Edit mode, Security label do not work, try adding new security label

Not an issue. it wasn't working due to issue # 14.

15) MATERIALIZED VIEW, Edit mode, Changing the definition does nothing, it should drop & re-create MV with all previous options & along with new definition

Fixed. 

16) SQL alignment,

eg: generated sql

ALTER MATERIALIZED VIEW pem."test_MV1"

SET(FILLFACTOR = 25);

Expected:

ALTER MATERIALIZED VIEW pem."test_MV1"

    SET(FILLFACTOR = 25);

Fixed. 

 

17) MATERIALIZED VIEW, Edit mode, we should not allow to TableSpace

It is fixed. it is reproduced due to select control, its option 'allow_clear' should be set default to 'false'.

18) Same issue in MATERIALIZED VIEW as mentioned in Issue-4

Fixed.

19) Same issue in MATERIALIZED VIEW as mentioned in Issue-2

Fixed. 

20) We can not Fill factore values in MV

Fixed. 

Regards,

Murtuza

---------- Forwarded message ----------
From: Surinder Kumar <surinder.kumar@enterprisedb.com>
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>
Date: Tue, 17 May 2016 11:52:35 +0530
Subject: Re: [pgadmin-hackers] [pgAdmin4][Patch]: View and Materialised View Nodes

Hi,

PFA updated patch.

Please find inline comments.

On Mon, May 16, 2016 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi 

Below are my review comments: 

• Definition box on the View dialogue not expanded as needed. It should expand on resize.

removed the fixed height of definition box, but its height is dependent on the number of sql code lines in it. 

• 
  
• "Do Instead" on Rule node under View/M-View node not working. Unable to generate proper SQL.

Done 

• I am still able to drop columns under view/mview node.

Done 

• "Save" button is enable by default when user opens Materialised View dialog.  

I checked it is not reproducible. 

On Fri, May 13, 2016 at 11:44 PM, Surinder Kumar <surinder.kumar@enterprisedb.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.joshi@enterprisedb.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--

Akshay Joshi

Principal Software Engineer 

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

--

Akshay Joshi

Principal Software Engineer 

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

<view_and_mat_view_nodes_v2.patch>

<view_and_mat_view_nodes_v3.patch>