public inbox for [email protected]  
help / color / mirror / Atom feed
From: Aditya Toshniwal <[email protected]>
To: pgadmin-hackers <[email protected]>
Cc: Akshay Joshi <[email protected]>
Subject: Re: [pgAdmin][RM4506] "can't execute an empty query" message displayed if user remove fill factor of any existing table
Date: Tue, 10 Dec 2019 12:56:46 +0530
Message-ID: <CAM9w-_mv1jj3WA01GC4jBU4mv+kGvGruLZVC=qJJCjx97nYQCQ@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDekzcP=boUFDRFXfb8RXVmmQh3bkqCtBWJpP2RLNS8Y+w@mail.gmail.com>
References: <CAM9w-_kNeQ5EANaFoFEB4UFFM03NAZBuwQzFmBdFd59rpWfriw@mail.gmail.com>
	<CANxoLDekzcP=boUFDRFXfb8RXVmmQh3bkqCtBWJpP2RLNS8Y+w@mail.gmail.com>

Hi Hackers,

Attached is the updated patch. The modified SQL missed the reset clause for
FILLFACTOR. That's added now.
I've changed the int/numeric field validation to disallow spaces in the
form field. Currently,  fill factor allows spaces and fails while saving as
"invalid value".
Also, added min and max for fill factor in Mview dialog.

All the test cases are passing. Kindly review.


On Mon, Dec 9, 2019 at 4:30 PM Akshay Joshi <[email protected]>
wrote:

> Hi Aditya
>
> The issue has not been resolved. Remove fill factor it won't generate
> MSQL. Please verify and fix it.
>
> On Mon, Dec 9, 2019 at 3:33 PM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hi Hackers,
>>
>> Attached is the patch to fix an issue where just clicking on an
>> empty textbox like fill factor in tables dialog considers it a change and
>> so the save button is enabled.
>> The fix is common will apply at other places also.
>>
>> Kindly review.
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>


-- 
Thanks and Regards,
Aditya Toshniwal
Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


Attachments:

  [application/octet-stream] RM4506_v2.patch (4.5K, 3-RM4506_v2.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/update.sql
index d12a83851..804620898 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/update.sql
@@ -56,6 +56,9 @@ ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
 {% if data.fillfactor and data.fillfactor != o_data.fillfactor %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
     SET (FILLFACTOR={{data.fillfactor}});
+{% elif data.fillfactor == '' and data.fillfactor != o_data.fillfactor %}
+ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
+    RESET (FILLFACTOR);
 
 {% endif %}
 {###############################}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql
index c8bf6db94..5673cff73 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/update.sql
@@ -64,6 +64,9 @@ ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
 {% if data.fillfactor and data.fillfactor != o_data.fillfactor %}
 ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
     SET (FILLFACTOR={{data.fillfactor}});
+{% elif data.fillfactor == '' and data.fillfactor != o_data.fillfactor %}
+ALTER TABLE {{conn|qtIdent(data.schema, data.name)}}
+    RESET (FILLFACTOR);
 
 {% endif %}
 {###############################}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/mview.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/mview.js
index 6d3dfbc05..77f5d6b60 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/mview.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/mview.js
@@ -203,7 +203,7 @@ define('pgadmin.node.mview', [
         },{
           id: 'fillfactor', label: gettext('Fill factor'),
           group: gettext('Storage'), mode: ['edit', 'create'],
-          type: 'int',
+          type: 'int', min: 10, max: 100,
         },{
           type: 'nested', control: 'tab', id: 'materialization',
           label: gettext('Parameter'), mode: ['edit', 'create'],
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 514c40392..564f7f2c9 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -367,7 +367,8 @@ define([
             return;
           }
           attrs[k] = v;
-          if (_.isEqual(self.origSessAttrs[k], v)) {
+          /* If the orig value was null and new one is empty string, then its a "no change" */
+          if (_.isEqual(self.origSessAttrs[k], v) || (self.origSessAttrs[k] === null && v === '')) {
             delete self.sessAttrs[k];
           } else {
             self.sessAttrs[k] = v;
@@ -738,9 +739,7 @@ define([
         field = this.fieldData[keys[i]];
         msg = null;
 
-        if (!(_.isUndefined(value) || _.isNull(value) ||
-            String(value).replace(/^\s+|\s+$/g, '') == '')) {
-
+        if (!(_.isUndefined(value) || _.isNull(value) || String(value) === '')) {
           if (!field) {
             continue;
           }
diff --git a/web/pgadmin/browser/templates/browser/js/messages.js b/web/pgadmin/browser/templates/browser/js/messages.js
index 7621ca3ae..41301c4fc 100644
--- a/web/pgadmin/browser/templates/browser/js/messages.js
+++ b/web/pgadmin/browser/templates/browser/js/messages.js
@@ -26,8 +26,8 @@ define(
     'SQL_NO_CHANGE': gettext('Nothing changed'),
     'MUST_BE_INT' : gettext("'%s' must be an integer."),
     'MUST_BE_NUM' : gettext("'%s' must be a numeric."),
-    'MUST_GR_EQ' : gettext("%s' must be greater than or equal to %d."),
-    'MUST_LESS_EQ' : gettext("'%s' must be less than or equal to %d."),
+    'MUST_GR_EQ' : gettext("'%s' must be greater than or equal to %s."),
+    'MUST_LESS_EQ' : gettext("'%s' must be less than or equal to %s."),
     'STATISTICS_LABEL': gettext("Statistics"),
     'STATISTICS_VALUE_LABEL': gettext("Value"),
     'NODE_HAS_NO_SQL': gettext("No SQL could be generated for the selected object."),


view thread (6+ 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: [pgAdmin][RM4506] "can't execute an empty query" message displayed if user remove fill factor of any existing table
  In-Reply-To: <CAM9w-_mv1jj3WA01GC4jBU4mv+kGvGruLZVC=qJJCjx97nYQCQ@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