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: Fri, 8 Apr 2016 19:22:37 +0530
Message-ID: <CAKKotZQ7tGDP8BR63pqYzpKi=8TfXZ02p+CMQzWvOqL3tZRZvA@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxyCifnk7KT9ZZGBNYtH6gjCoJ7j8tUwSoBxzgAtCPZFA@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>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

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
>


-- 
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] added_fulltype_function_v1.patch (3.6K, 3-added_fulltype_function_v1.patch)
  download | inline diff:
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..1072467 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
@@ -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):
     """


  [application/octet-stream] Fixed_rendering_issue.patch (2.1K, 4-Fixed_rendering_issue.patch)
  download | inline diff:
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 8eaed6d..5c62b58 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -900,6 +900,7 @@
     render: function() {
       // Clean up existing elements
       this.undelegateEvents();
+      this.$el.empty();
 
       var field = _.defaults(this.field.toJSON(), this.defaults),
           attributes = this.model.toJSON(),
@@ -978,7 +979,8 @@
           gridSchema.columns.unshift({
             name: "pg-backform-delete", label: "",
             cell: Backgrid.Extension.DeleteCell,
-            editable: false, cell_priority: -1
+            editable: false, cell_priority: -1,
+            canDeleteRow: data.canDeleteRow
           });
       }
 
@@ -990,7 +992,7 @@
 
           gridSchema.columns.unshift({
             name: "pg-backform-edit", label: "", cell : editCell,
-            cell_priority: -2
+            cell_priority: -2, canEditRow: data.canEditRow
           });
       }
 
@@ -1200,7 +1202,8 @@
           gridSchema.columns.unshift({
             name: "pg-backform-delete", label: "",
             cell: Backgrid.Extension.DeleteCell,
-            editable: false, cell_priority: -1
+            editable: false, cell_priority: -1,
+            canDeleteRow: data.canDeleteRow
           });
       }
 
@@ -1214,7 +1217,8 @@
 
           gridSchema.columns.unshift({
             name: "pg-backform-edit", label: "", cell : editCell,
-            cell_priority: -2, editable: canEdit
+            cell_priority: -2, editable: canEdit,
+            canEditRow: data.canEditRow
           });
       }
 
@@ -1664,6 +1668,8 @@
             canAdd: (disabled ? false : evalASFunc(s.canAdd)),
             canEdit: (disabled ? false : evalASFunc(s.canEdit)),
             canDelete: (disabled ? false : evalASFunc(s.canDelete)),
+            canEditRow: (disabled ? false : evalASFunc(s.canEditRow)),
+            canDeleteRow: (disabled ? false : evalASFunc(s.canDeleteRow)),
             transform: evalASFunc(s.transform),
             mode: mode,
             control: control,


  [application/octet-stream] updated_type_node.patch (8.8K, 5-updated_type_node.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..7e05fae 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,21 @@ 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'])
+                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
 
+                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)
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 5d79449..1be3447 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
@@ -306,23 +306,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 (External type only)", 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({
@@ -343,10 +335,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') }}',
@@ -522,7 +511,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';
           },
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..4c4929e 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,19 @@
 {% 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' %}
+-- This is a shell type for creating 'External' type.
+-- In order to use, You need to create respective Input/Output function
+-- Once you created them, Use pgAdmin4 to create 'External' type.
+
+-- Refer http://www.postgresql.org/docs/[PG_VERSION]/static/sql-createtype.html
+
+CREATE TYPE {{ conn|qtIdent(data.schema, data.name) }};
+{% else %}
 {###  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' %}
@@ -77,3 +87,5 @@ COMMENT ON TYPE {% if data.schema %}{{ conn|qtIdent(data.schema, data.name) }}{%
 {% endif %}
 {% endfor %}
 {% endif %}
+{## End of shell type if ##}
+{% 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..148256e 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,8 +1,10 @@
 {### 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 and length(nspname) != 'public' and length(proname) > 0  THEN
         concat(quote_ident(nspname), '.', quote_ident(proname))
+    CASE 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


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: <CAKKotZQ7tGDP8BR63pqYzpKi=8TfXZ02p+CMQzWvOqL3tZRZvA@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