Hi,
Please find the attached updated patch.
On Mon, Nov 27, 2017 at 5:15 PM, Dave Page <dp...@pgadmin.org> wrote:
> Hi
>
> On Thu, Nov 23, 2017 at 1:28 PM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached patch for RM #2849: Allow editing of data on
>> tables with OIDs but no primary key.
>>
>
> I like that if I add a new row or rows and hit Save, the OIDs are fetched
> immediately. However;
>
> - It marks the row as read-only. We do that currently because we don't
> return the key info on Save, and thus require a Refresh before any further
> editing. However, if we have the OID, we can edit again immediately.
>
> Fixed
> - If we can return the new OIDs on Save, can't we do the same for primary
> key values? That would be consistent with OIDs, and would remove the need
> to disable rows, leading to a much nicer use experience I think.
>
> Implemented
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
Thanks,
Khushboo
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 f3353d6..2c3e573 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
@@ -15,9 +15,13 @@ WHERE
{% if clid %}
AND att.attnum = {{ clid|qtLiteral }}
{% endif %}
- {### To show system objects ###}
- {% if not show_sys_objects %}
+{### To show system objects ###}
+{% if not show_sys_objects and not has_oids %}
AND att.attnum > 0
- {% endif %}
+{% endif %}
+{### To show oids in view data ###}
+{% if has_oids %}
+ AND (att.attnum > 0 OR (att.attname = 'oid' AND att.attnum < 0))
+{% endif %}
AND att.attisdropped IS FALSE
ORDER BY att.attnum
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 4f1de2a..584f7b1 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
@@ -16,8 +16,12 @@ WHERE
AND att.attnum = {{ clid|qtLiteral }}
{% endif %}
{### To show system objects ###}
-{% if not show_sys_objects %}
+{% if not show_sys_objects and not has_oids %}
AND att.attnum > 0
{% endif %}
+{### To show oids in view data ###}
+{% if has_oids %}
+ AND (att.attnum > 0 OR (att.attname = 'oid' AND att.attnum < 0))
+{% endif %}
AND att.attisdropped IS FALSE
ORDER BY att.attnum
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 8dcb444..363d6f8 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -433,6 +433,9 @@ def start_view_data(trans_id):
sql = trans_obj.get_sql()
pk_names, primary_keys = trans_obj.get_primary_keys(default_conn)
+ # Fetch OIDs status
+ has_oids = trans_obj.has_oids(default_conn)
+
# Fetch the applied filter.
filter_applied = trans_obj.is_filter_applied()
@@ -444,6 +447,10 @@ def start_view_data(trans_id):
# Store the primary keys to the session object
session_obj['primary_keys'] = primary_keys
+
+ # Store the OIDs status into session object
+ session_obj['has_oids'] = has_oids
+
update_session_grid_transaction(trans_id, session_obj)
# Execute sql asynchronously
@@ -655,6 +662,8 @@ def poll(trans_id):
types = {}
client_primary_key = None
rset = None
+ has_oids = False
+ oids = None
# Check the transaction and connection status
status, error_msg, conn, trans_obj, session_obj = check_transaction_status(trans_id)
@@ -680,6 +689,11 @@ def poll(trans_id):
if 'primary_keys' in session_obj:
primary_keys = session_obj['primary_keys']
+ if 'has_oids' in session_obj:
+ has_oids = session_obj['has_oids']
+ if has_oids:
+ oids = {'oid': 'oid'}
+
# Fetch column information
columns_info = conn.get_column_info()
client_primary_key = generate_client_primary_key_name(
@@ -698,7 +712,8 @@ def poll(trans_id):
SQL = render_template("/".join([template_path,
'nodes.sql']),
- tid=command_obj.obj_id)
+ tid=command_obj.obj_id,
+ has_oids=True)
# rows with attribute not_null
colst, rset = conn.execute_2darray(SQL)
if not colst:
@@ -811,7 +826,9 @@ def poll(trans_id):
'colinfo': columns_info,
'primary_keys': primary_keys,
'types': types,
- 'client_primary_key': client_primary_key
+ 'client_primary_key': client_primary_key,
+ 'has_oids': has_oids,
+ 'oids': oids
}
)
@@ -945,7 +962,7 @@ def save(trans_id):
and trans_obj is not None and session_obj is not None:
# If there is no primary key found then return from the function.
- if len(session_obj['primary_keys']) <= 0 or len(changed_data) <= 0:
+ if (len(session_obj['primary_keys']) <= 0 or len(changed_data) <= 0) and 'has_oids' not in session_obj:
return make_json_response(
data={
'status': False,
diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py
index d09bf2b..30a5d9b 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -364,16 +364,20 @@ class TableCommand(GridCommand):
# Fetch the primary keys for the table
pk_names, primary_keys = self.get_primary_keys(default_conn)
+ # Fetch OIDs status
+ has_oids = self.has_oids(default_conn)
+
sql_filter = self.get_filter()
if sql_filter is None:
sql = render_template("/".join([self.sql_path, 'objectquery.sql']), object_name=self.object_name,
nsp_name=self.nsp_name, pk_names=pk_names, cmd_type=self.cmd_type,
- limit=self.limit, primary_keys=primary_keys)
+ limit=self.limit, primary_keys=primary_keys, has_oids=has_oids)
else:
sql = render_template("/".join([self.sql_path, 'objectquery.sql']), object_name=self.object_name,
nsp_name=self.nsp_name, pk_names=pk_names, cmd_type=self.cmd_type,
- sql_filter=sql_filter, limit=self.limit, primary_keys=primary_keys)
+ sql_filter=sql_filter, limit=self.limit, primary_keys=primary_keys,
+ has_oids=has_oids)
return sql
@@ -418,6 +422,31 @@ class TableCommand(GridCommand):
def can_filter(self):
return True
+ def has_oids(self, default_conn=None):
+ """
+ This function checks whether the table has oids or not.
+ """
+ driver = get_driver(PG_DEFAULT_DRIVER)
+ if default_conn is None:
+ manager = driver.connection_manager(self.sid)
+ conn = manager.connection(did=self.did, conn_id=self.conn_id)
+ else:
+ conn = default_conn
+
+ if conn.connected():
+
+ # Fetch the table oids status
+ query = render_template("/".join([self.sql_path, 'has_oids.sql']), obj_id=self.obj_id)
+
+ status, has_oids = conn.execute_scalar(query)
+ if not status:
+ raise Exception(has_oids)
+
+ else:
+ raise Exception(gettext('Not connected to server or connection with the server has been closed.'))
+
+ return has_oids
+
def save(self,
changed_data,
columns_info,
@@ -489,6 +518,7 @@ class TableCommand(GridCommand):
# of not null which is set by default.
column_data = {}
pk_names, primary_keys = self.get_primary_keys()
+ has_oids = 'oid' in column_type
for each_row in changed_data[of_type]:
data = changed_data[of_type][each_row]['data']
@@ -507,8 +537,11 @@ class TableCommand(GridCommand):
object_name=self.object_name,
nsp_name=self.nsp_name,
data_type=column_type,
- pk_names=pk_names)
- list_of_sql[of_type].append({'sql': sql, 'data': data})
+ pk_names=pk_names,
+ has_oids=has_oids)
+ list_of_sql[of_type].append({'sql': sql, 'data': data,
+ 'has_oids': has_oids, 'client_row': each_row,
+ 'ret_ids': True})
# Reset column data
column_data = {}
@@ -568,13 +601,21 @@ class TableCommand(GridCommand):
for opr, sqls in list_of_sql.items():
for item in sqls:
if item['sql']:
- status, res = conn.execute_void(
- item['sql'], item['data'])
- rows_affected = conn.rows_affected()
-
- # store the result of each query in dictionary
- query_res[count] = {'status': status, 'result': res,
- 'sql': sql, 'rows_affected': rows_affected}
+ oids = None
+ pk_values = None
+ # Send oids/primary keys
+ if ('has_oids' in item and item['has_oids']) or\
+ ('ret_ids' in item and item['ret_ids']):
+ status, res = conn.execute_dict(
+ item['sql'], item['data'])
+
+ if 'has_oids' in item and item['has_oids']: # Get OIDs
+ oids = {item['client_row']: res['rows'][0]}
+ elif res and 'rows' in res and len(res['rows']) > 0: # Get Primary Keys
+ pk_values = {item['client_row']: res['rows'][0]}
+ else:
+ status, res = conn.execute_void(
+ item['sql'], item['data'])
if not status:
conn.execute_void('ROLLBACK;')
@@ -591,6 +632,14 @@ class TableCommand(GridCommand):
_rowid = 0
return status, res, query_res, _rowid
+
+ rows_affected = conn.rows_affected()
+
+ # store the result of each query in dictionary
+ query_res[count] = {'status': status, 'result': None if oids or pk_values else res,
+ 'sql': sql, 'rows_affected': rows_affected,
+ 'oids': oids, 'pk_values': pk_values}
+
count += 1
# Commit the transaction if there is no error found
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 8c1a82c..ca4d043 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -599,7 +599,10 @@ define('tools.querytool', [
options['width'] = column_size[table_name][c.name];
}
// If grid is editable then add editor else make it readonly
- if (c.cell == 'Json') {
+ if (c.cell == 'oid') {
+ options['editor'] = null;
+ }
+ else if (c.cell == 'Json') {
options['editor'] = is_editable ? Slick.Editors.JsonText
: Slick.Editors.ReadOnlyJsonText;
options['formatter'] = c.is_array ? Slick.Formatters.JsonStringArray : Slick.Formatters.JsonString;
@@ -688,13 +691,14 @@ define('tools.querytool', [
grid.registerPlugin(gridSelector);
var editor_data = {
- keys: self.handler.primary_keys,
+ keys: (_.isEmpty(self.handler.primary_keys) && self.handler.has_oids) ? self.handler.oids : self.handler.primary_keys,
vals: collection,
columns: columns,
grid: grid,
selection: grid.getSelectionModel(),
editor: self,
- client_primary_key: self.client_primary_key
+ client_primary_key: self.client_primary_key,
+ has_oids: self.handler.has_oids
};
self.handler.slickgrid = grid;
@@ -820,7 +824,8 @@ define('tools.querytool', [
// self.handler.data_store.updated will holds all the updated data
var changed_column = args.grid.getColumns()[args.cell].field,
updated_data = args.item[changed_column], // New value for current field
- _pk = args.item[self.client_primary_key] || null, // Unique key to identify row
+ _pk = args.item[self.client_primary_key] || null, // Unique key to identify row
+ has_oids = self.handler.has_oids || null, // Unique key to identify row
column_data = {},
_type;
@@ -852,6 +857,7 @@ define('tools.querytool', [
column_data[changed_column] = updated_data;
+
if (_pk) {
// Check if it is in newly added row by user?
if (_pk in self.handler.data_store.added) {
@@ -859,7 +865,7 @@ define('tools.querytool', [
self.handler.data_store.added[_pk]['data'],
column_data);
//Find type for current column
- self.handler.data_store.added[_pk]['err'] = false
+ self.handler.data_store.added[_pk]['err'] = false;
// Check if it is updated data from existing rows?
} else if (_pk in self.handler.data_store.updated) {
_.extend(
@@ -1900,18 +1906,20 @@ define('tools.querytool', [
_render: function (data) {
var self = this;
self.colinfo = data.col_info;
- self.primary_keys = data.primary_keys;
+ self.primary_keys = (_.isEmpty(data.primary_keys) && data.has_oids)? data.oids : data.primary_keys;
self.client_primary_key = data.client_primary_key;
self.cell_selected = false;
self.selected_model = null;
self.changedModels = [];
+ self.has_oids = data.has_oids;
+ self.oids = data.oids;
$('.sql-editor-explain').empty();
/* If object don't have primary keys then set the
* can_edit flag to false.
*/
- if (self.primary_keys === null || self.primary_keys === undefined
- || _.size(self.primary_keys) === 0)
+ if ((self.primary_keys === null || self.primary_keys === undefined
+ || _.size(self.primary_keys) === 0) && self.has_oids == false)
self.can_edit = false;
else
self.can_edit = true;
@@ -2072,6 +2080,9 @@ define('tools.querytool', [
}
// Identify cell type of column.
switch (type) {
+ case "oid":
+ col_cell = 'oid';
+ break;
case "json":
case "json[]":
case "jsonb":
@@ -2128,7 +2139,7 @@ define('tools.querytool', [
'pos': c.pos,
'label': column_label,
'cell': col_cell,
- 'can_edit': self.can_edit,
+ 'can_edit': (c.name == 'oid') ? false : self.can_edit,
'type': type,
'not_null': c.not_null,
'has_default_val': c.has_default_val,
@@ -2380,6 +2391,40 @@ define('tools.querytool', [
data_length = dataView.getLength(),
data = [];
if (res.data.status) {
+ if(self.has_oids && is_added) {
+ // Update the oids in a grid after addition
+ dataView.beginUpdate();
+ _.each(res.data.query_result, function (r) {
+ if(!_.isNull(r.oids)) {
+ var row_id = Object.keys(r.oids)[0];
+ var item = grid.getDataItem(row_id);
+ item['oid'] = r.oids[row_id];
+ }
+ });
+ dataView.endUpdate();
+ }
+
+ if(is_added) {
+ // Update the primary keys in a grid after addition
+ dataView.beginUpdate();
+ _.each(res.data.query_result, function (r) {
+ if(!_.isNull(r.pk_values)) {
+ // Fetch temp_id returned by server after addition
+ var row_id = Object.keys(r.pk_values)[0];
+ _.each(req_data.added_index, function(v, k) {
+ if (v == row_id) {
+ // Fetch item data through row index
+ var item = grid.getDataItem(k);
+ _.each(r.pk_values[row_id], function (p, k) {
+ // Update the primary key values if different
+ if(item[k] != p) item[k] = p;
+ })
+ }
+ })
+ }
+ });
+ dataView.endUpdate();
+ }
// Remove flag is_row_copied from copied rows
_.each(data, function (row, idx) {
if (row.is_row_copied) {
@@ -2412,15 +2457,6 @@ define('tools.querytool', [
grid.setSelectedRows([]);
}
- // whether a cell is editable or not is decided in
- // grid.onBeforeEditCell function (on cell click)
- // but this function should do its job after save
- // operation. So assign list of added rows to original
- // rows_to_disable array.
- if (is_added) {
- self.rows_to_disable = _.clone(self.temp_new_rows);
- }
-
grid.setSelectedRows([]);
// Reset data store
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/has_oids.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/has_oids.sql
new file mode 100644
index 0000000..edeeb83
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/has_oids.sql
@@ -0,0 +1,6 @@
+{# ============= Check object has OIDs or not ============= #}
+{% if obj_id %}
+SELECT rel.relhasoids AS has_oids
+FROM pg_class rel
+WHERE rel.oid = {{ obj_id }}::oid
+{% endif %}
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
index 23ffcb4..a4ed872 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
@@ -5,4 +5,6 @@ INSERT INTO {{ conn|qtIdent(nsp_name, object_name) }} (
) VALUES (
{% for col in data_to_be_saved %}
{% if not loop.first %}, {% endif %}%({{ col }})s::{{ data_type[col] }}{% endfor %}
-);
+)
+{% if pk_names %} returning {{pk_names}}{% endif %}
+{% if has_oids %} returning oid{% endif %};
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/objectquery.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/objectquery.sql
index 2c6ba58..1cb60d9 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/objectquery.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/objectquery.sql
@@ -1,5 +1,5 @@
{# SQL query for objects #}
-SELECT * FROM {{ conn|qtIdent(nsp_name, object_name) }}
+SELECT {% if has_oids %}oid, {% endif %}* FROM {{ conn|qtIdent(nsp_name, object_name) }}
{% if sql_filter %}
WHERE {{ sql_filter }}
{% endif %}
