public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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: Fri, 12 May 2017 11:09:52 +0530
Message-ID: <CAM5-9D-azs4qaCQ3wWwMr1t4ivTDvzjBbJopCEqReevE5gBrhQ@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxyPmmSHO=1EbJbbeEqFv-XN9eUEwA4hj4FG_qphGFwMQ@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>
<CAM5-9D9JdEim47AnTVKKhqOBJsbxoO+ufwE7FtPya9HmC6j8Zg@mail.gmail.com>
<CA+OCxozX9ea6b6a6Jmwgf5m0JNybZje5HGe27--bDg1t1LP8iA@mail.gmail.com>
<CA+OCxoxyPmmSHO=1EbJbbeEqFv-XN9eUEwA4hj4FG_qphGFwMQ@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi Dave,
Please find updated patch
On Thu, May 11, 2017 at 2:29 PM, Dave Page <[email protected]> wrote:
> Ooops, managed to hit send too soon.
>
> ... it leads to failed updates such as:
>
> 2017-05-11 09:55:47,570: SQL pgadmin: Execute (void) for server #1 -
> CONN:2096775 (Query-id: 4540472):
> UPDATE public.defaults SET
> data_default_no_nulls = 'asas' WHERE
> ;
> 2017-05-11 09:55:47,577: ERROR pgadmin:
> Failed to execute query (execute_void) for the server #1 - CONN:2096775
> (Query-id: 4540472):
> Error Message:ERROR: syntax error at or near ";"
> LINE 3: ;
>
The new rows added is kept disabled untill grid is not refreshed.
>
>
> On Thu, May 11, 2017 at 9:58 AM, Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> There seems to be couple of bugs in this;
>>
>> - When creating a new row with my test table, if I click in the id
>> column, don't change anything, then click in another column, the ID column
>> value changes from [default] to [null], making it impossible to save that
>> row with the default value. In this case I would expect it to stay at
>> [default] unless I explicitly entered a value.
>>
> Fixed.
>
>> - When I add a new row, but leave the id as [default], the row is saved,
>> but the [default] marker changes from gray to black (but only in the id
>> column.
>>
> I forgot to add 'grey_color' class for numeric type fields. Now added.
>
>> - I'm able to edit a freshly added row immediately after saving but
>> before refreshing. This shouldn't be allowed if we don't know what the
>> primary key value is, as it leads to failed updates such as:
>>
>>
>> On Wed, May 10, 2017 at 9:52 AM, Surinder Kumar <
>> [email protected]> wrote:
>>
>>> 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
>>>>
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> 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_v4.patch (16.3K, 3-RM_2257_v4.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..faad731 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,14 @@
};
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.
+ if ($input.val() == "" && _.isUndefined(defaultValue)) {
+ return false;
+ } else {
+ return (!($input.val() == "" && _.isNull(defaultValue))) &&
+ ($input.val() != defaultValue);
+ }
};
this.validate = function () {
@@ -253,7 +265,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 +294,11 @@
};
this.isValueChanged = function () {
- return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+ if ($input.val() == "" && _.isUndefined(defaultValue)) {
+ return false;
+ } else {
+ return (!($input.val() == "" && _.isNull(defaultValue))) && ($input.val() != defaultValue);
+ }
};
this.validate = function () {
@@ -498,6 +514,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 +859,14 @@
};
this.isValueChanged = function () {
- return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+ if ($input.val() == "" && _.isUndefined(defaultValue)) {
+ return false;
+ } else 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..642fc2f 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 grey_color'>[default]</span>";
}
- else if (value === "") {
- return '';
+ else if (
+ (_.isUndefined(value) || value === null || value === "") ||
+ (_.isUndefined(value) && columnDef.not_null)
+ ) {
+ return "<span class='pull-right grey_color'>[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..9601467 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -442,6 +442,25 @@ 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 = {}
+ pk_names, primary_keys = self.get_primary_keys()
+
+ 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 +469,19 @@ 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,
+ pk_names=pk_names)
list_of_sql.append(sql)
# For updated rows
diff --git a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
index 574ef53..1d572ca 100644
--- a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
+++ b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
@@ -420,6 +420,10 @@ input.editor-checkbox:focus {
background: #e46b6b;
}
+.grid-canvas .disabled_row {
+ background: #c1c1c1;
+}
+
/* color the first column */
.sr .sc:first-child {
background-color: #2c76b4;
@@ -445,4 +449,4 @@ input.editor-checkbox:focus {
.sr.ui-widget-content {
border-top: 1px solid silver;
-}
+}
\ No newline at end of file
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 2062aa2..ba9dd43 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
@@ -625,6 +627,12 @@ define(
}
}
}
+ // Disable rows having default values
+ if (!_.isUndefined(self.handler.rows_to_disable) &&
+ _.indexOf(self.handler.rows_to_disable, i) !== -1
+ ) {
+ cssClass += ' disabled_row';
+ }
return {'cssClasses': cssClass};
}
@@ -702,6 +710,14 @@ define(
// This will be used to collect primary key for that row
grid.onBeforeEditCell.subscribe(function (e, args) {
var before_data = args.item;
+
+ // If newly added row is saved but grid is not refreshed,
+ // then disable cell editing for that row
+ if(self.handler.rows_to_disable &&
+ _.contains(self.handler.rows_to_disable, args.row)) {
+ return false;
+ }
+
if(self.handler.can_edit && before_data && '__temp_PK' in before_data) {
var _pk = before_data.__temp_PK,
_keys = self.handler.primary_keys,
@@ -1629,6 +1645,8 @@ define(
self.query_start_time = new Date();
self.rows_affected = 0;
self._init_polling_flags();
+ // keep track of newly added rows
+ self.rows_to_disable = new Array();
self.trigger(
'pgadmin-sqleditor:loading-icon:show',
@@ -2077,7 +2095,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);
});
@@ -2320,6 +2340,10 @@ define(
grid.setSelectedRows([]);
}
+ // Add last row(new row) to keep track of it
+ if (is_added) {
+ self.rows_to_disable.push(grid.getDataLength()-1);
+ }
// Reset data store
self.data_store = {
'added': {},
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-9D-azs4qaCQ3wWwMr1t4ivTDvzjBbJopCEqReevE5gBrhQ@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