Subject: [pgAdmin4][PATCH] To fix the issue in handling of timestamp type

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 30 Jan 2017 11:07:37 +0530

Hi,

Please find a patch to fix the issue in timestamp & time type when user
tries to provide scale/lengt to them.
I did some refactoring in code to handle it at one place, affected nodes
would be Type, Table & Column.
*RM#2076*

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Dave Page <[email-masked]>
Date: Mon, 30 Jan 2017 14:37:13 +0000

Can you rebase this please?

Thanks.

On Mon, Jan 30, 2017 at 5:37 AM, Murtuza Zabuawala
<[email-masked]> wrote:
> RM#2076

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

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

---

From: Murtuza Zabuawala <[email-masked]>
Date: Tue, 31 Jan 2017 10:49:19 +0530

Hi Dave,

PFA updated patch.

---

From: Dave Page <[email-masked]>
Date: Wed, 1 Feb 2017 09:38:19 +0000

Hi

> PFA updated patch.

This seems to display "timestamp(0) with[out] timezone" columns
correctly in both the properties panel and dialog now, but the size is
still ignored if I try to add a new column through the table or column
dialogue.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Fri, 3 Feb 2017 17:16:51 +0530

Hi,

Please find updates patch for the same.
RM#2076

> still ignored if I try to add a new column through the table or column
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
index 9292989..646f12a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
@@ -654,13 +654,13 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                 # If we have length & precision both
                 matchObj = re.search(r'(\d+),(\d+)', fulltype)
                 if matchObj:
-                    column['attlen'] = int(matchObj.group(1))
-                    column['attprecision'] = int(matchObj.group(2))
+                    column['attlen'] = matchObj.group(1)
+                    column['attprecision'] = matchObj.group(2)
                 else:
                     # If we have length only
                     matchObj = re.search(r'(\d+)', fulltype)
                     if matchObj:
-                        column['attlen'] = int(matchObj.group(1))
+                        column['attlen'] = matchObj.group(1)
                         column['attprecision'] = None
                     else:
                         column['attlen'] = None
@@ -694,21 +694,7 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                     edit_types_list.append(present_type)
 
                 column['edit_types'] = edit_types_list
-
-                # Manual Data type formatting
-                # If data type has () with them then we need to remove them
-                # eg bit(1) because we need to match the name with combobox
-                isArray = False
-                if column['cltype'].endswith('[]'):
-                    isArray = True
-                    column['cltype'] = column['cltype'].rstrip('[]')
-
-                idx = column['cltype'].find('(')
-                if idx and column['cltype'].endswith(')'):
-                    column['cltype'] = column['cltype'][:idx]
-
-                if isArray:
-                    column['cltype'] += "[]"
+                column['cltype'] = DataTypeReader.parse_type_name(column['cltype'])
 
                 if 'indkey' in column:
                     # Current column
@@ -1316,6 +1302,24 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
         else:
             return data_type, False
 
+    @staticmethod
+    def convert_length_precision_to_string(data):
+        """
+        This function is used to convert length & precision to string
+        to handle case like when user gives 0 as length
+
+        Args:
+            data: Data from client
+
+        Returns:
+            Converted data
+        """
+        if 'attlen' in data and data['attlen'] is not None:
+            data['attlen'] = str(data['attlen'])
+        if 'attprecision' in data and data['attprecision'] is not None:
+            data['attprecision'] = str(data['attprecision'])
+        return data
+
     def _parse_format_columns(self, data, mode=None):
         """
         data:
@@ -1343,6 +1347,8 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                             # check type for '[]' in it
                             c['cltype'], c['hasSqrBracket'] = self._cltype_formatter(c['cltype'])
 
+                        c = self.convert_length_precision_to_string(c)
+
                     data['columns'][action] = final_columns
         else:
             # We need to exclude all the columns which are inherited from other tables
@@ -1363,6 +1369,8 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                     # check type for '[]' in it
                     c['cltype'], c['hasSqrBracket'] = self._cltype_formatter(c['cltype'])
 
+                c = self.convert_length_precision_to_string(c)
+
             data['columns'] = final_columns
 
         return data
@@ -2199,6 +2207,7 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                         old_data = res['rows'][0]
 
                         old_data['cltype'], old_data['hasSqrBracket'] = self._cltype_formatter(old_data['cltype'])
+                        old_data = self.convert_length_precision_to_string(old_data)
 
                         fulltype = self.get_full_type(
                             old_data['typnspname'], old_data['typname'],
@@ -2220,20 +2229,7 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                                 old_data['attlen'] = None
                                 old_data['attprecision'] = None
 
-                        # Manual Data type formatting
-                        # If data type has () with them then we need to remove them
-                        # eg bit(1) because we need to match the name with combobox
-                        isArray = False
-                        if old_data['cltype'].endswith('[]'):
-                            isArray = True
-                            old_data['cltype'] = old_data['cltype'].rstrip('[]')
-
-                        idx = old_data['cltype'].find('(')
-                        if idx and old_data['cltype'].endswith(')'):
-                            old_data['cltype'] = old_data['cltype'][:idx]
-
-                        if isArray:
-                            old_data['cltype'] += "[]"
+                        old_data['cltype'] = DataTypeReader.parse_type_name(old_data['cltype'])
 
                         # Sql for alter column
                         if 'inheritedfrom' not in c:
@@ -2250,6 +2246,9 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
                         if 'attacl' in c:
                             c['attacl'] = parse_priv_to_db(c['attacl'],
                                                            self.column_acl)
+
+                        c = self.convert_length_precision_to_string(c)
+
                         if 'inheritedfrom' not in c:
                             column_sql += render_template("/".join(
                                 [self.column_template_path, 'create.sql']),
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py
index 8d72d15..ac9a103 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py
@@ -436,20 +436,7 @@ class ColumnsView(PGChildNodeView, DataTypeReader):
 
         data['edit_types'] = edit_types_list
 
-        # Manual Data type formatting
-        # If data type has () with them then we need to remove them
-        # eg bit(1) because we need to match the name with combobox
-        isArray = False
-        if data['cltype'].endswith('[]'):
-            isArray = True
-            data['cltype'] = data['cltype'].rstrip('[]')
-
-        idx = data['cltype'].find('(')
-        if idx and data['cltype'].endswith(')'):
-            data['cltype'] = data['cltype'][:idx]
-
-        if isArray:
-            data['cltype'] += "[]"
+        data['cltype'] = DataTypeReader.parse_type_name(data['cltype'])
 
         return data
 
@@ -511,6 +498,24 @@ class ColumnsView(PGChildNodeView, DataTypeReader):
 
         return type
 
+    @staticmethod
+    def convert_length_precision_to_string(data):
+        """
+        This function is used to convert length & precision to string
+        to handle case like when user gives 0 as length
+
+        Args:
+            data: Data from client
+
+        Returns:
+            Converted data
+        """
+        if 'attlen' in data and data['attlen'] is not None:
+            data['attlen'] = str(data['attlen'])
+        if 'attprecision' in data and data['attprecision'] is not None:
+            data['attprecision'] = str(data['attprecision'])
+        return data
+
     @check_precondition
     def create(self, gid, sid, did, scid, tid):
         """
@@ -560,6 +565,7 @@ class ColumnsView(PGChildNodeView, DataTypeReader):
         # check type for '[]' in it
         data['cltype'] = self._cltype_formatter(data['cltype'])
         data['hasSqrBracket'] = self.hasSqrBracket
+        data = self.convert_length_precision_to_string(data)
 
         SQL = render_template("/".join([self.template_path,
                                         'create.sql']),
@@ -733,6 +739,8 @@ class ColumnsView(PGChildNodeView, DataTypeReader):
         """
         This function will genrate sql from model data
         """
+        data = self.convert_length_precision_to_string(data)
+
         if clid is not None:
             SQL = render_template("/".join([self.template_path,
                                             'properties.sql']), tid=tid, clid=clid
@@ -746,6 +754,11 @@ class ColumnsView(PGChildNodeView, DataTypeReader):
             # We will add table & schema as well
             old_data = self._formatter(scid, tid, clid, old_data)
 
+            # check type for '[]' in it
+            if 'cltype' in old_data:
+                old_data['cltype'] = self._cltype_formatter(old_data['cltype'])
+                old_data['hasSqrBracket'] = self.hasSqrBracket
+
             # If name is not present in data then
             # we will fetch it from old data, we also need schema & table name
             if 'name' not in data:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/update.sql
index 142f6ae..fcfd3c1 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/update.sql
@@ -1,6 +1,7 @@
 {% import 'column/macros/security.macros' as SECLABEL %}
 {% import 'column/macros/privilege.macros' as PRIVILEGE %}
 {% import 'macros/variable.macros' as VARIABLE %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {###  Rename column name ###}
 {% if data.name and data.name != o_data.name %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
@@ -8,11 +9,9 @@ ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
 
 {% endif %}
 {###  Alter column type and collation ###}
-{% if (data.cltype and data.cltype != o_data.cltype) or (data.attlen and data.attlen != o_data.attlen) or (data.attprecision or data.attprecision != o_data.attprecision) %}
+{% if (data.cltype and data.cltype != o_data.cltype) or (data.attlen and data.attlen != o_data.attlen) or (data.attprecision and data.attprecision != o_data.attprecision) %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
-    ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {% if data.cltype %}{{conn|qtTypeIdent(data.cltype)}} {% elif o_data.typnspname != 'pg_catalog' %}{{conn|qtTypeIdent(o_data.typnspname, o_data.cltype)}}{% else %}{{conn|qtTypeIdent(o_data.cltype)}} {% endif %}
-{% if data.attlen and data.attlen != 'None' %}({{data.attlen}}{% if data.attlen != 'None' and data.attprecision %}, {{data.attprecision}}){% elif (data.cltype is defined and not data.cltype) %}, {{o_data.attprecision}}){% elif data.attlen %}){% endif %}{% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}{% if data.collspcname and data.collspcname != o_data.collspcname %}
+    ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {{ GET_TYPE.UPDATE_TYPE_SQL(conn, data, o_data) }}{% if data.collspcname and data.collspcname != o_data.collspcname %}
  COLLATE {{data.collspcname}}{% endif %};
 {% endif %}
 {###  Alter column default value ###}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/create.sql
index 8f0e754..0fce446 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/create.sql
@@ -1,12 +1,11 @@
 {% import 'column/macros/security.macros' as SECLABEL %}
 {% import 'column/macros/privilege.macros' as PRIVILEGE %}
 {% import 'macros/variable.macros' as VARIABLE %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {###  Add column ###}
 {% if data.name and  data.cltype %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
-    ADD COLUMN {{conn|qtIdent(data.name)}} {{conn|qtTypeIdent(data.cltype)}}{% if data.attlen %}
-({{data.attlen}}{% if data.attprecision%}, {{data.attprecision}}{% endif %}){% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}{% if data.collspcname %}
+    ADD COLUMN {{conn|qtIdent(data.name)}} {{ GET_TYPE.CREATE_TYPE_SQL(conn, data.cltype, data.attlen, data.attprecision, data.hasSqrBracket) }}{% if data.collspcname %}
  COLLATE {{data.collspcname}}{% endif %}{% if data.attnotnull %}
  NOT NULL{% endif %}{% if data.defval %}
  DEFAULT {{data.defval}}{% endif %};
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/update.sql
index 3e71780..b045131 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/update.sql
@@ -1,6 +1,7 @@
 {% import 'column/macros/security.macros' as SECLABEL %}
 {% import 'column/macros/privilege.macros' as PRIVILEGE %}
 {% import 'macros/variable.macros' as VARIABLE %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {###  Rename column name ###}
 {% if data.name and data.name != o_data.name %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
@@ -8,11 +9,9 @@ ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
 
 {% endif %}
 {###  Alter column type and collation ###}
-{% if (data.cltype and data.cltype != o_data.cltype) or (data.attlen and data.attlen != o_data.attlen) or (data.attprecision or data.attprecision != o_data.attprecision) %}
+{% if (data.cltype and data.cltype != o_data.cltype) or (data.attlen and data.attlen != o_data.attlen) or (data.attprecision and data.attprecision != o_data.attprecision) %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.table)}}
-    ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {% if data.cltype %}{{conn|qtTypeIdent(data.cltype)}} {% elif o_data.typnspname != 'pg_catalog' %}{{conn|qtTypeIdent(o_data.typnspname, o_data.cltype)}}{% else %}{{conn|qtTypeIdent(o_data.cltype)}} {% endif %}
-{% if data.attlen and data.attlen != 'None' %}({{data.attlen}}{% if data.attlen != 'None' and data.attprecision %}, {{data.attprecision}}){% elif (data.cltype is defined and not data.cltype) %}, {{o_data.attprecision}}){% elif data.attlen %}){% endif %}{% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}{% if data.collspcname and data.collspcname != o_data.collspcname %}
+    ALTER COLUMN {% if data.name %}{{conn|qtTypeIdent(data.name)}}{% else %}{{conn|qtTypeIdent(o_data.name)}}{% endif %} TYPE {{ GET_TYPE.UPDATE_TYPE_SQL(conn, data, o_data) }}{% if data.collspcname and data.collspcname != o_data.collspcname %}
  COLLATE {{data.collspcname}}{% endif %};
 {% endif %}
 {###  Alter column default value ###}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/create.sql
index ba91ac5..e326d5f 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/create.sql
@@ -4,6 +4,7 @@
 {% import 'column/macros/security.macros' as COLUMN_SECLABEL %}
 {% import 'column/macros/privilege.macros' as COLUMN_PRIVILEGE %}
 {% import 'table/sql/macros/constraints.macro' as CONSTRAINTS %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {#===========================================#}
 {#====== MAIN TABLE TEMPLATE STARTS HERE ======#}
 {#===========================================#}
@@ -43,9 +44,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE {{conn|qtIdent(data
 {% if c.name and c.cltype %}
 {% if loop.index != 1 %},
 {% endif %}
-    {{conn|qtIdent(c.name)}} {{conn|qtTypeIdent(c.cltype)}}{% if c.attlen %}
-({{c.attlen}}{% if c.attprecision%}, {{c.attprecision}}{% endif %}){% endif %}{% if c.hasSqrBracket %}
-[]{% endif %}{% if c.collspcname %} COLLATE {{c.collspcname}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval %} DEFAULT {{c.defval}}{% endif %}
+    {{conn|qtIdent(c.name)}} {{ GET_TYPE.CREATE_TYPE_SQL(conn, c.cltype, c.attlen, c.attprecision, c.hasSqrBracket) }}{% if c.collspcname %} COLLATE {{c.collspcname}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval %} DEFAULT {{c.defval}}{% endif %}
 {% endif %}
 {% endfor %}
 {% endif %}
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 0bc8253..9be270d 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
@@ -347,6 +347,43 @@ class TypeView(PGChildNodeView, DataTypeReader):
             status=200
         )
 
+    def _cltype_formatter(self, type):
+        """
+
+        Args:
+            data: Type string
+
+        Returns:
+            We need to remove [] from type and append it
+            after length/precision so we will set flag for
+            sql template
+        """
+        if '[]' in type:
+            type = type.replace('[]', '')
+            self.hasSqrBracket = True
+        else:
+            self.hasSqrBracket = False
+
+        return type
+
+    @staticmethod
+    def convert_length_precision_to_string(data):
+        """
+        This function is used to convert length & precision to string
+        to handle case like when user gives 0 as length
+
+        Args:
+            data: Data from client
+
+        Returns:
+            Converted data
+        """
+        if 'tlength' in data and data['tlength'] is not None:
+            data['tlength'] = str(data['tlength'])
+        if 'precision' in data and data['precision'] is not None:
+            data['precision'] = str(data['precision'])
+        return data
+
     def additional_properties(self, copy_dict, tid):
         """
         We will use this function to add additional properties according to type
@@ -412,11 +449,17 @@ class TypeView(PGChildNodeView, DataTypeReader):
                 is_tlength = True if t_len else False
                 is_precision = True if t_prec else False
 
+                type_name = DataTypeReader.parse_type_name(row['typname'])
+
+                row['type'] = self._cltype_formatter(type_name)
+                row['hasSqrBracket'] = self.hasSqrBracket
+                row = self.convert_length_precision_to_string(row)
                 composite_lst.append({
-                    'attnum': row['attnum'], 'member_name': row['attname'], 'type': row['typname'],
-                    'collation': full_collate,
+                    'attnum': row['attnum'], 'member_name': row['attname'], 'type': type_name,
+                    'collation': full_collate, 'cltype': row['type'],
                     'tlength': t_len, 'precision': t_prec,
-                    'is_tlength': is_tlength, 'is_precision': is_precision})
+                    'is_tlength': is_tlength, 'is_precision': is_precision,
+                    'hasSqrBracket': row['hasSqrBracket']})
 
             # Adding both results
             res['member_list'] = ', '.join(properties_list)
@@ -900,6 +943,12 @@ class TypeView(PGChildNodeView, DataTypeReader):
         data = self._convert_for_sql(data)
 
         try:
+            if 'composite' in data and len(data['composite']) > 0:
+                for each_type in data['composite']:
+                    each_type = self.convert_length_precision_to_string(each_type)
+                    each_type['cltype'] = self._cltype_formatter(each_type['type'])
+                    each_type['hasSqrBracket'] = self.hasSqrBracket
+
             SQL = render_template("/".join([self.template_path, 'create.sql']),
                                   data=data, conn=self.conn)
             status, res = self.conn.execute_dict(SQL)
@@ -1118,6 +1167,15 @@ class TypeView(PGChildNodeView, DataTypeReader):
                     if 'deleted' in data[key]:
                         data[key]['deleted'] = parse_priv_to_db(data[key]['deleted'], self.acl)
 
+            if 'composite' in data and len(data['composite']) > 0:
+                for key in ['added', 'changed', 'deleted']:
+                    if key in data['composite']:
+                        for each_type in data['composite'][key]:
+                            each_type = self.convert_length_precision_to_string(each_type)
+                            if 'type' in each_type:
+                                each_type['cltype'] = self._cltype_formatter(each_type['type'])
+                                each_type['hasSqrBracket'] = self.hasSqrBracket
+
             SQL = render_template("/".join([self.template_path,
                                             'properties.sql']),
                                   scid=scid, tid=tid,
@@ -1169,7 +1227,15 @@ class TypeView(PGChildNodeView, DataTypeReader):
             # Privileges
             if 'typacl' in data and data['typacl'] is not None:
                 data['typacl'] = parse_priv_to_db(data['typacl'], self.acl)
+
             data = self._convert_for_sql(data)
+
+            if 'composite' in data and len(data['composite']) > 0:
+                for each_type in data['composite']:
+                    each_type = self.convert_length_precision_to_string(each_type)
+                    each_type['cltype'] = self._cltype_formatter(each_type['type'])
+                    each_type['hasSqrBracket'] = self.hasSqrBracket
+
             SQL = render_template("/".join([self.template_path,
                                             'create.sql']),
                                   data=data, conn=self.conn)
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 965826e..94f3139 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
@@ -113,6 +113,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
                   m.set('is_tlength', true, {silent: true});
                   m.set('min_val', o.min_val, {silent: true});
                   m.set('max_val', o.max_val, {silent: true});
+                } else {
+                  // set the values in model
+                  m.set('is_tlength', false, {silent: true});
                 }
             }
           });
@@ -140,6 +143,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
                   m.set('is_precision', true, {silent: true});
                   m.set('min_val', o.min_val, {silent: true});
                   m.set('max_val', o.max_val, {silent: true});
+                } else {
+                  // set the values in model
+                  m.set('is_precision', false, {silent: true});
                 }
             }
           });
@@ -166,7 +172,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backgrid) {
 
          if (flag) {
            setTimeout(function(){
-             m.set('collspcname', "");
+             m.set('collspcname', "", {silent: true});
            }, 10);
          }
          return flag;
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/macros/get_full_type_sql_format.macros b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/macros/get_full_type_sql_format.macros
index ce70aad..4b9c0e9 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/macros/get_full_type_sql_format.macros
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/macros/get_full_type_sql_format.macros
@@ -1,13 +1,17 @@
 {% macro CREATE_TYPE_SQL(conn, type_name, type_length, type_precision, is_type_array) %}
 {% if type_name.startswith('time') and type_length %}
+{#############################################################}
 {###### Need to check against separate time types - START ######}
+{#############################################################}
 {% if type_name == "timestamp without time zone" %}
 timestamp({{ type_length }}) without time zone{% elif type_name == "timestamp with time zone" %}
 timestamp({{ type_length }}) with time zone{% elif type_name == "time without time zone" %}
 time({{ type_length }}) without time zone{% elif type_name == "time with time zone" %}
 time({{ type_length }}) with time zone{% endif %}{% if is_type_array %}
 []{% endif %}
+{#############################################################}
 {###### Need to check against separate time types - END ######}
+{#############################################################}
 {% else %}
 {{ conn|qtTypeIdent(type_name) }}{% if type_length %}
 ({{ type_length }}{% if type_precision%}, {{ type_precision }}{% endif %}){% endif %}{% if is_type_array %}
@@ -19,45 +23,41 @@ time({{ type_length }}) with time zone{% endif %}{% if is_type_array %}
 {######################################################}
 {% macro UPDATE_TYPE_SQL(conn, data, o_data) %}
 {% if data.cltype and data.cltype.startswith('time') and data.attlen %}
+{#############################################################}
 {###### Need to check against separate time types - START ######}
+{#############################################################}
 {% if data.cltype == "timestamp without time zone" %}
-timestamp({{ data.attlen }}) without time zone
-{% elif data.cltype == "timestamp with time zone" %}
-timestamp({{ data.attlen }}) with time zone
-{% elif data.cltype == "time without time zone" %}
-time({{ data.attlen }}) without time zone
-{% elif data.cltype == "time with time zone" %}
-time({{ data.attlen }}) with time zone
-{% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}
+timestamp({{ data.attlen }}) without time zone {% elif data.cltype == "timestamp with time zone" %}
+timestamp({{ data.attlen }}) with time zone {% elif data.cltype == "time without time zone" %}
+time({{ data.attlen }}) without time zone {% elif data.cltype == "time with time zone" %}
+time({{ data.attlen }}) with time zone {% endif %}{% if data.hasSqrBracket %}[]{% endif %}
+{#############################################################}
 {# if only type changes, we need to give previous length to current type#}
+{#############################################################}
 {% elif data.cltype and data.cltype.startswith('time') and o_data.attlen != 'None' %}
 {% if data.cltype == "timestamp without time zone" %}
-timestamp({{ o_data.attlen }}) without time zone
-{% elif data.cltype == "timestamp with time zone" %}
-timestamp({{ o_data.attlen }}) with time zone
-{% elif data.cltype == "time without time zone" %}
-time({{ o_data.attlen }}) without time zone
-{% elif data.cltype == "time with time zone" %}
-time({{ o_data.attlen }}) with time zone
-{% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}
+timestamp({{ o_data.attlen }}) without time zone {% elif data.cltype == "timestamp with time zone" %}
+timestamp({{ o_data.attlen }}) with time zone {% elif data.cltype == "time without time zone" %}
+time({{ o_data.attlen }}) without time zone {% elif data.cltype == "time with time zone" %}
+time({{ o_data.attlen }}) with time zone {% endif %}{% if data.hasSqrBracket %}[]{% endif %}
+{#############################################################}
 {# if only length changes, we need to give previous length to current type#}
+{#############################################################}
 {% elif data.attlen and o_data.cltype.startswith('time') %}
 {% if o_data.cltype == "timestamp without time zone" %}
-timestamp({{ data.attlen }}) without time zone
-{% elif o_data.cltype == "timestamp with time zone" %}
-timestamp({{ data.attlen }}) with time zone
-{% elif o_data.cltype == "time without time zone" %}
-time({{ data.attlen }}) without time zone
-{% elif o_data.cltype == "time with time zone" %}
-time({{ data.attlen }}) with time zone
-{% endif %}{% if o_data.hasSqrBracket %}
-[]{% endif %}
+timestamp({{ data.attlen }}) without time zone {% elif o_data.cltype == "timestamp with time zone" %}
+timestamp({{ data.attlen }}) with time zone {% elif o_data.cltype == "time without time zone" %}
+time({{ data.attlen }}) without time zone {% elif o_data.cltype == "time with time zone" %}
+time({{ data.attlen }}) with time zone {% endif %}{% if o_data.hasSqrBracket %}[]{% endif %}
 {###### Need to check against separate time types - END ######}
-{% else %}
-{% if data.cltype %}{{conn|qtTypeIdent(data.cltype)}} {% elif o_data.typnspname != 'pg_catalog' %}{{conn|qtTypeIdent(o_data.typnspname, o_data.cltype)}}{% else %}{{conn|qtTypeIdent(o_data.cltype)}} {% endif %}
-{% if data.attlen and data.attlen != 'None' %}({{data.attlen}}{% if data.attlen != 'None' and data.attprecision %}, {{data.attprecision}}){% elif (data.cltype is defined and not data.cltype) %}, {{o_data.attprecision}}){% elif data.attlen %}){% endif %}{% endif %}{% if data.hasSqrBracket %}
-[]{% endif %}
+{% elif (data.cltype and not data.cltype.startswith('time')) or not o_data.cltype.startswith('time') %}
+{#############################################################}
+{########## We will create SQL for other types here ##########}
+{#############################################################}
+{% if data.cltype %}{{conn|qtTypeIdent(data.cltype)}} {% elif o_data.typnspname != 'pg_catalog' %}{{conn|qtTypeIdent(o_data.typnspname, o_data.cltype)}}{% else %}{{conn|qtTypeIdent(o_data.cltype)}} {% endif %}{% if (data.attlen and data.attlen != 'None') or (data.attprecision and data.attprecision != 'None') %}
+{% if data.attlen and data.attlen != 'None' %}
+({{ data.attlen }}{% elif data.attprecision and data.attprecision != 'None' %}({{ o_data.attlen }}{% endif %}{% if data.attprecision and data.attprecision != 'None' %}
+, {{ data.attprecision }}){% elif o_data.attprecision and o_data.attprecision != 'None' %}, {{ o_data.attprecision }}){% else %}){% endif %}
+{% endif %}
 {% endif %}
 {% endmacro %}
\ No newline at end of file
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/create.sql
index 53bd9f6..7598294 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/create.sql
@@ -1,5 +1,6 @@
 {% import 'macros/schemas/security.macros' as SECLABEL %}
 {% import 'macros/schemas/privilege.macros' as PRIVILEGE %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {## If user selected shell type then just create type template ##}
 {% if data and data.typtype == 'p' %}
 CREATE TYPE {{ conn|qtIdent(data.schema, data.name) }};
@@ -7,7 +8,7 @@ CREATE TYPE {{ conn|qtIdent(data.schema, data.name) }};
 {###  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
-({{"\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"}});
+({{"\n\t"}}{% if data.composite %}{% for d in data.composite %}{% if loop.index != 1 %},{{"\n\t"}}{% endif %}{{ conn|qtIdent(d.member_name) }} {{ GET_TYPE.CREATE_TYPE_SQL(conn, d.cltype, d.tlength, d.precision, d.hasSqrBracket) }}{% if d.collation %} COLLATE {{d.collation}}{% endif %}{% endfor %}{% endif %}{{"\n"}});
 {% endif %}
 {###  Enum Type ###}
 {% if data and data.typtype == 'e' %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/update.sql
index 3af2ea1..7f38e97 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/default/update.sql
@@ -1,6 +1,6 @@
 {% import 'macros/schemas/security.macros' as SECLABEL %}
 {% import 'macros/schemas/privilege.macros' as PRIVILEGE %}
-{% if data %}
+{% import 'type/macros/get_full_type_sql_format.macros' as GET_TYPE %}
 {#======================================#}
 {# Below will change object owner #}
 {% if data.typeowner and data.typeowner != o_data.typeowner %}
@@ -28,29 +28,48 @@ ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
 {% if 'added' in composite and composite.added|length > 0 %}
 {% for r in composite.added %}
 ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
-    ADD ATTRIBUTE {{conn|qtIdent(r.member_name)}} {{conn|qtTypeIdent(r.type)}}{% if r.is_tlength and r.tlength %}
-({{r.tlength}}{% if r.is_precision and r.precision %},{{r.precision}}{% endif %}){% endif %}{% if r.collation %}
+    ADD ATTRIBUTE {{conn|qtIdent(r.member_name)}} {{ GET_TYPE.CREATE_TYPE_SQL(conn, r.cltype, r.tlength, r.precision, r.hasSqrBracket) }}{% if r.collation %}
  COLLATE {{r.collation}}{% endif %};
 {% endfor %}
 {% endif %}
 {% if 'changed' in composite and composite.changed|length > 0 %}
 {% for r in composite.changed %}
 {% for o in o_data.composite %}
-{% if o.attnum == r.attnum and r.member_name and o.member_name != r.member_name %}
+{##### Variables for the loop #####}
+{% set member_name = o.member_name %}
+{% set cltype = o.cltype %}
+{% set tlength = o.tlength %}
+{% set precision = o.precision %}
+{% set hasSqrBracket = o.hasSqrBracket %}
+{##### If member name changed #####}
+{% if o.attnum == r.attnum %}
+{% if r.member_name and o.member_name != r.member_name %}
 ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
     RENAME ATTRIBUTE {{o.member_name}} TO {{r.member_name}};
-{% if r.type and o.type != r.type %}
-ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
-        ALTER ATTRIBUTE {{conn|qtIdent(r.member_name)}} SET DATA TYPE {{conn|qtTypeIdent(r.type)}}{% if r.is_tlength and r.tlength %}
-({{r.tlength}}{% if r.is_precision and r.precision %},{{r.precision}}{% endif %}){% endif %}{% if r.collation %}
- COLLATE {{r.collation}}{% endif %};
-{% else %}
+{% set member_name = r.member_name %}
+{% endif %}
+{##### If type changed #####}
+{% if r.cltype and cltype != r.cltype %}
+{% set cltype = r.cltype %}
+{% set hasSqrBracket = r.hasSqrBracket %}
+{##### If length is not allowed on type #####}
+{% if not r.is_tlength %}
+{% set tlength = 0 %}
+{% set precision = 0 %}
+{% endif %}
+{% endif %}
+{##### If length changed #####}
+{% if r.tlength and tlength != r.tlength %}
+{% set tlength = r.tlength %}
+{% endif %}
+{##### If precision changed #####}
+{% if tlength and r.precision and precision != r.precision %}
+{% set precision = r.precision %}
+{% endif %}
 ALTER TYPE {{ conn|qtIdent(o_data.schema, o_data.name) }}
-        ALTER ATTRIBUTE {{conn|qtIdent(r.member_name)}} SET DATA TYPE {{conn|qtTypeIdent(o.type)}}{% if o.is_tlength and o.tlength %}
-({{o.tlength}}{% if o.is_precision and o.precision %},{{o.precision}}{% endif %}){% endif %}{% if o.collation %}
+        ALTER ATTRIBUTE {{conn|qtIdent(member_name)}} SET DATA TYPE {{ GET_TYPE.CREATE_TYPE_SQL(conn, cltype, tlength, precision, hasSqrBracket) }}{% if r.collation %}
  COLLATE {{r.collation}}{% endif %};
 {% endif%}
-{% endif%}
 {% endfor %}
 {% endfor %}
 {% endif %}
@@ -135,5 +154,4 @@ ALTER TYPE {% if data.name and data.name != o_data.name %}{{ conn|qtIdent(o_data
 {% else %}{{ conn|qtIdent(o_data.schema, o_data.name) }}
 {% endif %}
     SET SCHEMA {{ conn|qtIdent(data.schema) }};
-{% endif %}
-{% endif %}
+{% endif %}
\ No newline at end of file
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 2490d70..a09cfff 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py
@@ -234,6 +234,44 @@ class DataTypeReader:
         else:
             return name + length + array
 
+    @classmethod
+    def parse_type_name(cls, type_name):
+        """
+        Returns prase type name without length and precision
+        so that we can match the end result with types in the select2.
+
+        Args:
+            self: self
+            type_name: Type name
+        """
+
+        # Manual Data type formatting
+        # If data type has () with them then we need to remove them
+        # eg bit(1) because we need to match the name with combobox
+
+        is_array = False
+        if type_name.endswith('[]'):
+            is_array = True
+            type_name = type_name.rstrip('[]')
+
+        idx = type_name.find('(')
+        if idx and type_name.endswith(')'):
+            type_name = type_name[:idx]
+        # We need special handling of timestamp types as
+        # variable precision is between the type
+        elif idx and type_name.startswith("time"):
+            end_idx = type_name.find(')')
+            # If we found the end then form the type string
+            if end_idx != 1:
+                from re import sub as sub_str
+                pattern = r'(\(\d+\))'
+                type_name = sub_str(pattern, '', type_name)
+
+        if is_array:
+            type_name += "[]"
+
+        return type_name
+
 
 def trigger_definition(data):
     """

---

From: Dave Page <[email-masked]>
Date: Fri, 3 Feb 2017 13:52:10 +0000

Thanks - patch applied.

---

From: Dave Page <[email-masked]>
Date: Sat, 4 Feb 2017 14:04:42 +0000

Hi Murtuza,

I clearly neglected to run the regression tests with this patch, and
unfortunately it looks like it broke them. Can you look at this ASAP
please?

runTest (pgadmin.browser.server_groups.servers.databases.schemas.tables.column.tests.test_column_add.ColumnAddTestCase)
This function will add column under table node. (Add table Node URL)
... 2017-02-04 14:43:55,191: ERROR pgadmin: Failed to execute query
(execute_scalar) for the server #4 - DB:test_db_9e176 (Query-id:
4792242):
Error Message:ERROR:  syntax error at or near "False"
LINE 2:     ADD COLUMN test_column_add_f0d5d char(False);
                                                  ^
FAIL

As far as I can see, the app works fine - it's only the test that
broke (probably because the application uses type name aliases (e.g.
character) rather than the base type name that the test is using
("char").

Thanks.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 6 Feb 2017 11:18:04 +0530

Sure, Checking.

On Sat, Feb 4, 2017 at 7:34 PM, Dave Page <[email-masked]> wrote:

> runTest (pgadmin.browser.server_groups.servers.databases.
> schemas.tables.column.tests.test_column_add.ColumnAddTestCase)

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 6 Feb 2017 11:42:35 +0530

Hi Dave,

Please find a fix for the same, attribute length was set to False instead
of None.

On Mon, Feb 6, 2017 at 11:18 AM, Murtuza Zabuawala <
[email-masked]> wrote:

>> runTest (pgadmin.browser.server_groups.servers.databases.schemas.
>> tables.column.tests.test_column_add.ColumnAddTestCase)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
index 25542d7..fb2bbe7 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
@@ -55,7 +55,7 @@ class ColumnAddTestCase(BaseTestGenerator):
                 "attacl": [],
                 "is_primary_key": False,
                 "attnotnull": False,
-                "attlen": False,
+                "attlen": None,
                 "attprecision": None,
                 "attoptions": [],
                 "seclabels": []

---

From: Dave Page <[email-masked]>
Date: Mon, 6 Feb 2017 09:49:28 +0000

Thanks, applied.

On Mon, Feb 6, 2017 at 6:12 AM, Murtuza Zabuawala
<[email-masked]> wrote:
> Hi Dave,
>
> Please find a fix for the same, attribute length was set to False instead of
> None.
>
>
> --
> Regards,
> Murtuza Zabuawala
> On Mon, Feb 6, 2017 at 11:18 AM, Murtuza Zabuawala
>>> runTest
>>> (pgadmin.browser.server_groups.servers.databases.schemas.tables.column.tests.test_column_add.ColumnAddTestCase)