[GSoC][New Feature] Editable and Read-only Columns

Yosry Muhammad <[email protected]>
Newsgroups gmane.comp.db.postgresql.pgadmin.devel
Message-ID <CAFSMqn9zz=eWZme5GAJh0RvmQjn2CBukcxszmhp8nMbRwfxyzA@mail.gmail.com>
Hi, hackers !

Please find attached a work-in-progress patch adding a new feature to the
Query Tool on top of updatable result-sets.

This patch allows individual columns of an updatable result-set to be
editable or read-only. This allows for a wider variety of updatable
result-sets, for example:

- Result-sets with duplicated columns.
- Result-sets with renamed columns (if a column is renamed to a primary key
name, the real primary key can be correctly identified) .
- Result-sets including columns that are not selected directly from a table
(e.g concatenation of 2 columns or system columns).

In the above cases, these columns would be read-only while other columns of
the result-set are editable. Editable/Read-only columns are identified by
icons and tooltips in the column header.

This is still a work-in-progress, updates to tests and documentation is
still due. Looking forward to your thoughts and feedback!

Also, do you think the editable/read-only icons should apply in both
View/Edit Data and Query Tool for consistency? or hidden from View/Edit
Data as all columns are editable anyway?

Thanks.

-- 
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/
editable_readonly_columns.patch (text/x-patch, 21 KB)
diff --git a/web/pgadmin/static/js/sqleditor_utils.js b/web/pgadmin/static/js/sqleditor_utils.js
index 9d14ac467..ad7f784c4 100644
--- a/web/pgadmin/static/js/sqleditor_utils.js
+++ b/web/pgadmin/static/js/sqleditor_utils.js
@@ -211,6 +211,24 @@ define(['jquery', 'sources/gettext', 'sources/url_for'],
           'title': encodeURIComponent(value),
         };
       },
+
+      addEditableIcon: function(columnDefinition, is_editable) {
+        let content = null;
+        if(is_editable) {
+          content = '<i class="fa fa-pencil"></i>';
+        }
+        else {
+          content = '<i class="fa fa-lock"></i>';
+        }
+        columnDefinition.header = {
+          buttons: [
+            {
+              cssClass: 'editable-column-header-icon',
+              content: content,
+            },
+          ],
+        };
+      },
     };
     return sqlEditorUtils;
   });
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 5091c11dd..53a863c43 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -430,40 +430,17 @@ def poll(trans_id):
                         oids = {'oid': 'oid'}
 
                 if columns_info is not None:
-                    # If it is a QueryToolCommand that has obj_id attribute
-                    # then it should also be editable
-                    if hasattr(trans_obj, 'obj_id') and \
-                        (not isinstance(trans_obj, QueryToolCommand) or
-                         trans_obj.can_edit()):
-                        # Get the template path for the column
-                        template_path = 'columns/sql/#{0}#'.format(
-                            conn.manager.version
-                        )
-
-                        SQL = render_template(
-                            "/".join([template_path, 'nodes.sql']),
-                            tid=trans_obj.obj_id,
-                            has_oids=True
-                        )
-                        # rows with attribute not_null
-                        colst, rset = conn.execute_2darray(SQL)
-                        if not colst:
-                            return internal_server_error(errormsg=rset)
-
-                    for key, col in enumerate(columns_info):
-                        col_type = dict()
-                        col_type['type_code'] = col['type_code']
-                        col_type['type_name'] = None
-                        col_type['internal_size'] = col['internal_size']
-                        columns[col['name']] = col_type
-
-                        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']
+                    # Only QueryToolCommand or TableCommand can be editable
+                    if hasattr(trans_obj, 'obj_id') and trans_obj.can_edit():
+                        columns = trans_obj.get_columns_types(conn)
+
+                    else:
+                        for key, col in enumerate(columns_info):
+                            col_type = dict()
+                            col_type['type_code'] = col['type_code']
+                            col_type['type_name'] = None
+                            col_type['internal_size'] = col['internal_size']
+                            columns[col['name']] = col_type
 
                 if columns:
                     st, types = fetch_pg_types(columns, trans_obj)
diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py
index 01815f8cf..520e9d0a9 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -22,6 +22,7 @@ from pgadmin.utils.driver import get_driver
 from pgadmin.tools.sqleditor.utils.is_query_resultset_updatable \
     import is_query_resultset_updatable
 from pgadmin.tools.sqleditor.utils.save_changed_data import save_changed_data
+from pgadmin.tools.sqleditor.utils.get_column_types import get_columns_types
 
 from config import PG_DEFAULT_DRIVER
 
@@ -677,6 +678,15 @@ class TableCommand(GridCommand):
                                  client_primary_key=client_primary_key,
                                  conn=conn)
 
+    def get_columns_types(self, conn):
+        columns_info = conn.get_column_info()
+        has_oids = self.has_oids()
+        table_oid = self.obj_id
+        return get_columns_types(conn=conn,
+                                 columns_info=columns_info,
+                                 has_oids=has_oids,
+                                 table_oid=table_oid)
+
 
 class ViewCommand(GridCommand):
     """
@@ -864,6 +874,7 @@ class QueryToolCommand(BaseCommand, FetchedRowTracker):
         self.primary_keys = None
         self.pk_names = None
         self.table_has_oids = False
+        self.columns_types = None
 
     def get_sql(self, default_conn=None):
         return None
@@ -874,6 +885,9 @@ class QueryToolCommand(BaseCommand, FetchedRowTracker):
     def get_primary_keys(self):
         return self.pk_names, self.primary_keys
 
+    def get_columns_types(self, conn=None):
+        return self.columns_types
+
     def has_oids(self):
         return self.table_has_oids
 
@@ -906,8 +920,9 @@ class QueryToolCommand(BaseCommand, FetchedRowTracker):
         # Get the path to the sql templates
         sql_path = 'sqleditor/sql/#{0}#'.format(manager.version)
 
-        self.is_updatable_resultset, self.table_has_oids, self.primary_keys, \
-            pk_names, table_oid = is_query_resultset_updatable(conn, sql_path)
+        self.is_updatable_resultset, self.table_has_oids,\
+            self.primary_keys, pk_names, table_oid,\
+            self.columns_types = is_query_resultset_updatable(conn, sql_path)
 
         # Create pk_names attribute in the required format
         if pk_names is not None:
diff --git a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
index 21448327f..2894ef6b8 100644
--- a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
+++ b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
@@ -377,7 +377,7 @@ input.editor-checkbox:focus {
 
 
 /* For geometry column button */
-.div-view-geometry-column {
+.div-view-geometry-column, .editable-column-header-icon {
   float: right;
   height: 100%;
   display: flex;
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index ca9083f1e..8c24ba03e 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -772,6 +772,7 @@ define('tools.querytool', [
           not_null: c.not_null,
           has_default_val: c.has_default_val,
           is_array: c.is_array,
+          can_edit: c.can_edit,
         };
 
         // Get the columns width based on longer string among data type or
@@ -789,17 +790,17 @@ define('tools.querytool', [
         if (c.cell == 'oid' && c.name == 'oid') {
           options['editor'] = null;
         } else if (c.cell == 'Json') {
-          options['editor'] = is_editable ? Slick.Editors.JsonText :
+          options['editor'] = c.can_edit ? Slick.Editors.JsonText :
             Slick.Editors.ReadOnlyJsonText;
           options['formatter'] = Slick.Formatters.JsonString;
         } else if (c.cell == 'number' || c.cell == 'oid' ||
           $.inArray(c.type, ['xid', 'real']) !== -1
         ) {
-          options['editor'] = is_editable ? Slick.Editors.CustomNumber :
+          options['editor'] = c.can_edit ? Slick.Editors.CustomNumber :
             Slick.Editors.ReadOnlyText;
           options['formatter'] = Slick.Formatters.Numbers;
         } else if (c.cell == 'boolean') {
-          options['editor'] = is_editable ? Slick.Editors.Checkbox :
+          options['editor'] = c.can_edit ? Slick.Editors.Checkbox :
             Slick.Editors.ReadOnlyCheckbox;
           options['formatter'] = Slick.Formatters.Checkmark;
         } else if (c.cell == 'binary') {
@@ -809,22 +810,39 @@ define('tools.querytool', [
           // increase width to add 'view' button
           options['width'] += 28;
         } else {
-          options['editor'] = is_editable ? Slick.Editors.pgText :
+          options['editor'] = c.can_edit ? Slick.Editors.pgText :
             Slick.Editors.ReadOnlypgText;
           options['formatter'] = Slick.Formatters.Text;
         }
 
+        if(!_.isUndefined(c.can_edit)) {
+          // Increase width for editable/read-only icon
+          options['width'] += 12;
+
+          let tooltip = '';
+          if(c.can_edit)
+            tooltip = 'Editable column';
+          else
+            tooltip = 'Read-only column';
+
+          options['toolTip'] = tooltip;
+        }
+
         grid_columns.push(options);
       });
 
       var gridSelector = new GridSelector();
       grid_columns = self.grid_columns = gridSelector.getColumnDefinitions(grid_columns);
 
-      // add 'view' button in geometry and geography type column header
       _.each(grid_columns, function (c) {
+        // Add 'view' button in geometry and geography type column headers
         if (c.column_type_internal == 'geometry' || c.column_type_internal == 'geography') {
           GeometryViewer.add_header_button(c);
         }
+        // Add editable/read-only icon to columns
+        else if (!_.isUndefined(c.can_edit)) {
+          SqlEditorUtils.addEditableIcon(c, c.can_edit);
+        }
       });
 
       if (rows_affected) {
@@ -2540,12 +2558,13 @@ define('tools.querytool', [
 
         // Create columns required by slick grid to render
         _.each(colinfo, function(c) {
-          var is_primary_key = false;
+          var is_primary_key = false,
+            is_editable = self.can_edit && (!self.is_query_tool || c.is_editable);
 
-          // Check whether table have primary key
-          if (_.size(primary_keys) > 0) {
+          // Check whether this column is a primary key
+          if (is_editable && _.size(primary_keys) > 0) {
             _.each(primary_keys, function(value, key) {
-              if (key === c.name)
+              if (key === c.display_name)
                 is_primary_key = true;
             });
           }
@@ -2644,7 +2663,7 @@ define('tools.querytool', [
               'pos': c.pos,
               'label': column_label,
               'cell': col_cell,
-              'can_edit': (c.name == 'oid') ? false : self.can_edit,
+              'can_edit': (c.name == 'oid') ? false : is_editable,
               'type': type,
               'not_null': c.not_null,
               'has_default_val': c.has_default_val,
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/get_columns.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/get_columns.sql
index 610747dfb..851b98523 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/get_columns.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/get_columns.sql
@@ -1,6 +1,6 @@
 {# ============= Fetch the columns ============= #}
 {% if obj_id %}
-SELECT at.attname, ty.typname
+SELECT at.attname, ty.typname, at.attnum
     FROM pg_attribute at
     LEFT JOIN pg_type ty ON (ty.oid = at.atttypid)
 WHERE attrelid={{obj_id}}::oid
diff --git a/web/pgadmin/tools/sqleditor/utils/get_column_types.py b/web/pgadmin/tools/sqleditor/utils/get_column_types.py
new file mode 100644
index 000000000..655741dcb
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/utils/get_column_types.py
@@ -0,0 +1,44 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2019, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+"""
+    Get the column types for QueryToolCommand or TableCommand when
+    the result-set is editable.
+"""
+
+from flask import render_template
+
+
+def get_columns_types(columns_info, table_oid, conn, has_oids):
+    nodes_sqlpath = 'columns/sql/#{0}#'.format(conn.manager.version)
+    query = render_template(
+        "/".join([nodes_sqlpath, 'nodes.sql']),
+        tid=table_oid,
+        has_oids=has_oids
+    )
+
+    colst, rset = conn.execute_2darray(query)
+    if not colst:
+        raise Exception(rset)
+
+    column_types = dict()
+    for col in columns_info:
+        col_type = dict()
+        col_type['type_code'] = col['type_code']
+        col_type['type_name'] = None
+        col_type['internal_size'] = col['internal_size']
+        column_types[col['name']] = col_type
+
+        for row in rset['rows']:
+            if row['oid'] == col['table_column']:
+                col_type['not_null'] = col['not_null'] = row['not_null']
+
+                col_type['has_default_val'] = \
+                    col['has_default_val'] = row['has_default_val']
+
+    return column_types
diff --git a/web/pgadmin/tools/sqleditor/utils/is_query_resultset_updatable.py b/web/pgadmin/tools/sqleditor/utils/is_query_resultset_updatable.py
index 9c2bd09a6..bb45ca786 100644
--- a/web/pgadmin/tools/sqleditor/utils/is_query_resultset_updatable.py
+++ b/web/pgadmin/tools/sqleditor/utils/is_query_resultset_updatable.py
@@ -8,11 +8,18 @@
 ##########################################################################
 
 """
-    Check if the result-set of a query is updatable, A resultset is
-    updatable (as of this version) if:
-        - All columns belong to the same table.
-        - All the primary key columns of the table are present in the resultset
-        - No duplicate columns
+    Check if the result-set of a query is editable, A result-set is
+    editable if:
+        - All columns are either selected directly from a single table, or
+          are not table columns at all (e.g. concatenation of 2 columns).
+          Only columns that are selected directly from a the table are
+          editable, other columns are read-only.
+        - All the primary key columns or oids (if applicable) of the table are
+          present in the result-set.
+
+    Note:
+        - Duplicate columns (selected twice) or renamed columns are also
+          read-only.
 """
 from flask import render_template
 from flask_babelex import gettext
@@ -20,16 +27,18 @@ try:
     from collections import OrderedDict
 except ImportError:
     from ordereddict import OrderedDict
+from pgadmin.tools.sqleditor.utils.get_column_types import get_columns_types
 
 
 def is_query_resultset_updatable(conn, sql_path):
     """
         This function is used to check whether the last successful query
-        produced updatable results.
+        produced editable results.
 
         Args:
             conn: Connection object.
-            sql_path: the path to the sql templates.
+            sql_path: the path to the sql templates
+                      primary_keys.sql & columns.sql.
     """
     columns_info = conn.get_column_info()
 
@@ -37,26 +46,45 @@ def is_query_resultset_updatable(conn, sql_path):
         return return_not_updatable()
 
     table_oid = _check_single_table(columns_info)
-    if not table_oid:
-        return return_not_updatable()
-
-    if not _check_duplicate_columns(columns_info):
+    if table_oid is None:
         return return_not_updatable()
 
     if conn.connected():
-        primary_keys, pk_names = _check_primary_keys(conn=conn,
-                                                     columns_info=columns_info,
-                                                     table_oid=table_oid,
-                                                     sql_path=sql_path)
-
+        # Get all the table columns
+        table_columns = _get_table_columns(conn=conn,
+                                           table_oid=table_oid,
+                                           sql_path=sql_path)
+
+        # Add 'is_editable' key to columns_info items
+        _check_editable_columns(table_columns=table_columns,
+                                results_columns=columns_info)
+
+        # Editable columns are columns that are not renamed or duplicated
+        editable_columns = \
+            [col for col in columns_info if col['is_editable']]
+
+        # Only check editable columns for primary keys
+        primary_keys, pk_names = \
+            _check_primary_keys(conn=conn,
+                                columns_info=editable_columns,
+                                table_oid=table_oid,
+                                sql_path=sql_path)
+
+        # Check all columns for oids
         has_oids = _check_oids(conn=conn,
                                columns_info=columns_info,
                                table_oid=table_oid,
                                sql_path=sql_path)
 
         if has_oids or primary_keys is not None:
-            return True, has_oids, primary_keys, pk_names, table_oid
+            column_types = get_columns_types(columns_info=editable_columns,
+                                             table_oid=table_oid,
+                                             conn=conn,
+                                             has_oids=has_oids)
+            return True, has_oids, primary_keys, \
+                pk_names, table_oid, column_types
         else:
+            _set_all_columns_not_editable(columns_info=columns_info)
             return return_not_updatable()
     else:
         raise Exception(
@@ -66,20 +94,34 @@ def is_query_resultset_updatable(conn, sql_path):
 
 
 def _check_single_table(columns_info):
-    table_oid = columns_info[0]['table_oid']
+    table_oid = None
     for column in columns_info:
-        if column['table_oid'] != table_oid:
+        # Skip columns that are not directly from tables
+        if column['table_oid'] is None:
+            continue
+        # If we don't have a table_oid yet, store this one
+        if table_oid is None:
+            table_oid = column['table_oid']
+        # If we already have one, check that all the columns have the same one
+        elif column['table_oid'] != table_oid:
             return None
     return table_oid
 
 
-def _check_duplicate_columns(columns_info):
-    column_numbers = \
-        [col['table_column'] for col in columns_info]
-    is_duplicate_columns = len(column_numbers) != len(set(column_numbers))
-    if is_duplicate_columns:
-        return False
-    return True
+def _check_editable_columns(table_columns, results_columns):
+    table_columns_numbers = set()
+    for results_column in results_columns:
+        table_column_number = results_column['table_column']
+        if table_column_number is None:  # Not a table column
+            results_column['is_editable'] = False
+        elif table_column_number in table_columns_numbers:  # Duplicate
+            results_column['is_editable'] = False
+        elif results_column['display_name'] \
+                != table_columns[table_column_number]:
+            results_column['is_editable'] = False
+        else:
+            results_column['is_editable'] = True
+            table_columns_numbers.add(table_column_number)
 
 
 def _check_oids(conn, sql_path, table_oid, columns_info):
@@ -111,26 +153,20 @@ def _check_primary_keys(conn, columns_info, sql_path, table_oid):
                           table_oid=table_oid,
                           sql_path=sql_path)
 
-    if not _check_primary_keys_uniquely_exist(primary_keys_columns,
-                                              columns_info):
+    if not _check_all_primary_keys_exist(primary_keys_columns,
+                                         columns_info):
         primary_keys = None
         pk_names = None
     return primary_keys, pk_names
 
 
-def _check_primary_keys_uniquely_exist(primary_keys_columns, columns_info):
+def _check_all_primary_keys_exist(primary_keys_columns, columns_info):
     for pk in primary_keys_columns:
         pk_exists = False
         for col in columns_info:
             if col['table_column'] == pk['column_number']:
                 pk_exists = True
-                # If the primary key column is renamed
-                if col['display_name'] != pk['name']:
-                    return False
-            # If a normal column is renamed to a primary key column name
-            elif col['display_name'] == pk['name']:
-                return False
-
+                break
         if not pk_exists:
             return False
     return True
@@ -160,5 +196,26 @@ def _get_primary_keys(sql_path, table_oid, conn):
     return primary_keys, primary_keys_columns, pk_names
 
 
+def _get_table_columns(sql_path, table_oid, conn):
+    query = render_template(
+        "/".join([sql_path, 'get_columns.sql']),
+        obj_id=table_oid
+    )
+    status, result = conn.execute_dict(query)
+    if not status:
+        raise Exception(result)
+
+    columns = {}
+    for row in result['rows']:
+        columns[row['attnum']] = row['attname']
+
+    return columns
+
+
+def _set_all_columns_not_editable(columns_info):
+    for col in columns_info:
+        col['is_editable'] = False
+
+
 def return_not_updatable():
-    return False, False, None, None, None
+    return False, False, None, None, None, None
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.