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/
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