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

Thanks - patch applied!

On Thu, Apr 14, 2016 at 2:31 PM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> Sorry my bad I misinterpreted, I thought you meant rename/changing schema
> only.
>
> PFA updated patch.
>
> And For Dialog not closing issue is generic and I have already created
> internal ticket for it.
>
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Apr 14, 2016 at 6:44 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi,
>>
>> The first issue is only partially fixed - it'll still let me try to
>> rename a shell type.
>>
>> Also, I notice when hitting Save on my changes, they're applied (if
>> they're valid), but the dialogue doesn't close. I see the following in
>> the browser console:
>>
>> Uncaught TypeError: Cannot read property 'sessChanged' of
>> null(anonymous function) @ datamodel.js:333
>> _.each._.forEach @ underscore.js:153
>> pgBrowser.DataModel.Backbone.Model.extend.toJSON @ datamodel.js:322
>> (anonymous function) @ node.js:850
>> jQuery.event.dispatch @ jquery-1.11.2.js:4665
>> elemData.handle @ jquery-1.11.2.js:4333
>>
>> On Thu, Apr 14, 2016 at 1:57 PM, Murtuza Zabuawala
>> <murtuza.zabuawala@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > PFA patch to fixed given issues.
>> >
>> >
>> > --
>> > Regards,
>> > Murtuza Zabuawala
>> > EnterpriseDB: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>> >
>> > On Thu, Apr 14, 2016 at 5:11 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Thanks - patch applied. Unfortunately there are 3 more minor issues
>> >> for you to fix:
>> >>
>> >> - Renaming or changing the schema for a shell type should not be
>> >> allowed.
>> >>
>> > Done
>> >>
>> >> - I'm allowed to try to add ACL entries or security labels to an
>> >> existing shell type. This should be disallowed.
>> >>
>> > Done
>> >>
>> >> - Changing the schema on a (non-shell) type doesn't work - the type
>> >> name is omitted, e.g.
>> >>
>> > Done
>> >>
>> >> ALTER TYPE pem
>> >>     SET SCHEMA pemhistory;
>> >>
>> >> Which should be:
>> >>
>> >> ALTER TYPE pem.foo
>> >>     SET SCHEMA pemhistory;
>> >>
>> >> On Thu, Apr 14, 2016 at 8:36 AM, Murtuza Zabuawala
>> >> <murtuza.zabuawala@enterprisedb.com> wrote:
>> >> > Hi Dave,
>> >> >
>> >> > PFA updated patch which will fix mentioned issues.
>> >> >
>> >> > Hopefully the last one :-)
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Murtuza Zabuawala
>> >> > EnterpriseDB: http://www.enterprisedb.com
>> >> > The Enterprise PostgreSQL Company
>> >> >
>> >> > On Wed, Apr 13, 2016 at 6:18 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >> >>
>> >> >> Hi
>> >> >>
>> >> >> I'm seeing some more issues now, some related to new functionality:
>> >> >>
>> >> >> - Please don't include the descriptive header for shell types in
>> >> >> create.sql.
>> >> >
>> >> > Done
>> >> >>
>> >> >>
>> >> >> - Shell types can (and should) have owner/comments set if specified.
>> >> >>
>> >> > Done
>> >> >>
>> >> >> - The Type dropdown should just say "Shell" for shell types (no
>> >> >> additional text).
>> >> >>
>> >> > Done
>> >> >>
>> >> >> - Privileges and labels grids should be disabled for shell types.
>> >> >>
>> >> > Done
>> >> >>
>> >> >> - When I select External type, I see the following error (and a 500
>> >> >> error in the browser console):
>> >> >>
>> >> >> 2016-04-13 13:44:49,953: ERROR pgadmin: Failed to execute query
>> >> >> (execute_2darray) for the server #1 - DB:pem (Query-id: 4020898):
>> >> >> Error Message:ERROR:  syntax error at or near "and"
>> >> >> LINE 2:     CASE WHEN length(nspname) > 0 AND and length(nspname)
>> >> >> !=...
>> >> >>
>> >> > Done
>> >> >>
>> >> >> - Length/Precision and scale are not shown in the dialogue for
>> >> >> existing
>> >> >> types.
>> >> >>
>> >> > Done (PFA screenshot)
>> >> >
>> >> >>
>> >> >> Thanks.
>> >> >>
>> >> >>
>> >> >> On Fri, Apr 8, 2016 at 2:52 PM, Murtuza Zabuawala
>> >> >> <murtuza.zabuawala@enterprisedb.com> wrote:
>> >> >> > Hi Dave,
>> >> >> >
>> >> >> > Please find updated patches to fix all the mentioned issues.
>> >> >> >
>> >> >> > There are three patches,
>> >> >> > 1) One for fixing rendering issue when selecting type
>> >> >> > 2) Added utility function to fetch full type name with length &
>> >> >> > precision
>> >> >> > 3) Added new Shell type for External types.
>> >> >> >
>> >> >> >
>> >> >> > Regards,
>> >> >> > Murtuza
>> >> >> >
>> >> >> >
>> >> >> > --
>> >> >> > Regards,
>> >> >> > Murtuza Zabuawala
>> >> >> > EnterpriseDB: http://www.enterprisedb.com
>> >> >> > The Enterprise PostgreSQL Company
>> >> >> >
>> >> >> > On Thu, Apr 7, 2016 at 5:06 PM, Dave Page <dpage@pgadmin.org>
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> Hi Murtuza
>> >> >> >>
>> >> >> >> On Tue, Mar 22, 2016 at 9:22 AM, Dave Page <dpage@pgadmin.org>
>> >> >> >> wrote:
>> >> >> >> > Hi
>> >> >> >> >
>> >> >> >> > On Tue, Mar 22, 2016 at 8:14 AM, Murtuza Zabuawala
>> >> >> >> > <murtuza.zabuawala@enterprisedb.com> wrote:
>> >> >> >> >> Hi Dave,
>> >> >> >> >>
>> >> >> >> >> We can create new external type using below method, By running
>> >> >> >> >> all
>> >> >> >> >> of
>> >> >> >> >> below
>> >> >> >> >> queries at the same time , we can not create separate external
>> >> >> >> >> type
>> >> >> >> >> by
>> >> >> >> >> only
>> >> >> >> >> using create type statement.
>> >> >> >> >>
>> >> >> >> >> So as per my discussion with Ashesh, We should not allow user
>> >> >> >> >> to
>> >> >> >> >> create
>> >> >> >> >> external type in pgAdmin4 but only show definition in edit
>> >> >> >> >> mode.
>> >> >> >> >
>> >> >> >> > Hmm, would it not make sense to allow the user to create the
>> >> >> >> > shell
>> >> >> >> > type as well (perhaps, with a new type of "SHELL")? Then they
>> >> >> >> > could
>> >> >> >> > do
>> >> >> >> > what is needed (and that should be easy, as it's just CREATE
>> >> >> >> > TYPE
>> >> >> >> > foo;)
>> >> >> >> >
>> >> >> >> > For example:
>> >> >> >> >
>> >> >> >> > CREATE TYPE box;
>> >> >> >> >
>> >> >> >> > CREATE FUNCTION my_box_in_function(cstring) RETURNS box AS ...
>> >> >> >> > ;
>> >> >> >> > CREATE FUNCTION my_box_out_function(box) RETURNS cstring AS ...
>> >> >> >> > ;
>> >> >> >> >
>> >> >> >> > CREATE TYPE box (
>> >> >> >> >     INTERNALLENGTH = 16,
>> >> >> >> >     INPUT = my_box_in_function,
>> >> >> >> >     OUTPUT = my_box_out_function
>> >> >> >> > );
>> >> >> >> >
>> >> >> >> > CREATE TABLE myboxes (
>> >> >> >> >     id integer,
>> >> >> >> >     description box
>> >> >> >> > );
>> >> >> >>
>> >> >> >> In the interests of making progress, I've committed the most
>> >> >> >> recent
>> >> >> >> patch, with a number of minor changes most significantly, the
>> >> >> >> Postgres
>> >> >> >> docs and system catalogs seem to have different ideas about what
>> >> >> >> to
>> >> >> >> call length, precision and scale. pgAdmin 3 followed the catalogs
>> >> >> >> and
>> >> >> >> used length and precision, however I've updated pgAdmin 4 to use
>> >> >> >> "Length/precision" and "Scale" which is inline with the Postgres
>> >> >> >> docs.
>> >> >> >> That's only in the UI though - the code follows the catalogs.
>> >> >> >>
>> >> >> >> There are still a couple of issues - please provide fixes ASAP:
>> >> >> >>
>> >> >> >> 1) If you create a composite type that contains a sized type
>> >> >> >> (e.g.
>> >> >> >> numeric(5, 4), the precision and scale are not shown if you later
>> >> >> >> open
>> >> >> >> the properties dialogue, or in the reverse engineered SQL.
>> >> >> >>
>> >> >> >> E.g. what pgAdmin3 shows as:
>> >> >> >>
>> >> >> >> CREATE TYPE pem.blergh AS
>> >> >> >>    (c1 text COLLATE pg_catalog."C",
>> >> >> >>     c2 numeric(5));
>> >> >> >>
>> >> >> >> Is shown by pgAdmin4 as:
>> >> >> >>
>> >> >> >> CREATE TYPE pem.blergh AS
>> >> >> >>     (c1 text COLLATE pg_catalog."C", c2 numeric);
>> >> >> >>
>> >> >> >> (adding the \n's would be good too).
>> >> >> >>
>> >> >> >> 2) If you select a different type of type in create mode, the new
>> >> >> >> options are shown below those for the previously selected type,
>> >> >> >> instead of replacing them. Please see the attached screenshot.
>> >> >> >>
>> >> >> >> 3) I would still like us to support External types. I believe the
>> >> >> >> simple option here is to re-add the code you had previously, and
>> >> >> >> to
>> >> >> >> add a new type of type called "SHELL" as discussed in my previous
>> >> >> >> email above. The user would then be able to create a SHELL type,
>> >> >> >> add
>> >> >> >> the required functions, then come back and create the EXTERNAL
>> >> >> >> type.
>> >> >> >>
>> >> >> >> I'll add cards to our internal kanban chart for these issues.
>> >> >> >>
>> >> >> >> 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
>> >> >
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> 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
>
>



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

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