public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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 13:06:00 +0530
Message-ID: <CAKKotZSJP1nB18HBHkZTcRJhN0kioSmkhszT_d0x5vPN5LuBng@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozkSRx_Wp1r5wvPuG9neAvgDJ1rBvTCsAEAravbBAhvsw@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>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

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
>


-- 
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] updates_type_node_v6.patch (14.9K, 3-updates_type_node_v6.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
index 6a46bb0..8c6b157 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
@@ -16,7 +16,7 @@ from pgadmin.utils.ajax import make_json_response, \
     make_response as ajax_response, internal_server_error
 from pgadmin.browser.utils import PGChildNodeView
 from pgadmin.browser.server_groups.servers.databases.schemas.utils \
-    import SchemaChildModule
+    import SchemaChildModule, DataTypeReader
 import pgadmin.browser.server_groups.servers.databases as database
 from pgadmin.browser.server_groups.servers.utils import parse_priv_from_db, \
     parse_priv_to_db
@@ -87,7 +87,7 @@ class TypeModule(SchemaChildModule):
 blueprint = TypeModule(__name__)


-class TypeView(PGChildNodeView):
+class TypeView(PGChildNodeView, DataTypeReader):
     """
     This class is responsible for generating routes for Type node

@@ -335,7 +335,13 @@ class TypeView(PGChildNodeView):
             composite_lst = []

             for row in rset['rows']:
-                typelist = ' '.join([row['attname'], row['typname']])
+                # We will fetch Full type name
+                fulltype = self.get_full_type(
+                    row['collnspname'], row['typname'],
+                    row['isdup'], row['attndims'], row['atttypmod']
+                )
+
+                typelist = ' '.join([row['attname'], fulltype])
                 if not row['collname'] or (row['collname'] == 'default'
                                            and row['collnspname'] == 'pg_catalog'):
                     full_collate = ''
@@ -349,19 +355,28 @@ class TypeView(PGChildNodeView):

                 # Below logic will allow us to split length, precision from type name for grid
                 import re
-                matchObj = re.match( r'(.*)\((.*?),(.*?)\)', row['typname'])
+                # If we have length & precision both
+                matchObj = re.search(r'(\d+),(\d+)', fulltype)
                 if matchObj:
-                    t_name = matchObj.group(1)
-                    t_len = matchObj.group(2)
-                    t_prec = matchObj.group(3)
+                    t_len = matchObj.group(1)
+                    t_prec = matchObj.group(2)
                 else:
-                    t_name = row['typname']
-                    t_len = None
-                    t_prec = None
+                    # If we have length only
+                    matchObj = re.search(r'(\d+)', fulltype)
+                    if matchObj:
+                        t_len = matchObj.group(1)
+                        t_prec = None
+                    else:
+                        t_len = None
+                        t_prec = None
+
+                is_tlength = True if t_len else False
+                is_precision = True if t_prec else False

                 composite_lst.append({
-                    'attnum':row['attnum'], 'member_name': row['attname'], 'type': t_name, 'collation': full_collate,
-                    'tlength': t_len, 'precision': t_prec })
+                    'attnum':row['attnum'], 'member_name': row['attname'], 'type': row['typname'], 'collation': full_collate,
+                    'tlength': t_len, 'precision': t_prec,
+                    'is_tlength': is_tlength, 'is_precision': is_precision })

             # Adding both results
             res['member_list'] = ', '.join(properties_list)
@@ -442,7 +457,6 @@ class TypeView(PGChildNodeView):
         # it from properties sql
         copy_dict['typacl'] = []

-
         for row in acl['rows']:
             priv = parse_priv_from_db(row)
             if row['deftype'] in copy_dict:
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 571e0d5..e0182a8 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
@@ -308,23 +308,15 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
           group: '{{ _('Definition') }}',
           mode: ['edit', 'create'],
           select2: { width: "50%" },
-          options: function() {
-           if(!this.model.isNew()) {
+          options: function(obj) {
               return [
                 {label: "Composite", value: "c"},
                 {label: "Enumeration", value: "e"},
                 {label: "External", value: "b"},
                 {label: "Range", value: "r"},
+                {label: "Shell", value: "s"}
               ]
-            } else {
-              return [
-                {label: "Composite", value: "c"},
-                {label: "Enumeration", value: "e"},
-                {label: "Range", value: "r"},
-              ]
-
-           }
-          },
+           },
           disabled: 'inSchemaWithModelCheck',
           // If create mode then by default open composite type
           control: Backform.Select2Control.extend({
@@ -345,10 +337,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
           canAdd: true, canEdit: true, canDelete: true, disabled: 'inSchema',
           deps: ['typtype'], deps: ['typtype'],
           visible: function(m) {
-           if (m.get('typtype') === 'c') {
-             return true;
-           }
-           return false;
+           return m.get('typtype') === 'c';
           }
         },{
           id: 'enum', label: '{{ _('Enumeration Type') }}',
@@ -524,7 +513,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
         },{
           type: 'nested', control: 'tab', group: '{{ _('Definition') }}',
           label: '{{ _('External Type') }}', deps: ['typtype'],
-          mode: ['edit'],
+          mode: ['create', 'edit'],
           visible: function(m) {
             return m.get('typtype') === 'b';
           },
@@ -731,14 +720,23 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
           id: 'typacl', label: 'Privileges', type: 'collection',
           group: '{{ _('Security') }}', control: 'unique-col-collection',
           model: pgAdmin.Browser.Node.PrivilegeRoleModel.extend({privileges: ['U']}),
-          mode: ['edit', 'create'], canAdd: true, canDelete: true,
-          uniqueCol : ['grantee']
+          mode: ['edit', 'create'], canDelete: true,
+          uniqueCol : ['grantee'], deps: ['typtype'],
+          canAdd: function(m) {
+            // Do not allow to add when shell type is selected
+            return !(m.get('typtype') === 's');
+          }
         },{
           id: 'seclabels', label: '{{ _('Security Labels') }}',
           model: SecurityModel, editable: false, type: 'collection',
           group: '{{ _('Security') }}', mode: ['edit', 'create'],
-          min_version: 90100, canAdd: true,
-          canEdit: false, canDelete: true, control: 'unique-col-collection'
+          min_version: 90100, canEdit: false, canDelete: true,
+          control: 'unique-col-collection', deps: ['typtype'],
+          canAdd: function(m) {
+            // Do not allow to add when shell type is selected
+            return !(m.get('typtype') === 's');
+          }
+
         }],
         validate: function() {
         // Validation code for required fields
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 bd1f8b7..35403b6 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,9 +1,13 @@
 {% 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' %}
+CREATE TYPE {{ conn|qtIdent(data.schema, data.name) }};
+{% endif %}
 {###  Composite Type ###}
 {% if data and data.typtype == 'c' %}
 CREATE TYPE {% if data.schema %}{{ conn|qtIdent(data.schema, data.name) }}{% else %}{{ conn|qtIdent(data.name) }}{% endif %} AS
-    ({% if data.composite %}{% for d in data.composite %}{% if loop.index != 1 %}, {% endif %}{{ conn|qtIdent(d.member_name) }} {{ d.type }}{% if d.is_tlength and d.tlength %}({{d.tlength}}{% if d.is_precision and d.precision %},{{d.precision}}{% endif %}){% endif %}{% if d.collation %} COLLATE {{d.collation}}{% endif %}{% endfor %}{% endif %});
+({{"\n\t"}}{% if data.composite %}{% for d in data.composite %}{% if loop.index != 1 %},{{"\n\t"}}{% endif %}{{ conn|qtIdent(d.member_name) }} {{ d.type }}{% if d.is_tlength and d.tlength %}({{d.tlength}}{% if d.is_precision and d.precision %},{{d.precision}}{% endif %}){% endif %}{% if d.collation %} COLLATE {{d.collation}}{% endif %}{% endfor %}{% endif %}{{"\n"}});
 {% endif %}
 {###  Enum Type ###}
 {% if data and data.typtype == 'e' %}
@@ -76,4 +80,4 @@ COMMENT ON TYPE {% if data.schema %}{{ conn|qtIdent(data.schema, data.name) }}{%
 {{ SECLABLE.SET(conn, 'TYPE', data.name, r.provider, r.security_label, data.schema) }}
 {% endif %}
 {% endfor %}
-{% endif %}
+{% endif %}
\ No newline at end of file
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_external_functions.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_external_functions.sql
index 86648a2..809efbf 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_external_functions.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_external_functions.sql
@@ -1,16 +1,18 @@
 {### Input/Output/Send/Receive/Analyze function list also append into TypModeIN/TypModOUT ###}
 {% if extfunc %}
 SELECT proname, nspname,
-    CASE WHEN length(nspname) > 0 AND length(proname) > 0  THEN
+    CASE WHEN (length(nspname) > 0 AND nspname != 'public') and length(proname) > 0  THEN
         concat(quote_ident(nspname), '.', quote_ident(proname))
+    WHEN length(proname) > 0 THEN
+        quote_ident(proname)
     ELSE '' END AS func
 FROM (
     SELECT proname, nspname, max(proargtypes[0]) AS arg0, max(proargtypes[1]) AS arg1
 FROM pg_proc p
     JOIN pg_namespace n ON n.oid=pronamespace
 GROUP BY proname, nspname
-HAVING count(proname) = 1   ) AS uniquefunc
-WHERE arg0 <> 0 AND arg1 IS NULL;
+HAVING count(proname) = 1) AS uniquefunc
+WHERE arg0 <> 0 AND (arg1 IS NULL OR arg1 <> 0);
 {% endif %}
 {### TypmodIN list ###}
 {% if typemodin %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
index 129c19e..29c7936 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
@@ -13,6 +13,7 @@ from pgadmin.browser.collection import CollectionNodeModule
 from pgadmin.browser.utils import PGChildNodeView
 from flask import render_template
 from pgadmin.utils.ajax import internal_server_error
+import json

 class SchemaChildModule(CollectionNodeModule):
     """
@@ -141,6 +142,97 @@ class DataTypeReader:

         return True, res

+    def get_full_type(self, nsp, typname, isDup, numdims, typmod):
+        """
+        Returns full type name with Length and Precision.
+
+        Args:
+            conn: Connection Object
+            condition: condition to restrict SQL statement
+        """
+        needSchema = isDup
+        schema = nsp if nsp is not None else ''
+        name = ''
+        array = ''
+        length = ''
+
+        # Above 7.4, format_type also sends the schema name if it's not included
+        # in the search_path, so we need to skip it in the typname
+        if typname.find(schema + '".') >= 0:
+            name = typname[len(schema)+3]
+        elif typname.find(schema + '.') >= 0:
+            name = typname[len(schema)+1]
+        else:
+            name = typname
+
+        if name.startswith('_'):
+            if not numdims:
+                numdims = 1
+            name = name[1:]
+
+        if name.endswith('[]'):
+            if not numdims:
+                numdims = 1
+            name = name[:-2]
+
+        if name.startswith('"') and name.endswith('"'):
+            name = name[1:-1]
+
+        if numdims > 0:
+            while numdims:
+                array += '[]'
+                numdims -= 1
+
+        if typmod != -1:
+            length = '('
+            if name == 'numeric':
+                _len = (typmod - 4) >> 16;
+                _prec = (typmod - 4) & 0xffff;
+                length += str(_len)
+                if(_prec):
+                    length += ',' + str(_prec)
+            elif name == 'time' or \
+                    name == 'timetz' or \
+                    name == 'time without time zone' or \
+                    name == 'time with time zone' or \
+                    name == 'timestamp' or \
+                    name == 'timestamptz'or \
+                    name == 'timestamp without time zone' or \
+                    name == 'timestamp with time zone' or \
+                    name == 'bit' or \
+                    name == 'bit varying' or \
+                    name == 'varbit':
+                _prec = 0
+                _len = typmod
+                length += str(_len)
+            elif name == 'interval':
+                _prec = 0
+                _len = typmod & 0xffff
+                length += str(_len)
+            elif name == 'date':
+                # Clear length
+                length = ''
+            else:
+                _len = typmod - 4
+                _prec = 0
+                length += str(_len)
+
+            if len(length) > 0:
+                length += ')'
+
+        if name == 'char' and schema == 'pg_catalog':
+            return '"char"' + array
+        elif name == 'time with time zone':
+            return 'time' + length + ' with time zone' + array
+        elif name == 'time without time zone':
+            return 'time' + length + ' without time zone' + array
+        elif name == 'timestamp with time zone':
+            return 'timestamp' + length + ' with time zone' + array
+        elif name == 'timestamp without time zone':
+            return 'timestamp' + length + ' without time zone' + array
+        else:
+            return name + length + array
+

 def trigger_definition(data):
     """
@@ -246,4 +338,4 @@ def parse_rule_definition(res):
         res_data['condition'] = condition
     except Exception as e:
         return internal_server_error(errormsg=str(e))
-    return res_data
+    return res_data
\ No newline at end of file


  [image/png] Screen Shot 2016-04-14 at 1.03.43 pm.png (69.4K, 4-Screen%20Shot%202016-04-14%20at%201.03.43%20pm.png)
  download | view image

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: <CAKKotZSJP1nB18HBHkZTcRJhN0kioSmkhszT_d0x5vPN5LuBng@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