Re: PATCH: Added Node Type & Catalog objects [pgAdmin4]

Hi Dave,

Thank you for reviewing my patch.

I have made changes according to your feedback.

Please review updated patch.

Regards,
Murtuza

On Fri, Mar 11, 2016 at 9:44 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> And this time with the patch.
>
> On Fri, Mar 11, 2016 at 4:11 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> > On Tue, Mar 8, 2016 at 1:38 PM, Murtuza Zabuawala
> > <murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
> >> Hi,
> >>
> >> PFA patch to add new nodes in pgAdmin4.
> >> 1) Type node
> >> 2) Catalog objects
> >>
> >> Note: Both above nodes depended on schema/catalog node, Please apply
> them
> >> after latest patch of schema/catalog from Ashesh.
> >> - Type node also depends on parse_priv_function_templates.patch (which I
> >> sent in separate email today)
> >
> > The type node seems to need quite a bit of work - please review and
> > test it carefully before re-submitting. There's an updated patch
> > attached, and some review info below:
> >
> > Changed in the attached patch:
> >
> > - s/Oid/OID
> > - Set defaults for schema and owner
> > - Rename the Type Defintion group to Definition.
> > - Moved some properties (acl, members) into the appropriate group
> > - s/Enumration/Enumeration
> >
> > To be fixed:
> >
> > - This module is derived from SchemaChildModule, so why does it still
> > have the backend support SQL?
> >
>
Done (Apologies I forgot to delete them)

> > - I cleaned up most of the SQL, which was improperly indented in many
> > places. However I have not event tried to fix up the create.sql
> > scripts. These need reformatting to:
> > - Use 4 character indents
> > - Not start a line with a comma - e.g. " ,FOO=bar", which should be
> > " FOO=bar," (obviously the commas need to trail from the line
> > before).
>
Done

> >
> > - The "Show System Objects" option is not honoured.
> >
>
Done

> > - The members list should be delimited with ", " not ","
> >
>
Done

> > - The ACL columns don't match other objects on the subnode panel -
> > Grantee/Granter/Privileges should be Grantee/Privileges/Granter
> >
>
Done

> > - Attempting to add a security label with empty values gives: SECURITY
> > LABEL FOR None ON TYPE foo_enum IS NULL;
> >
> > - The schema is omitted from GRANT statements when creating an object,
> e.g.
> >
> > CREATE TYPE pem.foo_enum AS ENUM
> > ('foo', 'bar', 'whizz');
> >
> > ALTER TYPE pem.foo_enum OWNER TO postgres;
> >
> > COMMENT ON TYPE pem.foo_enum IS 'This is the foo enum';
> >
> > GRANT ALL ON TYPE foo_enum TO pem_admin;
> >
>
Done

> > - Move remaining Properties display properties to the appropriate
> > group - e.g. ENUM labels should be in the Definition group. Only Name,
> > OID, Owner, Alias, System Type and Comment should be under General.
> >
>
Done

> > - The schema name is omitted when dropping a type, leading to: type
> > "foo_enum" does not exist
> >
>
Done

> > - The dependencies and dependents tabs don't display any data.
> >
>
Done

> Thanks.
> >
> > --
> > Dave Page
> > Blog: http://pgsnake.blogspot.com
> > Twitter: @pgsnake
> >
> > EnterpriseDB UK: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>