public inbox for [email protected]  
help / color / mirror / Atom feed
From: Surinder Kumar <[email protected]>
To: Shruti B Iyer <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Matthew Kleiman <[email protected]>
Subject: Re: Re: [pgAdmin4][Patch][Feature #1971]: Remember column sizes between executions of the same query in the query tool
Date: Tue, 6 Jun 2017 13:52:46 +0530
Message-ID: <CAM5-9D8yKkmvVdhexSK7qaCP3UUr3ORk9D7TNnPwJVJW=LSx7A@mail.gmail.com> (raw)
In-Reply-To: <CACrUwh+iWeuscD94nZ8SGxfyvs=-JjundJ6sUZDeqDXsxFXC3g@mail.gmail.com>
References: <CAM5-9D8qbLXBzs3rAjAMUZtj32hDNV11rnymXgxUfKePkqL-rQ@mail.gmail.com>
	<CAM5-9D_AAy6vLL3kMDVKPhtpDe_bNFH0o+uc-4RajZFeypFe+g@mail.gmail.com>
	<CACrUwh+iWeuscD94nZ8SGxfyvs=-JjundJ6sUZDeqDXsxFXC3g@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi All,

Please find updated patch which includes Jasmine test cases for functions
getHash and calculateColumnWidth

Thanks,
Surinder

On Mon, Jun 5, 2017 at 11:38 PM, Shruti B Iyer <[email protected]> wrote:

> Hi Surinder!
>
> We reviewed this patch. The changes look good and we especially like that
> you have extracted out the new utility functions and the epicRandomString
> function too.
>
> This patch will likely affect the Query Results patch that is currently
> under review
> <https://www.postgresql.org/message-id/flat/CAAtBm9V-tNQrtjxt4n8JJek5M4v9KW_h3sgbL1ydcBB%2BtLus2w%40m...;.
> In order to assist either us or yourself when making a merge between these
> patches, it would help to have jasmine unit testing for the two new
> functions, getHash and calculate_column_width.
>
> Also, we suggest that you rename calculate_column_width to
> calculateColumnWidth for consistency with javascript code style.
>
> Thanks,
> Shruti and Matt
>
> On Mon, Jun 5, 2017 at 9:16 AM Surinder Kumar <
> [email protected]> wrote:
>
>> Staged changes are missed in previous patch, so please ignore.
>> Please find attached updated patch.
>>
>> On Mon, Jun 5, 2017 at 4:29 PM, Surinder Kumar <
>> [email protected]> wrote:
>>
>>> Hi
>>>
>>> This patch contains two fixes:
>>>
>>> 1) In Query/tool or Edit grid, the width of table column header is fixed
>>> depending on the column type(int, boolean, char etc.) due to which the
>>> column name or type appears cut from right and doesn't looks good from user
>>> point of view. The main concern was to display as much as the content of
>>> column should be displayed.
>>>
>>> Now the width of column is decided using the text length of column name
>>> or column type so that the column takes exact width it required and it
>>> don't appears cut.
>>>
>>> 2) Remember column size after re-running a query.
>>>
>>> The approach is to extract table name from the query executed and use it
>>> to store its columns width.
>>> Whenever the column(s) width of a table is adjusted, the corresponding
>>> values are updated into the object and used every time the same query is
>>> executed.
>>>
>>> If a query is executed for e.g:
>>>
>>> SELECT generate_series(1, 1000) as id, generate_series(1, 1000) as name,
>>> generate_series(1, 1000) as age
>>>
>>> ​it ​
>>> displays 3 columns
>>> ​but don't have any table name. In that case,
>>>  i use a hash generator function which returns unique hash for a query
>>> written in query editor and adjusted column(s) width are stored against
>>> that hash in object.
>>>
>>> Is there any way to get temporary table name(avoiding unique hash) for
>>> such queries ?​
>>>
>>> Also, Moved utilities functions into pgadmin/static/utils.js
>>>
>>> Please find attached patch and review.
>>>
>>> Thanks,
>>> Surinder Kumar
>>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list ([email protected])
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>


-- 
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] Feature_1971_with_jasmine_tests.patch (10.9K, 3-Feature_1971_with_jasmine_tests.patch)
  download | inline diff:
diff --git a/web/pgadmin/static/js/sqleditor_utils.js b/web/pgadmin/static/js/sqleditor_utils.js
new file mode 100644
index 0000000..2afc4de
--- /dev/null
+++ b/web/pgadmin/static/js/sqleditor_utils.js
@@ -0,0 +1,58 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2017, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+// This file contains common utilities functions used in sqleditor modules
+
+define(['jquery'],
+  function ($) {
+    var sqlEditorUtils = {
+      /* Reference link http://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript
+       * Modified as per requirement.
+       */
+      epicRandomString: function(length) {
+        var s = [];
+        var hexDigits = "0123456789abcdef";
+        for (var i = 0; i < 36; i++) {
+            s[i] = hexDigits.substr(
+                    Math.floor(Math.random() * 0x10), 1
+                  );
+        }
+        // bits 12-15 of the time_hi_and_version field to 0010
+        s[14] = "4";
+        // bits 6-7 of the clock_seq_hi_and_reserved to 01
+        s[19] = hexDigits.substr((s[19] & 0x3) | 0x8, 1);
+        s[8] = s[13] = s[18] = s[23] = "-";
+
+        var uuid = s.join("");
+        return uuid.replace(/-/g, '').substr(0, length);
+      },
+
+      // Returns a unique hash for input string
+      getHash: function(input) {
+        var hash = 0, len = input.length;
+        for (var i = 0; i < len; i++) {
+          hash  = ((hash << 5) - hash) + input.charCodeAt(i);
+          hash |= 0; // to 32bit integer
+        }
+        return hash;
+      },
+      calculateColumnWidth: function (text) {
+        // Calculate column header width based on column name or type
+        // Create a temporary element with given label, append to body
+        // calculate its width and remove the element.
+        $('body').append(
+            '<span id="pg_text" style="visibility: hidden;">'+ text + '</span>'
+        );
+        var width = $('#pg_text').width() + 30;
+        $('#pg_text').remove(); // remove element
+
+        return width;
+      }
+    };
+    return sqlEditorUtils;
+});
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 597c436..3170bc1 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -3,7 +3,7 @@ define(
     'jquery', 'underscore', 'underscore.string', 'alertify', 'pgadmin',
     'backbone', 'backgrid', 'codemirror', 'pgadmin.misc.explain',
     'sources/selection/grid_selector', 'sources/selection/clipboard',
-    'sources/selection/copy_data',
+    'sources/selection/copy_data', 'sources/sqleditor_utils',
 
     'slickgrid', 'bootstrap', 'pgadmin.browser', 'wcdocker',
     'codemirror/mode/sql/sql', 'codemirror/addon/selection/mark-selection',
@@ -27,7 +27,8 @@ define(
     'slickgrid/slick.grid'
   ],
   function(
-    $, _, S, alertify, pgAdmin, Backbone, Backgrid, CodeMirror, pgExplain, GridSelector, clipboard, copyData
+    $, _, S, alertify, pgAdmin, Backbone, Backgrid, CodeMirror, pgExplain, GridSelector, clipboard, copyData,
+    SqlEditorUtils
   ) {
     /* Return back, this has been called more than once */
     if (pgAdmin.SqlEditor)
@@ -39,28 +40,6 @@ define(
         pgBrowser = pgAdmin.Browser,
         Slick = window.Slick;
 
-    /* Reference link
-     * http://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript
-     * Modified as per requirement.
-     */
-    function epicRandomString(b) {
-      var s = [];
-      var hexDigits = "0123456789abcdef";
-      for (var i = 0; i < 36; i++) {
-          s[i] = hexDigits.substr(
-                  Math.floor(Math.random() * 0x10), 1
-                );
-      }
-      // bits 12-15 of the time_hi_and_version field to 0010
-      s[14] = "4";
-      // bits 6-7 of the clock_seq_hi_and_reserved to 01
-      s[19] = hexDigits.substr((s[19] & 0x3) | 0x8, 1);
-      s[8] = s[13] = s[18] = s[23] = "-";
-
-      var uuid = s.join("");
-      return uuid.replace(/-/g, '').substr(0, b);
-    };
-
     // Define key codes for shortcut keys
     var F5_KEY = 116,
         F7_KEY = 118,
@@ -73,6 +52,7 @@ define(
       initialize: function(opts) {
         this.$el = opts.el;
         this.handler = opts.handler;
+        this.handler['col_size'] = {};
       },
 
       // Bind all the events
@@ -410,12 +390,18 @@ define(
       /* To prompt user for unsaved changes */
       user_confirmation: function(panel, msg) {
         // If there is anything to save then prompt user
+        var that = this;
         alertify.confirm("{{ _('Unsaved changes') }}", msg,
           function() {
             // Do nothing as user do not want to save, just continue
             window.onbeforeunload = null;
             panel.off(wcDocker.EVENT.CLOSING);
+            // remove col_size object on panel close
+            if (!_.isUndefined(that.handler.col_size)) {
+              delete that.handler.col_size;
+            }
             window.top.pgAdmin.Browser.docker.removePanel(panel);
+
           },
           function() {
             // Stop, User wants to save
@@ -426,53 +412,6 @@ define(
         return false;
       },
 
-      get_column_width: function (column_type, grid_width) {
-
-        switch(column_type) {
-          case "bigint":
-          case "bigint[]":
-          case "bigserial":
-          case "bit":
-          case "bit[]":
-          case "bit varying":
-          case "bit varying[]":
-          case "\"char\"":
-          case "decimal":
-          case "decimal[]":
-          case "double precision":
-          case "double precision[]":
-          case "int4range":
-          case "int4range[]":
-          case "int8range":
-          case "int8range[]":
-          case "integer":
-          case "integer[]":
-          case "money":
-          case "money[]":
-          case "numeric":
-          case "numeric[]":
-          case "numrange":
-          case "numrange[]":
-          case "oid":
-          case "oid[]":
-          case "real":
-          case "real[]":
-          case "serial":
-          case "smallint":
-          case "smallint[]":
-          case "smallserial":
-            return 80;
-          case "boolean":
-          case "boolean[]":
-            return 60;
-        }
-
-        /* In case of other data types we will calculate
-         * 20% of the total container width and return it.
-         */
-        return Math.round((grid_width * 20)/ 100)
-      },
-
       /* Regarding SlickGrid usage in render_grid function.
 
        SlickGrid Plugins:
@@ -585,8 +524,22 @@ define(
         }
 
         var grid_columns = [];
+        var column_size = self.handler['col_size'],
+          query = self.handler.query,
+          // Extract table name from query
+          table_list = query.match(/select.*from\s+(\w+)/i);
+
+        if (!table_list) {
+          table_name = SqlEditorUtils.getHash(query);
+        }
+        else {
+          table_name = table_list[1];
+        }
+
+        self.handler['table_name'] = table_name;
+        column_size[table_name] = column_size[table_name] || {};
 
-        var grid_width = $($('#editor-panel').find('.wcFrame')[1]).width()
+        var grid_width = $($('#editor-panel').find('.wcFrame')[1]).width();
         _.each(columns, function(c) {
             var options = {
               id: c.name,
@@ -597,8 +550,18 @@ define(
               has_default_val: c.has_default_val
             };
 
-            // Get the columns width based on data type
-            options['width'] = self.get_column_width(c.type, grid_width);
+            // Get the columns width based on longer string among data type or
+            // column name.
+            var label = c.label.split('<br>');
+            label = label[0].length > label[1].length ? label[0] : label[1];
+
+            if (_.isUndefined(column_size[table_name][c.name])) {
+                options['width'] = SqlEditorUtils.calculateColumnWidth(label)
+                column_size[table_name][c.name] = SqlEditorUtils.calculateColumnWidth(label);
+            }
+            else {
+                options['width'] = column_size[table_name][c.name];
+            }
 
             // If grid is editable then add editor else make it readonly
             if(c.cell == 'Json') {
@@ -642,7 +605,7 @@ define(
 
         // Add our own custom primary key to keep track of changes
         _.each(collection, function(row){
-          row['__temp_PK'] = epicRandomString(15);
+          row['__temp_PK'] = SqlEditorUtils.epicRandomString(15);
         });
 
         // Add-on function which allow us to identify the faulty row after insert/update
@@ -773,6 +736,13 @@ define(
            }.bind(editor_data));
         }
 
+        grid.onColumnsResized.subscribe(function (e, args) {
+            var columns = this.getColumns();
+            _.each(columns, function(col, key) {
+                var column_size = self.handler['col_size'];
+                column_size[self.handler['table_name']][col['id']] = col['width'];
+            });
+        });
 
         // Listener function which will be called before user updates existing cell
         // This will be used to collect primary key for that row
@@ -921,7 +891,7 @@ define(
         // Listener function which will be called when user adds new rows
         grid.onAddNewRow.subscribe(function (e, args) {
           // self.handler.data_store.added will holds all the newly added rows/data
-          var _key = epicRandomString(10),
+          var _key = SqlEditorUtils.epicRandomString(10),
             column = args.column,
             item = args.item,
             data_length = this.grid.getDataLength(),
diff --git a/web/regression/javascript/sqleditor_utils_spec.js b/web/regression/javascript/sqleditor_utils_spec.js
new file mode 100644
index 0000000..4b79dc0
--- /dev/null
+++ b/web/regression/javascript/sqleditor_utils_spec.js
@@ -0,0 +1,27 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2017, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+define(["sources/sqleditor_utils"],
+function (SqlEditorUtils) {
+  describe("SqlEditorUtils", function () {
+
+    describe("Generate a random string of size 10", function () {
+      it("returns string of length 10", function () {
+        expect(SqlEditorUtils.epicRandomString(10).length).toEqual(10);
+      });
+    });
+
+    describe("Generate a unique hash for given string", function () {
+      it("returns unique hash", function () {
+        expect(SqlEditorUtils.getHash('select * from test')).toEqual(403379630);
+      });
+    });
+
+  });
+});


view thread (10+ 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], [email protected]
  Subject: Re: Re: [pgAdmin4][Patch][Feature #1971]: Remember column sizes between executions of the same query in the query tool
  In-Reply-To: <CAM5-9D8yKkmvVdhexSK7qaCP3UUr3ORk9D7TNnPwJVJW=LSx7A@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