public inbox for [email protected]
help / color / mirror / Atom feedFrom: Murtuza Zabuawala <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: PATCH: Added Node Type & Catalog objects [pgAdmin4]
Date: Thu, 14 Apr 2016 18:27:28 +0530
Message-ID: <CAKKotZSp29xieppuYKCDD--5UJ=ViPhrUvpToUQYHz3J86vN5g@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoyiqKrkQHqJv1jY-+RnFkmk5wySheeBd=Y3o875owEZeA@mail.gmail.com>
References: <CAKKotZSda6+wgyzckDJPXKK3dc8U-UXho9hmaaBp+LwZzdnhaQ@mail.gmail.com>
<CA+OCxoxNx3ghQZ+YnWeNpyVs_BaJE+yL7C6H-PUFh6ddpr061Q@mail.gmail.com>
<CA+OCxozEyaxygwg4KKNWKjKcLw=a-s6R64Tij9Z2JSVzzEKLrg@mail.gmail.com>
<CAKKotZTGn2aUnkrotK5u=9hh+nD4gUC9mvKWo2FcDVuEhH5oyA@mail.gmail.com>
<CA+OCxoyeSbbyKFm236agok7DNr4iRt16On9Kea+Z3tUj_iXa_g@mail.gmail.com>
<CAKKotZQVJPRkHKzhix+s9zc+QnwUfWfOj+sVFfk6c7abKuyECg@mail.gmail.com>
<CA+OCxowYVt2V-sr5YQm-YDxhmNiam-4iSudNSmydu_e2_T3eMA@mail.gmail.com>
<CAKKotZSrb=YrxbaasHQF9DXA-roFmzmA7cVC3tVPMdhYwL58cw@mail.gmail.com>
<CA+OCxoz_rEVOMa1nfJxH16-ZB6YXozm-DAAV8HOE5EORqAwQLQ@mail.gmail.com>
<CAKKotZTFTsZ2fHwMQem6v9prw8Un_qa8SqRmSanvU5eMXmTW8A@mail.gmail.com>
<CA+OCxowAevPwtNb1O6Shx1NndTjNJYPBd_ie-xnk4Hw4j-nKqA@mail.gmail.com>
<CA+OCxoxyCifnk7KT9ZZGBNYtH6gjCoJ7j8tUwSoBxzgAtCPZFA@mail.gmail.com>
<CAKKotZQ7tGDP8BR63pqYzpKi=8TfXZ02p+CMQzWvOqL3tZRZvA@mail.gmail.com>
<CA+OCxozkSRx_Wp1r5wvPuG9neAvgDJ1rBvTCsAEAravbBAhvsw@mail.gmail.com>
<CAKKotZSJP1nB18HBHkZTcRJhN0kioSmkhszT_d0x5vPN5LuBng@mail.gmail.com>
<CA+OCxoyiqKrkQHqJv1jY-+RnFkmk5wySheeBd=Y3o875owEZeA@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
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 <[email protected]> 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
> <[email protected]> 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 <[email protected]> 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
> >> <[email protected]> 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 <[email protected]> wrote:
> >> >>
> >> >> Hi Murtuza
> >> >>
> >> >> On Tue, Mar 22, 2016 at 9:22 AM, Dave Page <[email protected]>
> wrote:
> >> >> > Hi
> >> >> >
> >> >> > On Tue, Mar 22, 2016 at 8:14 AM, Murtuza Zabuawala
> >> >> > <[email protected]> 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
>
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] fixed_type_node_v7.patch (5.4K, 3-fixed_type_node_v7.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/js/type.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/js/type.js
index 7a41179..2d00877 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/js/type.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/js/type.js
@@ -282,7 +282,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
},{
id: 'schema', label:'{{ _('Schema') }}', cell: 'string',
type: 'text', mode: ['create', 'edit'], node: 'schema',
- disabled: 'inSchema', filter: function(d) {
+ disabled: 'schemaCheck', filter: function(d) {
// If schema name start with pg_* then we need to exclude them
if(d && d.label.match(/^pg_/))
{
@@ -290,18 +290,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
}
return true;
},
- control: Backform.NodeListByNameControl.extend({
- render: function(){
- // Initialize parent's render method
- Backform.NodeListByNameControl.prototype.render.apply(this, arguments);
-
- // Set schema default value to its parent Schema
- if(this.model.isNew()){
- this.model.set({'schema': this.model.node_info.schema.label});
- }
- return this;
- }
- })
+ control: 'node-list-by-name'
},{
id: 'typtype', label:'{{ _('Type') }}',
mode: ['create','edit'], disabled: 'inSchemaWithModelCheck',
@@ -314,7 +303,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
{label: "Enumeration", value: "e"},
{label: "External", value: "b"},
{label: "Range", value: "r"},
- {label: "Shell", value: "s"}
+ {label: "Shell", value: "p"}
]
},
disabled: 'inSchemaWithModelCheck',
@@ -724,7 +713,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
uniqueCol : ['grantee'], deps: ['typtype'],
canAdd: function(m) {
// Do not allow to add when shell type is selected
- return !(m.get('typtype') === 's');
+ return !(m.get('typtype') === 'p');
}
},{
id: 'seclabels', label: '{{ _('Security Labels') }}',
@@ -734,7 +723,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
control: 'unique-col-collection', deps: ['typtype'],
canAdd: function(m) {
// Do not allow to add when shell type is selected
- return !(m.get('typtype') === 's');
+ return !(m.get('typtype') === 'p');
}
}],
@@ -789,6 +778,17 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
}
return false;
},
+ schemaCheck: function(m) {
+ if(this.node_info && 'schema' in this.node_info)
+ {
+ if (m.isNew()) {
+ return false;
+ } else {
+ return m.get('typtype') === 'p';
+ }
+ }
+ return false;
+ },
// We will check if we are under schema node & in 'create' mode
inSchemaWithModelCheck: function(m) {
if(this.node_info && 'schema' in this.node_info)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/create.sql
index 35403b6..b4a44d4 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/create.sql
@@ -1,7 +1,7 @@
{% import 'macros/schemas/security.macros' as SECLABLE %}
{% import 'macros/schemas/privilege.macros' as PRIVILEGE %}
{## If user selected shell type then just create type template ##}
-{% if data and data.typtype == 's' %}
+{% if data and data.typtype == 'p' %}
CREATE TYPE {{ conn|qtIdent(data.schema, data.name) }};
{% endif %}
{### Composite Type ###}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/update.sql
index 170b03a..ab617b0 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/update.sql
@@ -131,9 +131,9 @@ ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
{# Below will change the schema for object #}
{# with extra if condition we will also make sure that object has correct name #}
{% if data.schema and data.schema != o_data.schema %}
-ALTER TYPE {% if data.name != o_data.name %}{{ conn|qtIdent(o_data.schema, data.name) }}
-{% else %}{{ conn|qtIdent(o_data.schema, o_data.name) }}{% endif %}
+ALTER TYPE {% if data.name and data.name != o_data.name %}{{ conn|qtIdent(o_data.schema, data.name) }}
+{% else %}{{ conn|qtIdent(o_data.schema, o_data.name) }}
+{% endif %}
SET SCHEMA {{ conn|qtIdent(data.schema) }};
-
{% endif %}
{% endif %}
\ No newline at end of file
view thread (26+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: PATCH: Added Node Type & Catalog objects [pgAdmin4]
In-Reply-To: <CAKKotZSp29xieppuYKCDD--5UJ=ViPhrUvpToUQYHz3J86vN5g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox