public inbox for [email protected]  
help / color / mirror / Atom feed
From: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values
Date: Wed, 10 May 2017 14:22:01 +0530
Message-ID: <CAM5-9D9JdEim47AnTVKKhqOBJsbxoO+ufwE7FtPya9HmC6j8Zg@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxGQfP-HXTfBF+hDLvkex1YDQA+Xovppt6ogT+H8juXGw@mail.gmail.com>
References: <CAM5-9D_SSL81uT4AqsRr8WPABWA6S-iE34OxLb8YqfVN3myeJg@mail.gmail.com>
	<CA+OCxoySjV87N+YEkhXRzWaaGNCX4o+KuKnVGrYhLuWYeDx+SA@mail.gmail.com>
	<CAM5-9D8MdAqvix74_K+eMkeGkP7r4Aost1Uqz4hupMRPxwqzAw@mail.gmail.com>
	<CA+OCxoxkfbwJaWgCTTZ7eqrkv7Y49jU2EUmDsH_5raZqSFwZsA@mail.gmail.com>
	<CAM5-9D_ZEysH0ewuCoOoon18mim_G9r-2es=+MbyYXnsLqrTEw@mail.gmail.com>
	<CAM5-9D-kZEWrqgxr6-PyB_YVCdovJm8PpXGDpdfcpo65t_Sakg@mail.gmail.com>
	<CA+OCxoyFLFK9nxbpTGJ=f2em1D_fmmFUaxh7LgUeNGWQr3fj6w@mail.gmail.com>
	<CAM5-9D9Pm3bGgiv+fSjfr3Ouc-puMS3m6x4N+vGdELmWVJNcig@mail.gmail.com>
	<CA+OCxozDsoz0NX+4ywBH4gvyHQ2=Lk0f9QT8q2Mj0v=5p+5B_A@mail.gmail.com>
	<CAM5-9D-nWM3i-krc0KbOcAtDQNMefJs+xOFkgP0oaV84wPBbAw@mail.gmail.com>
	<CA+OCxoyNm6zP-=ozL2eTaGpsVgggkp5stU2ZyKhdTzspkAc8sQ@mail.gmail.com>
	<CAM5-9D_76iVD-C+80_7WL1y9skxo5ntmoQbKVOVaMb+0Xa90yg@mail.gmail.com>
	<CA+OCxowq-52VR5hTgBTGa3zGi6NVkAmM2AoGDLW2_wm_QdL0hA@mail.gmail.com>
	<CA+OCxox6_JojQkaujk4qE=egt5Zj6hYe0V33kLYTxQhPMH4H2Q@mail.gmail.com>
	<CAM5-9D8TfBb5YXvEcyd+1=oarWVYOFnN-DVYNx_N3zNRKPGbdg@mail.gmail.com>
	<CA+OCxoxGQfP-HXTfBF+hDLvkex1YDQA+Xovppt6ogT+H8juXGw@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi Dave,

Please find attached patch for RM only.

*Changes:*

 - All formatters now handles both [null] and [default] values

 - the cell values are validated on server side as in pgAdmin3.

 - added light grey color for cells with [null] and [default] placeholders.

On Wed, May 10, 2017 at 2:12 PM, Dave Page <[email protected]> wrote:

>
>
> On Wed, May 10, 2017 at 9:39 AM, Surinder Kumar <
> [email protected]> wrote:
>
>> Hi Dave,
>>
>> On Wed, May 10, 2017 at 2:06 PM, Dave Page <[email protected]> wrote:
>>
>>> Any chance we can get this wrapped up today Surinder?
>>>
>> ​I have fixed RM case, I am currently writing its feature test cases
>> which is taking some time.
>> Should I send patch for RM case only?​ I will try to complete test cases
>> by today eod.
>>
>
> Yes please.
>
> 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] RM_2257_v3.patch (13.4K, 3-RM_2257_v3.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql
index 759e657..f3353d6 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql
@@ -1,4 +1,5 @@
-SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype
+SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype,
+att.attnotnull as not_null, att.atthasdef as has_default_val
 FROM pg_attribute att
   JOIN pg_type ty ON ty.oid=atttypid
   JOIN pg_namespace tn ON tn.oid=ty.typnamespace
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql
index 7536a9c..4f1de2a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql
@@ -1,4 +1,5 @@
-SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype
+SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype,
+att.attnotnull as not_null, att.atthasdef as has_default_val
 FROM pg_attribute att
     JOIN pg_type ty ON ty.oid=atttypid
     JOIN pg_namespace tn ON tn.oid=ty.typnamespace
diff --git a/web/pgadmin/static/css/pgadmin.css b/web/pgadmin/static/css/pgadmin.css
index 6508feb..1a2d443 100644
--- a/web/pgadmin/static/css/pgadmin.css
+++ b/web/pgadmin/static/css/pgadmin.css
@@ -780,4 +780,7 @@ lgg-el-container[el=md] .pg-el-lg-8,
 }
 .user-language select{
   height: 25px !important;
+}
+.grey_color {
+  color: #999999;
 }
\ No newline at end of file
diff --git a/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js b/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js
index cdfba4d..0bdd1ab 100644
--- a/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js
+++ b/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js
@@ -110,7 +110,12 @@
 
     // When text editor opens
     this.loadValue = function (item) {
-      if (item[args.column.pos] === "") {
+      var col = args.column;
+
+      if (_.isUndefined(item[args.column.pos]) && col.has_default_val) {
+        $input.val("");
+      }
+      else if (item[args.column.pos] === "") {
         $input.val("''");
       }
       else {
@@ -145,7 +150,10 @@
     };
 
     this.isValueChanged = function () {
-      return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+      // Use _.isNull(value) for comparison for null instead of
+      // defaultValue == null, because it returns true for undefined value.
+      return (!($input.val() == "" && _.isNull(defaultValue))) &&
+        ($input.val() != defaultValue);
     };
 
     this.validate = function () {
@@ -253,7 +261,7 @@
 
     this.loadValue = function (item) {
       var data = defaultValue = item[args.column.pos];
-      if (typeof data === "object" && !Array.isArray(data)) {
+      if (data && typeof data === "object" && !Array.isArray(data)) {
         data = JSON.stringify(data);
       } else if (Array.isArray(data)) {
         var temp = [];
@@ -282,7 +290,7 @@
     };
 
     this.isValueChanged = function () {
-      return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+      return (!($input.val() == "" && _.isNull(defaultValue))) && ($input.val() != defaultValue);
     };
 
     this.validate = function () {
@@ -498,6 +506,12 @@
     };
 
     this.validate = function () {
+      if (args.column.validator) {
+        var validationResults = args.column.validator(this.serializeValue());
+        if (!validationResults.valid) {
+          return validationResults;
+        }
+      }
       return {
         valid: true,
         msg: null
@@ -837,7 +851,12 @@
     };
 
     this.isValueChanged = function () {
-      return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+      if ($input.val() == "" && defaultValue == "") {
+        return true;
+      } else {
+        return (!($input.val() == "" && _.isNull(defaultValue ))) &&
+        ($input.val() != defaultValue);
+      }
     };
 
     this.validate = function () {
diff --git a/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js b/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js
index 290bddd..924f523 100644
--- a/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js
+++ b/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js
@@ -19,8 +19,15 @@
   });
 
   function JsonFormatter(row, cell, value, columnDef, dataContext) {
-    if (value == null || value === "") {
-      return "";
+    // If column has default value, set placeholder
+    if (_.isUndefined(value) && columnDef.has_default_val) {
+      return "<span class='pull-left grey_color'>[default]</span>";
+    }
+    else if (
+      (_.isUndefined(value) && columnDef.not_null) ||
+      (_.isUndefined(value) || value === null)
+    ) {
+      return "<span class='pull-left grey_color'>[null]</span>";
     } else {
       // Stringify only if it's json object
       if (typeof value === "object" && !Array.isArray(value)) {
@@ -42,11 +49,15 @@
   }
 
   function NumbersFormatter(row, cell, value, columnDef, dataContext) {
-    if (_.isUndefined(value) || value === null) {
-      return "<span class='pull-right'>[null]</span>";
+    // If column has default value, set placeholder
+    if (_.isUndefined(value) && columnDef.has_default_val) {
+      return "<span class='pull-right'>[default]</span>";
     }
-    else if (value === "") {
-      return '';
+    else if (
+      (_.isUndefined(value) || value === null || value === "") ||
+      (_.isUndefined(value) && columnDef.not_null)
+    ) {
+      return "<span class='pull-right'>[null]</span>";
     }
     else {
       return "<span style='float:right'>" + _.escape(value) + "</span>";
@@ -57,17 +68,30 @@
     /* Checkbox has 3 states
      * 1) checked=true
      * 2) unchecked=false
-     * 3) indeterminate=null/''
+     * 3) indeterminate=null
      */
-    if (value == null || value === "") {
-      return "<span class='pull-left'>[null]</span>";
+    if (_.isUndefined(value) && columnDef.has_default_val) {
+      return "<span class='pull-left grey_color'>[default]</span>";
+    }
+    else if (
+      (_.isUndefined(value) && columnDef.not_null) ||
+      (value == null || value === "")
+    ) {
+      return "<span class='pull-left grey_color'>[null]</span>";
     }
     return value ? "true" : "false";
   }
 
   function TextFormatter(row, cell, value, columnDef, dataContext) {
-    if (_.isUndefined(value) || value === null) {
-      return "<span class='pull-left'>[null]</span>";
+    // If column has default value, set placeholder
+    if (_.isUndefined(value) && columnDef.has_default_val) {
+        return "<span class='pull-left grey_color'>[default]</span>";
+    }
+    else if (
+      (_.isUndefined(value) && columnDef.not_null) ||
+      (_.isUndefined(value) || _.isNull(value))
+    ) {
+      return "<span class='pull-left grey_color'>[null]</span>";
     }
     else {
       return _.escape(value);
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index d114988..f7466d8 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -440,8 +440,23 @@ def get_columns(trans_id):
     columns = dict()
     columns_info = None
     primary_keys = None
+    rset = None
     status, error_msg, conn, trans_obj, session_obj = check_transaction_status(trans_id)
     if status and conn is not None and session_obj is not None:
+
+        ver = conn.manager.version
+        # Get the template path for the column
+        template_path = 'column/sql/#{0}#'.format(ver)
+        command_obj = pickle.loads(session_obj['command_obj'])
+        if hasattr(command_obj, 'obj_id'):
+            SQL = render_template("/".join([template_path,
+                                            'nodes.sql']),
+                                  tid=command_obj.obj_id)
+            # rows with attribute not_null
+            status, rset = conn.execute_2darray(SQL)
+            if not status:
+                return internal_server_error(errormsg=rset)
+
         # Check PK column info is available or not
         if 'primary_keys' in session_obj:
             primary_keys = session_obj['primary_keys']
@@ -449,10 +464,17 @@ def get_columns(trans_id):
         # Fetch column information
         columns_info = conn.get_column_info()
         if columns_info is not None:
-            for col in columns_info:
+            for key, col in enumerate(columns_info):
                 col_type = dict()
                 col_type['type_code'] = col['type_code']
                 col_type['type_name'] = None
+                if rset:
+                    col_type['not_null'] = col['not_null'] = \
+                        rset['rows'][key]['not_null']
+
+                    col_type['has_default_val'] = col['has_default_val'] = \
+                        rset['rows'][key]['has_default_val']
+
                 columns[col['name']] = col_type
 
         # As we changed the transaction object we need to
@@ -602,6 +624,7 @@ def save(trans_id):
     status, error_msg, conn, trans_obj, session_obj = check_transaction_status(trans_id)
     if status and conn is not None \
             and trans_obj is not None and session_obj is not None:
+        setattr(trans_obj, 'columns_info', session_obj['columns_info'])
 
         # If there is no primary key found then return from the function.
         if len(session_obj['primary_keys']) <= 0 or len(changed_data) <= 0:
diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py
index 1795155..29ed23f 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -442,6 +442,23 @@ class TableCommand(GridCommand):
 
                 # For newly added rows
                 if of_type == 'added':
+
+                    # When new rows are added, only changed columns data is
+                    # sent from client side. But if column is not_null and has
+                    # no_default_value, set column to blank, instead
+                    # of not null which is set by default.
+                    column_data = {}
+                    column_type = {}
+                    for each_col in self.columns_info:
+                        if (
+                            self.columns_info[each_col]['not_null'] and
+                            not self.columns_info[each_col][
+                                'has_default_val']
+                        ):
+                            column_data[each_col] = None
+                            column_type[each_col] =\
+                                self.columns_info[each_col]['type_name']
+
                     for each_row in changed_data[of_type]:
                         data = changed_data[of_type][each_row]['data']
                         # Remove our unique tracking key
@@ -450,12 +467,18 @@ class TableCommand(GridCommand):
                         data_type = set_column_names(changed_data[of_type][each_row]['data_type'])
                         list_of_rowid.append(data.get('__temp_PK'))
 
+                        # Update columns value and data type
+                        # with columns having not_null=False and has
+                        # no default value
+                        column_data.update(data)
+                        column_type.update(data_type)
+
                         sql = render_template("/".join([self.sql_path, 'insert.sql']),
-                                              data_to_be_saved=data,
+                                              data_to_be_saved=column_data,
                                               primary_keys=None,
                                               object_name=self.object_name,
                                               nsp_name=self.nsp_name,
-                                              data_type=data_type)
+                                              data_type=column_type)
                         list_of_sql.append(sql)
 
                 # For updated rows
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 2062aa2..bd2b3ff 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -557,7 +557,9 @@ define(
               id: c.name,
               pos: c.pos,
               field: c.name,
-              name: c.label
+              name: c.label,
+              not_null: c.not_null,
+              has_default_val: c.has_default_val
             };
 
             // Get the columns width based on data type
@@ -2077,7 +2079,9 @@ define(
                     'label': column_label,
                     'cell': col_cell,
                     'can_edit': self.can_edit,
-                    'type': type
+                    'type': type,
+                    'not_null': c.not_null,
+                    'has_default_val': c.has_default_val
                   };
                   columns.push(col);
                 });


view thread (25+ 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: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values
  In-Reply-To: <CAM5-9D9JdEim47AnTVKKhqOBJsbxoO+ufwE7FtPya9HmC6j8Zg@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