public inbox for [email protected]  
help / color / mirror / Atom feed
[pgAdmin4] RM5965 Couldn't download file of Marcos query results
7+ messages / 3 participants
[nested] [flat]

* [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-07 09:25  Rahul Shirsat <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Rahul Shirsat @ 2020-12-07 09:25 UTC (permalink / raw)
  To: pgadmin-hackers

Hi Hackers,

Please find the attached patch which resolves the issue of macros query
results download, have used async cursor to achieve this functionality,
where for downloading the results, cursor is scrolled back to 0 and end of
the records, and reset again while user scrolling on Data Output table.

QA/Reviewer needs to observe below issues if it occurs:

   1. If records are more like 5000 or 10000, try to fetch records by
   scrolling at least (2000), now save the results by clicking the download
   button, it should save the file, and now try scrolling again, the data
   should be shown continuously and not any abrupt end or unexpected records
   order.
   2. Also, the download button is now "Save results to CSV/TXT" where it
   will only get enabled when there are valid records in the Data Output.


Also a minor fix of the add folder icon issue is also added into this patch.

-- 
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.


Attachments:

  [application/octet-stream] RM5965.patch (20.4K, 3-RM5965.patch)
  download | inline diff:
diff --git a/docs/en_US/query_tool_toolbar.rst b/docs/en_US/query_tool_toolbar.rst
index 2168c3930..a6c2c6ac5 100644
--- a/docs/en_US/query_tool_toolbar.rst
+++ b/docs/en_US/query_tool_toolbar.rst
@@ -179,10 +179,10 @@ Query Execution
    |                      |                                                                                                   |                |
    |                      |  * Select *Clear History* to erase the content of the *History* tab.                              |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
-   | *Download as CSV/TXT*| Click the *Download as CSV/TXT* icon to download the result set of the current query as a *.csv*  | F8             |
-   |                      | or as a *.txt* file. if *CSV field seperator* set to comma(,) else as a *.txt* file.              |                |
-   |                      | You can specify the CSV/TXT settings through *Preferences -> SQL Editor -> CSV/TXT output*        |                |
-   |                      | dialogue.                                                                                         |                |
+   | *Save results to*    | Click the *Save results to file as CSV/TXT* icon to download the result set of the current query  | F8             |
+   |  *file as CSV/TXT*   | as a *.csv* or as a *.txt* file. if *CSV field seperator* set to comma(,) else as a *.txt* file.  |                |
+   |                      | Button will only be enabled when there are results to save. You can specify the CSV/TXT settings  |                |
+   |                      | through *Preferences -> SQL Editor -> CSV/TXT output* dialogue.                                   |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
    | *Macros*             | Click the *Macros* icon to manage the macros. You can create, edit or clear the macros through    |                |
    |                      | the *Manage Macros* option.                                                                       |                |
diff --git a/web/pgadmin/misc/file_manager/static/scss/_file_manager.scss b/web/pgadmin/misc/file_manager/static/scss/_file_manager.scss
index d2682f686..4d15c348c 100644
--- a/web/pgadmin/misc/file_manager/static/scss/_file_manager.scss
+++ b/web/pgadmin/misc/file_manager/static/scss/_file_manager.scss
@@ -333,7 +333,7 @@
   top: -8px;
   left: -6px;
   font-size: 8px;
-  margin-right: -8px;
+  margin-right: -7px;
 }
 
 table.tablesorter {
diff --git a/web/pgadmin/static/js/keyboard_shortcuts.js b/web/pgadmin/static/js/keyboard_shortcuts.js
index abda8d8f2..059df0628 100644
--- a/web/pgadmin/static/js/keyboard_shortcuts.js
+++ b/web/pgadmin/static/js/keyboard_shortcuts.js
@@ -211,8 +211,10 @@ function keyboardShortcutsQueryTool(
     this._stopEventPropagation(event);
     queryToolActions.explainAnalyze(sqlEditorController);
   } else if (this.validateShortcutKeys(downloadCsvKeys, event)) {
-    this._stopEventPropagation(event);
-    queryToolActions.download(sqlEditorController);
+    if(!sqlEditorController.is_save_results_to_file_disabled) {
+      this._stopEventPropagation(event);
+      queryToolActions.download(sqlEditorController);
+    }
   } else if (this.validateShortcutKeys(toggleCaseKeys, event)) {
     this._stopEventPropagation(event);
     queryToolActions.toggleCaseOfSelectedText(sqlEditorController);
diff --git a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
index d8d364722..2c52baf10 100644
--- a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
+++ b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
@@ -43,6 +43,7 @@ export function callRenderAfterPoll(sqlEditor, alertify, res) {
     if (isNotificationEnabled(sqlEditor)) {
       alertify.success(msg, sqlEditor.info_notifier_timeout);
     }
+    sqlEditor.enable_disable_download_btn(true);
   }
 
   if (isQueryTool(sqlEditor)) {
diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js
index 6c4069d24..389655fa9 100644
--- a/web/pgadmin/static/js/sqleditor/execute_query.js
+++ b/web/pgadmin/static/js/sqleditor/execute_query.js
@@ -83,6 +83,7 @@ class ExecuteQuery {
         } else {
           self.loadingScreen.hide();
           self.enableSQLEditorButtons();
+          self.disableDownloadButton();
           // Enable/Disable commit and rollback button.
           if (result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INTRANS
             || result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INERROR) {
@@ -201,7 +202,7 @@ class ExecuteQuery {
     this.loadingScreen.show(gettext('Running query...'));
 
     $('#btn-flash').prop('disabled', true);
-    $('#btn-download').prop('disabled', true);
+    this.disableDownloadButton();
 
     this.sqlServerObject.query_start_time = new Date();
     if (typeof sqlStatement === 'object') {
@@ -281,6 +282,10 @@ class ExecuteQuery {
     }
   }
 
+  disableDownloadButton() {
+    this.sqlServerObject.enable_disable_download_btn(true)
+  }
+
   enableSQLEditorButtons() {
     this.sqlServerObject.disable_tool_buttons(false);
   }
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_actions.js b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
index 81937058c..a3e16bdff 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_actions.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
@@ -81,13 +81,7 @@ let queryToolActions = {
   },
 
   download: function (sqlEditorController) {
-    let sqlQuery = sqlEditorController.gridView.query_tool_obj.getSelection();
 
-    if (!sqlQuery) {
-      sqlQuery = sqlEditorController.gridView.query_tool_obj.getValue();
-    }
-
-    if (!sqlQuery) return;
     let extension = sqlEditorController.preferences.csv_field_separator === ',' ? '.csv': '.txt';
     let filename = 'data-' + new Date().getTime() + extension;
 
@@ -95,7 +89,7 @@ let queryToolActions = {
       filename = sqlEditorController.table_name + extension;
     }
 
-    sqlEditorController.trigger_csv_download(sqlQuery, filename);
+    sqlEditorController.trigger_csv_download(filename);
   },
 
   commentBlockCode: function (sqlEditorController) {
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
index ba4e7c882..e2d4a65ca 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
@@ -112,11 +112,11 @@ function updateUIPreferences(sqlEditor) {
     .attr('aria-label',
       shortcut_title(gettext('Explain Analyze'),preferences.explain_analyze_query));
 
-  $el.find('#btn-download')
+  $el.find('#btn-save-results-to-file')
     .attr('title',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv))
+      shortcut_title(gettext('Save results to file as CSV/TXT'),preferences.download_csv))
     .attr('aria-label',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv));
+      shortcut_title(gettext('Save results to file as CSV/TXT'),preferences.download_csv));
 
   $el.find('#btn-save-data')
     .attr('title',
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index cbee98222..d9d0d2254 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -374,9 +374,9 @@
                 </ul>
             </div>
             <div class="btn-group" role="group" aria-label="">
-                <button id="btn-download" type="button" class="btn btn-sm btn-primary-icon"
+                <button id="btn-save-results-to-file" type="button" class="btn btn-sm btn-primary-icon"
                         title=""
-                        tabindex="0">
+                        tabindex="0" disabled>
                     <i class="fa fa-download sql-icon-lg" aria-hidden="true" role="img"></i>
                 </button>
             </div>
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 0784c5468..a9f5627c6 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -1336,7 +1336,7 @@ def start_query_download_tool(trans_id):
         )
 
     data = request.values if request.values else None
-    if data is None or (data and 'query' not in data):
+    if data is None:
         return make_json_response(
             status=410,
             success=0,
@@ -1346,12 +1346,9 @@ def start_query_download_tool(trans_id):
         )
 
     try:
-        sql = data['query']
 
         # This returns generator of records.
-        status, gen = sync_conn.execute_on_server_as_csv(
-            sql, records=2000
-        )
+        status, gen = sync_conn.execute_on_server_as_csv(records=2000)
 
         if not status:
             return make_json_response(
@@ -1362,6 +1359,7 @@ def start_query_download_tool(trans_id):
 
         r = Response(
             gen(
+                trans_obj,
                 quote=blueprint.csv_quoting.get(),
                 quote_char=blueprint.csv_quote_char.get(),
                 field_separator=blueprint.csv_field_separator.get(),
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 00791fb2c..2f0b2c992 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -136,7 +136,7 @@ define('tools.querytool', [
       'click #btn-flash': 'on_flash',
       'click #btn-flash-menu': 'on_flash',
       'click #btn-cancel-query': 'on_cancel_query',
-      'click #btn-download': 'on_download',
+      'click #btn-save-results-to-file': 'on_download',
       'click #btn-clear': 'on_clear',
       'click #btn-auto-commit': 'on_auto_commit',
       'click #btn-auto-rollback': 'on_auto_rollback',
@@ -1358,7 +1358,7 @@ define('tools.querytool', [
       self.handler.fetching_rows = true;
 
       $('#btn-flash').prop('disabled', true);
-      $('#btn-download').prop('disabled', true);
+      self.enable_disable_download_btn(true);
 
       if (fetch_all) {
         self.handler.trigger(
@@ -1382,7 +1382,7 @@ define('tools.querytool', [
         .done(function(res) {
           self.handler.has_more_rows = res.data.has_more_rows;
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.update_grid_data(res.data.result);
           self.handler.fetching_rows = false;
@@ -1392,7 +1392,7 @@ define('tools.querytool', [
         })
         .fail(function(e) {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.handler.has_more_rows = false;
           self.handler.fetching_rows = false;
@@ -2534,6 +2534,7 @@ define('tools.querytool', [
         self.server_type = url_params.server_type;
         self.url_params = url_params;
         self.is_transaction_buttons_disabled = true;
+        self.is_save_results_to_file_disabled = true;
 
         // We do not allow to call the start multiple times.
         if (self.gridView)
@@ -2783,7 +2784,7 @@ define('tools.querytool', [
         );
 
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
 
         self.trigger(
           'pgadmin-sqleditor:loading-icon:message',
@@ -3058,7 +3059,12 @@ define('tools.querytool', [
             // Hide the loading icon
             self_col.trigger('pgadmin-sqleditor:loading-icon:hide');
             $('#btn-flash').prop('disabled', false);
-            $('#btn-download').prop('disabled', false);
+            if (!_.isUndefined(data) && Array.isArray(data.result) && data.result.length > 0) {
+              self.enable_disable_download_btn(false);
+            }
+            else {
+              self.enable_disable_download_btn(true);
+            }
           }.bind(self)
         );
       },
@@ -3239,7 +3245,6 @@ define('tools.querytool', [
 
         if (status != 'Busy') {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
           if(!self.total_time) {
@@ -3301,6 +3306,12 @@ define('tools.querytool', [
         return (self.get('can_edit'));
       },
 
+      enable_disable_download_btn: function(is_save_results_to_file_disabled) {
+        var self = this;
+        self.is_save_results_to_file_disabled = is_save_results_to_file_disabled;
+        $('#btn-save-results-to-file').prop('disabled', is_save_results_to_file_disabled);
+      },
+
       rows_to_delete: function(data) {
         let self = this;
         let tmp_keys = self.primary_keys;
@@ -4371,7 +4382,6 @@ define('tools.querytool', [
             if(!_.isUndefined(self.download_csv_obj)) {
               self.download_csv_obj.abort();
               $('#btn-flash').prop('disabled', false);
-              $('#btn-download').prop('disabled', false);
               self.trigger(
                 'pgadmin-sqleditor:loading-icon:hide');
             }
@@ -4389,16 +4399,16 @@ define('tools.querytool', [
       },
 
       // Trigger query result download to csv.
-      trigger_csv_download: function(query, filename) {
+      trigger_csv_download: function(filename) {
         var self = this,
           url = url_for('sqleditor.query_tool_download', {
             'trans_id': self.transId,
           }),
-          data = { query: query, filename: filename };
+          data = { filename: filename };
 
         // Disable the Execute button
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
         self.disable_tool_buttons(true);
         self.set_sql_message('');
         self.trigger(
@@ -4443,14 +4453,14 @@ define('tools.querytool', [
 
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
         }).fail(function(err) {
           let msg = '';
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
diff --git a/web/pgadmin/utils/driver/psycopg2/connection.py b/web/pgadmin/utils/driver/psycopg2/connection.py
index 4f40e652b..f7eb957e4 100644
--- a/web/pgadmin/utils/driver/psycopg2/connection.py
+++ b/web/pgadmin/utils/driver/psycopg2/connection.py
@@ -728,10 +728,8 @@ WHERE db.datname = current_database()""")
         if self.async_ == 1:
             self._wait(cur.connection)
 
-    def execute_on_server_as_csv(self,
-                                 query, params=None,
-                                 formatted_exception_msg=False,
-                                 records=2000):
+    def execute_on_server_as_csv(self, params=None,
+                                 formatted_exception_msg=False, records=2000):
         """
         To fetch query result and generate CSV output
 
@@ -743,39 +741,36 @@ WHERE db.datname = current_database()""")
         Returns:
             Generator response
         """
-        status, cur = self.__cursor()
-        self.row_count = 0
+        cur = self.__async_cursor
+        if not cur:
+            return False, self.CURSOR_NOT_FOUND
 
-        if not status:
-            return False, str(cur)
-        query_id = random.randint(1, 9999999)
+        if self.conn.isexecuting():
+            return False, gettext(
+                "Asynchronous query execution/operation underway."
+            )
 
         current_app.logger.log(
             25,
             "Execute (with server cursor) for server #{server_id} - "
-            "{conn_id} (Query-id: {query_id}):\n{query}".format(
-                server_id=self.manager.sid,
-                conn_id=self.conn_id,
-                query=query,
-                query_id=query_id
-            )
+            "{conn_id}".format(server_id=self.manager.sid,
+                               conn_id=self.conn_id)
         )
         try:
             # Unregistering type casting for large size data types.
             unregister_numeric_typecasters(self.conn)
-            self.__internal_blocking_execute(cur, query, params)
+            if self.async_ == 1:
+                self._wait(cur.connection)
         except psycopg2.Error as pe:
             cur.close()
             errmsg = self._formatted_exception_msg(pe, formatted_exception_msg)
             current_app.logger.error(
                 "failed to execute query ((with server cursor) "
                 "for the server #{server_id} - {conn_id} "
-                "(query-id: {query_id}):\n"
-                "error message:{errmsg}".format(
+                "\nerror message:{errmsg}".format(
                     server_id=self.manager.sid,
                     conn_id=self.conn_id,
                     errmsg=errmsg,
-                    query_id=query_id
                 )
             )
             return False, errmsg
@@ -809,13 +804,12 @@ WHERE db.datname = current_database()""")
 
             return results
 
-        def gen(quote='strings', quote_char="'", field_separator=',',
-                replace_nulls_with=None):
+        def gen(trans_obj, quote='strings', quote_char="'",
+                field_separator=',', replace_nulls_with=None):
 
+            cur.scroll(0, mode='absolute')
             results = cur.fetchmany(records)
             if not results:
-                if not cur.closed:
-                    cur.close()
                 yield gettext('The query executed did not return any data.')
                 return
 
@@ -857,8 +851,6 @@ WHERE db.datname = current_database()""")
                 results = cur.fetchmany(records)
 
                 if not results:
-                    if not cur.closed:
-                        cur.close()
                     break
                 res_io = StringIO()
 
@@ -874,9 +866,21 @@ WHERE db.datname = current_database()""")
                     results = handle_null_values(results, replace_nulls_with)
                 csv_writer.writerows(results)
                 yield res_io.getvalue()
+
+            try:
+                # try to reset the cursor scroll back to where it was,
+                # bypass error, if cannot scroll back
+                rows_fetched_from = trans_obj.get_fetched_row_cnt()
+                cur.scroll(rows_fetched_from, mode='absolute')
+            except psycopg2.Error:
+                # bypassing the error as cursor tried to scroll on the specified
+                # position, but end of records found
+                pass
+
         # Registering back type caster for large size data types to string
         # which was unregistered at starting
         register_string_typecasters(self.conn)
+        # self.conn.autocommit = False
         return True, gen
 
     def execute_scalar(self, query, params=None,
@@ -1224,6 +1228,7 @@ WHERE db.datname = current_database()""")
         Args:
           records: no of records to fetch. use -1 to fetchall.
           formatted_exception_msg:
+          for_download: if True, will fetch all records and reset the cursor
 
         Returns:
 
diff --git a/web/pgadmin/utils/driver/psycopg2/cursor.py b/web/pgadmin/utils/driver/psycopg2/cursor.py
index 82ec3d592..b740750b6 100644
--- a/web/pgadmin/utils/driver/psycopg2/cursor.py
+++ b/web/pgadmin/utils/driver/psycopg2/cursor.py
@@ -228,6 +228,13 @@ class DictCursor(_cursor):
         if tuples is not None:
             return [self._dict_tuple(t) for t in tuples]
 
+    def rollback(self):
+        """
+
+        :return:
+        """
+        _cursor.rollback()
+
     def __iter__(self):
         it = _cursor.__iter__(self)
         try:


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-07 09:39  Rahul Shirsat <[email protected]>
  parent: Rahul Shirsat <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Rahul Shirsat @ 2020-12-07 09:39 UTC (permalink / raw)
  To: pgadmin-hackers

Hi Hackers,

Please find the updated patch below.

On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <[email protected]>
wrote:

> Hi Hackers,
>
> Please find the attached patch which resolves the issue of macros query
> results download, have used async cursor to achieve this functionality,
> where for downloading the results, cursor is scrolled back to 0 and end of
> the records, and reset again while user scrolling on Data Output table.
>
> QA/Reviewer needs to observe below issues if it occurs:
>
>    1. If records are more like 5000 or 10000, try to fetch records by
>    scrolling at least (2000), now save the results by clicking the download
>    button, it should save the file, and now try scrolling again, the data
>    should be shown continuously and not any abrupt end or unexpected records
>    order.
>    2. Also, the download button is now "Save results to CSV/TXT" where it
>    will only get enabled when there are valid records in the Data Output.
>
>
> Also a minor fix of the add folder icon issue is also added into this
> patch.
>
> --
> *Rahul Shirsat*
> Senior Software Engineer | EnterpriseDB Corporation.
>


-- 
*Rahul Shirsat*
Software Engineer | EnterpriseDB Corporation.


Attachments:

  [application/octet-stream] RM5965_v2.patch (20.0K, 3-RM5965_v2.patch)
  download | inline diff:
diff --git a/docs/en_US/query_tool_toolbar.rst b/docs/en_US/query_tool_toolbar.rst
index 2168c3930..a6c2c6ac5 100644
--- a/docs/en_US/query_tool_toolbar.rst
+++ b/docs/en_US/query_tool_toolbar.rst
@@ -179,10 +179,10 @@ Query Execution
    |                      |                                                                                                   |                |
    |                      |  * Select *Clear History* to erase the content of the *History* tab.                              |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
-   | *Download as CSV/TXT*| Click the *Download as CSV/TXT* icon to download the result set of the current query as a *.csv*  | F8             |
-   |                      | or as a *.txt* file. if *CSV field seperator* set to comma(,) else as a *.txt* file.              |                |
-   |                      | You can specify the CSV/TXT settings through *Preferences -> SQL Editor -> CSV/TXT output*        |                |
-   |                      | dialogue.                                                                                         |                |
+   | *Save results to*    | Click the *Save results to file as CSV/TXT* icon to download the result set of the current query  | F8             |
+   |  *file as CSV/TXT*   | as a *.csv* or as a *.txt* file. if *CSV field seperator* set to comma(,) else as a *.txt* file.  |                |
+   |                      | Button will only be enabled when there are results to save. You can specify the CSV/TXT settings  |                |
+   |                      | through *Preferences -> SQL Editor -> CSV/TXT output* dialogue.                                   |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
    | *Macros*             | Click the *Macros* icon to manage the macros. You can create, edit or clear the macros through    |                |
    |                      | the *Manage Macros* option.                                                                       |                |
diff --git a/web/pgadmin/static/js/keyboard_shortcuts.js b/web/pgadmin/static/js/keyboard_shortcuts.js
index abda8d8f2..059df0628 100644
--- a/web/pgadmin/static/js/keyboard_shortcuts.js
+++ b/web/pgadmin/static/js/keyboard_shortcuts.js
@@ -211,8 +211,10 @@ function keyboardShortcutsQueryTool(
     this._stopEventPropagation(event);
     queryToolActions.explainAnalyze(sqlEditorController);
   } else if (this.validateShortcutKeys(downloadCsvKeys, event)) {
-    this._stopEventPropagation(event);
-    queryToolActions.download(sqlEditorController);
+    if(!sqlEditorController.is_save_results_to_file_disabled) {
+      this._stopEventPropagation(event);
+      queryToolActions.download(sqlEditorController);
+    }
   } else if (this.validateShortcutKeys(toggleCaseKeys, event)) {
     this._stopEventPropagation(event);
     queryToolActions.toggleCaseOfSelectedText(sqlEditorController);
diff --git a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
index d8d364722..2c52baf10 100644
--- a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
+++ b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
@@ -43,6 +43,7 @@ export function callRenderAfterPoll(sqlEditor, alertify, res) {
     if (isNotificationEnabled(sqlEditor)) {
       alertify.success(msg, sqlEditor.info_notifier_timeout);
     }
+    sqlEditor.enable_disable_download_btn(true);
   }
 
   if (isQueryTool(sqlEditor)) {
diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js
index 6c4069d24..389655fa9 100644
--- a/web/pgadmin/static/js/sqleditor/execute_query.js
+++ b/web/pgadmin/static/js/sqleditor/execute_query.js
@@ -83,6 +83,7 @@ class ExecuteQuery {
         } else {
           self.loadingScreen.hide();
           self.enableSQLEditorButtons();
+          self.disableDownloadButton();
           // Enable/Disable commit and rollback button.
           if (result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INTRANS
             || result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INERROR) {
@@ -201,7 +202,7 @@ class ExecuteQuery {
     this.loadingScreen.show(gettext('Running query...'));
 
     $('#btn-flash').prop('disabled', true);
-    $('#btn-download').prop('disabled', true);
+    this.disableDownloadButton();
 
     this.sqlServerObject.query_start_time = new Date();
     if (typeof sqlStatement === 'object') {
@@ -281,6 +282,10 @@ class ExecuteQuery {
     }
   }
 
+  disableDownloadButton() {
+    this.sqlServerObject.enable_disable_download_btn(true)
+  }
+
   enableSQLEditorButtons() {
     this.sqlServerObject.disable_tool_buttons(false);
   }
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_actions.js b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
index 81937058c..a3e16bdff 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_actions.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
@@ -81,13 +81,7 @@ let queryToolActions = {
   },
 
   download: function (sqlEditorController) {
-    let sqlQuery = sqlEditorController.gridView.query_tool_obj.getSelection();
 
-    if (!sqlQuery) {
-      sqlQuery = sqlEditorController.gridView.query_tool_obj.getValue();
-    }
-
-    if (!sqlQuery) return;
     let extension = sqlEditorController.preferences.csv_field_separator === ',' ? '.csv': '.txt';
     let filename = 'data-' + new Date().getTime() + extension;
 
@@ -95,7 +89,7 @@ let queryToolActions = {
       filename = sqlEditorController.table_name + extension;
     }
 
-    sqlEditorController.trigger_csv_download(sqlQuery, filename);
+    sqlEditorController.trigger_csv_download(filename);
   },
 
   commentBlockCode: function (sqlEditorController) {
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
index ba4e7c882..e2d4a65ca 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
@@ -112,11 +112,11 @@ function updateUIPreferences(sqlEditor) {
     .attr('aria-label',
       shortcut_title(gettext('Explain Analyze'),preferences.explain_analyze_query));
 
-  $el.find('#btn-download')
+  $el.find('#btn-save-results-to-file')
     .attr('title',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv))
+      shortcut_title(gettext('Save results to file as CSV/TXT'),preferences.download_csv))
     .attr('aria-label',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv));
+      shortcut_title(gettext('Save results to file as CSV/TXT'),preferences.download_csv));
 
   $el.find('#btn-save-data')
     .attr('title',
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index cbee98222..d9d0d2254 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -374,9 +374,9 @@
                 </ul>
             </div>
             <div class="btn-group" role="group" aria-label="">
-                <button id="btn-download" type="button" class="btn btn-sm btn-primary-icon"
+                <button id="btn-save-results-to-file" type="button" class="btn btn-sm btn-primary-icon"
                         title=""
-                        tabindex="0">
+                        tabindex="0" disabled>
                     <i class="fa fa-download sql-icon-lg" aria-hidden="true" role="img"></i>
                 </button>
             </div>
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 0784c5468..a9f5627c6 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -1336,7 +1336,7 @@ def start_query_download_tool(trans_id):
         )
 
     data = request.values if request.values else None
-    if data is None or (data and 'query' not in data):
+    if data is None:
         return make_json_response(
             status=410,
             success=0,
@@ -1346,12 +1346,9 @@ def start_query_download_tool(trans_id):
         )
 
     try:
-        sql = data['query']
 
         # This returns generator of records.
-        status, gen = sync_conn.execute_on_server_as_csv(
-            sql, records=2000
-        )
+        status, gen = sync_conn.execute_on_server_as_csv(records=2000)
 
         if not status:
             return make_json_response(
@@ -1362,6 +1359,7 @@ def start_query_download_tool(trans_id):
 
         r = Response(
             gen(
+                trans_obj,
                 quote=blueprint.csv_quoting.get(),
                 quote_char=blueprint.csv_quote_char.get(),
                 field_separator=blueprint.csv_field_separator.get(),
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index c8d3cbcf3..8b80eeefb 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -136,7 +136,7 @@ define('tools.querytool', [
       'click #btn-flash': 'on_flash',
       'click #btn-flash-menu': 'on_flash',
       'click #btn-cancel-query': 'on_cancel_query',
-      'click #btn-download': 'on_download',
+      'click #btn-save-results-to-file': 'on_download',
       'click #btn-clear': 'on_clear',
       'click #btn-auto-commit': 'on_auto_commit',
       'click #btn-auto-rollback': 'on_auto_rollback',
@@ -1358,7 +1358,7 @@ define('tools.querytool', [
       self.handler.fetching_rows = true;
 
       $('#btn-flash').prop('disabled', true);
-      $('#btn-download').prop('disabled', true);
+      self.enable_disable_download_btn(true);
 
       if (fetch_all) {
         self.handler.trigger(
@@ -1382,7 +1382,7 @@ define('tools.querytool', [
         .done(function(res) {
           self.handler.has_more_rows = res.data.has_more_rows;
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.update_grid_data(res.data.result);
           self.handler.fetching_rows = false;
@@ -1392,7 +1392,7 @@ define('tools.querytool', [
         })
         .fail(function(e) {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.handler.has_more_rows = false;
           self.handler.fetching_rows = false;
@@ -2534,6 +2534,7 @@ define('tools.querytool', [
         self.server_type = url_params.server_type;
         self.url_params = url_params;
         self.is_transaction_buttons_disabled = true;
+        self.is_save_results_to_file_disabled = true;
 
         // We do not allow to call the start multiple times.
         if (self.gridView)
@@ -2783,7 +2784,7 @@ define('tools.querytool', [
         );
 
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
 
         self.trigger(
           'pgadmin-sqleditor:loading-icon:message',
@@ -3058,7 +3059,12 @@ define('tools.querytool', [
             // Hide the loading icon
             self_col.trigger('pgadmin-sqleditor:loading-icon:hide');
             $('#btn-flash').prop('disabled', false);
-            $('#btn-download').prop('disabled', false);
+            if (!_.isUndefined(data) && Array.isArray(data.result) && data.result.length > 0) {
+              self.enable_disable_download_btn(false);
+            }
+            else {
+              self.enable_disable_download_btn(true);
+            }
           }.bind(self)
         );
       },
@@ -3239,7 +3245,6 @@ define('tools.querytool', [
 
         if (status != 'Busy') {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
           if(!self.total_time) {
@@ -3301,6 +3306,12 @@ define('tools.querytool', [
         return (self.get('can_edit'));
       },
 
+      enable_disable_download_btn: function(is_save_results_to_file_disabled) {
+        var self = this;
+        self.is_save_results_to_file_disabled = is_save_results_to_file_disabled;
+        $('#btn-save-results-to-file').prop('disabled', is_save_results_to_file_disabled);
+      },
+
       rows_to_delete: function(data) {
         let self = this;
         let tmp_keys = self.primary_keys;
@@ -4376,7 +4387,6 @@ define('tools.querytool', [
             if(!_.isUndefined(self.download_csv_obj)) {
               self.download_csv_obj.abort();
               $('#btn-flash').prop('disabled', false);
-              $('#btn-download').prop('disabled', false);
               self.trigger(
                 'pgadmin-sqleditor:loading-icon:hide');
             }
@@ -4394,16 +4404,16 @@ define('tools.querytool', [
       },
 
       // Trigger query result download to csv.
-      trigger_csv_download: function(query, filename) {
+      trigger_csv_download: function(filename) {
         var self = this,
           url = url_for('sqleditor.query_tool_download', {
             'trans_id': self.transId,
           }),
-          data = { query: query, filename: filename };
+          data = { filename: filename };
 
         // Disable the Execute button
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
         self.disable_tool_buttons(true);
         self.set_sql_message('');
         self.trigger(
@@ -4448,14 +4458,14 @@ define('tools.querytool', [
 
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
         }).fail(function(err) {
           let msg = '';
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
diff --git a/web/pgadmin/utils/driver/psycopg2/connection.py b/web/pgadmin/utils/driver/psycopg2/connection.py
index 4f40e652b..f7eb957e4 100644
--- a/web/pgadmin/utils/driver/psycopg2/connection.py
+++ b/web/pgadmin/utils/driver/psycopg2/connection.py
@@ -728,10 +728,8 @@ WHERE db.datname = current_database()""")
         if self.async_ == 1:
             self._wait(cur.connection)
 
-    def execute_on_server_as_csv(self,
-                                 query, params=None,
-                                 formatted_exception_msg=False,
-                                 records=2000):
+    def execute_on_server_as_csv(self, params=None,
+                                 formatted_exception_msg=False, records=2000):
         """
         To fetch query result and generate CSV output
 
@@ -743,39 +741,36 @@ WHERE db.datname = current_database()""")
         Returns:
             Generator response
         """
-        status, cur = self.__cursor()
-        self.row_count = 0
+        cur = self.__async_cursor
+        if not cur:
+            return False, self.CURSOR_NOT_FOUND
 
-        if not status:
-            return False, str(cur)
-        query_id = random.randint(1, 9999999)
+        if self.conn.isexecuting():
+            return False, gettext(
+                "Asynchronous query execution/operation underway."
+            )
 
         current_app.logger.log(
             25,
             "Execute (with server cursor) for server #{server_id} - "
-            "{conn_id} (Query-id: {query_id}):\n{query}".format(
-                server_id=self.manager.sid,
-                conn_id=self.conn_id,
-                query=query,
-                query_id=query_id
-            )
+            "{conn_id}".format(server_id=self.manager.sid,
+                               conn_id=self.conn_id)
         )
         try:
             # Unregistering type casting for large size data types.
             unregister_numeric_typecasters(self.conn)
-            self.__internal_blocking_execute(cur, query, params)
+            if self.async_ == 1:
+                self._wait(cur.connection)
         except psycopg2.Error as pe:
             cur.close()
             errmsg = self._formatted_exception_msg(pe, formatted_exception_msg)
             current_app.logger.error(
                 "failed to execute query ((with server cursor) "
                 "for the server #{server_id} - {conn_id} "
-                "(query-id: {query_id}):\n"
-                "error message:{errmsg}".format(
+                "\nerror message:{errmsg}".format(
                     server_id=self.manager.sid,
                     conn_id=self.conn_id,
                     errmsg=errmsg,
-                    query_id=query_id
                 )
             )
             return False, errmsg
@@ -809,13 +804,12 @@ WHERE db.datname = current_database()""")
 
             return results
 
-        def gen(quote='strings', quote_char="'", field_separator=',',
-                replace_nulls_with=None):
+        def gen(trans_obj, quote='strings', quote_char="'",
+                field_separator=',', replace_nulls_with=None):
 
+            cur.scroll(0, mode='absolute')
             results = cur.fetchmany(records)
             if not results:
-                if not cur.closed:
-                    cur.close()
                 yield gettext('The query executed did not return any data.')
                 return
 
@@ -857,8 +851,6 @@ WHERE db.datname = current_database()""")
                 results = cur.fetchmany(records)
 
                 if not results:
-                    if not cur.closed:
-                        cur.close()
                     break
                 res_io = StringIO()
 
@@ -874,9 +866,21 @@ WHERE db.datname = current_database()""")
                     results = handle_null_values(results, replace_nulls_with)
                 csv_writer.writerows(results)
                 yield res_io.getvalue()
+
+            try:
+                # try to reset the cursor scroll back to where it was,
+                # bypass error, if cannot scroll back
+                rows_fetched_from = trans_obj.get_fetched_row_cnt()
+                cur.scroll(rows_fetched_from, mode='absolute')
+            except psycopg2.Error:
+                # bypassing the error as cursor tried to scroll on the specified
+                # position, but end of records found
+                pass
+
         # Registering back type caster for large size data types to string
         # which was unregistered at starting
         register_string_typecasters(self.conn)
+        # self.conn.autocommit = False
         return True, gen
 
     def execute_scalar(self, query, params=None,
@@ -1224,6 +1228,7 @@ WHERE db.datname = current_database()""")
         Args:
           records: no of records to fetch. use -1 to fetchall.
           formatted_exception_msg:
+          for_download: if True, will fetch all records and reset the cursor
 
         Returns:
 
diff --git a/web/pgadmin/utils/driver/psycopg2/cursor.py b/web/pgadmin/utils/driver/psycopg2/cursor.py
index 82ec3d592..b740750b6 100644
--- a/web/pgadmin/utils/driver/psycopg2/cursor.py
+++ b/web/pgadmin/utils/driver/psycopg2/cursor.py
@@ -228,6 +228,13 @@ class DictCursor(_cursor):
         if tuples is not None:
             return [self._dict_tuple(t) for t in tuples]
 
+    def rollback(self):
+        """
+
+        :return:
+        """
+        _cursor.rollback()
+
     def __iter__(self):
         it = _cursor.__iter__(self)
         try:


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-09 08:23  Akshay Joshi <[email protected]>
  parent: Rahul Shirsat <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Akshay Joshi @ 2020-12-09 08:23 UTC (permalink / raw)
  To: Aditya Toshniwal <[email protected]>; +Cc: pgadmin-hackers; Rahul Shirsat <[email protected]>

Hi Aditya

Can you please review this patch?

On Mon, Dec 7, 2020 at 3:10 PM Rahul Shirsat <[email protected]>
wrote:

> Hi Hackers,
>
> Please find the updated patch below.
>
> On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <
> [email protected]> wrote:
>
>> Hi Hackers,
>>
>> Please find the attached patch which resolves the issue of macros query
>> results download, have used async cursor to achieve this functionality,
>> where for downloading the results, cursor is scrolled back to 0 and end of
>> the records, and reset again while user scrolling on Data Output table.
>>
>> QA/Reviewer needs to observe below issues if it occurs:
>>
>>    1. If records are more like 5000 or 10000, try to fetch records by
>>    scrolling at least (2000), now save the results by clicking the download
>>    button, it should save the file, and now try scrolling again, the data
>>    should be shown continuously and not any abrupt end or unexpected records
>>    order.
>>    2. Also, the download button is now "Save results to CSV/TXT" where
>>    it will only get enabled when there are valid records in the Data Output.
>>
>>
>> Also a minor fix of the add folder icon issue is also added into this
>> patch.
>>
>> --
>> *Rahul Shirsat*
>> Senior Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> *Rahul Shirsat*
> Software Engineer | EnterpriseDB Corporation.
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-09 09:28  Aditya Toshniwal <[email protected]>
  parent: Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Aditya Toshniwal @ 2020-12-09 09:28 UTC (permalink / raw)
  To: Rahul Shirsat <[email protected]>; +Cc: pgadmin-hackers; Akshay Joshi <[email protected]>

Hello Rahul,

Found below issues:
1) The data grid is not fetching more than 1000 records now, gives a
console error:
sqleditor.js:1264 Uncaught TypeError: self.enable_disable_download_btn is
not a function
    at child.fetch_next (sqleditor.js:1264)

2) The sqleditor test cases are failing.
3) Fix pep8 issues.
4) Fix linter issues.
5) Please check the doc changes again, it's not clear to me.

I didn't check but make sure the GUI tests for the sqleditor runs fine
since the behaviour has changed now.

On Wed, Dec 9, 2020 at 1:53 PM Akshay Joshi <[email protected]>
wrote:

> Hi Aditya
>
> Can you please review this patch?
>
> On Mon, Dec 7, 2020 at 3:10 PM Rahul Shirsat <
> [email protected]> wrote:
>
>> Hi Hackers,
>>
>> Please find the updated patch below.
>>
>> On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <
>> [email protected]> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the attached patch which resolves the issue of macros query
>>> results download, have used async cursor to achieve this functionality,
>>> where for downloading the results, cursor is scrolled back to 0 and end of
>>> the records, and reset again while user scrolling on Data Output table.
>>>
>>> QA/Reviewer needs to observe below issues if it occurs:
>>>
>>>    1. If records are more like 5000 or 10000, try to fetch records by
>>>    scrolling at least (2000), now save the results by clicking the download
>>>    button, it should save the file, and now try scrolling again, the data
>>>    should be shown continuously and not any abrupt end or unexpected records
>>>    order.
>>>    2. Also, the download button is now "Save results to CSV/TXT" where
>>>    it will only get enabled when there are valid records in the Data Output.
>>>
>>>
>>> Also a minor fix of the add folder icon issue is also added into this
>>> patch.
>>>
>>> --
>>> *Rahul Shirsat*
>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>
>>
>>
>> --
>> *Rahul Shirsat*
>> Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>


-- 
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
<http://edbpostgres.com;
"Don't Complain about Heat, Plant a TREE"


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-10 13:24  Rahul Shirsat <[email protected]>
  parent: Aditya Toshniwal <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Rahul Shirsat @ 2020-12-10 13:24 UTC (permalink / raw)
  To: Aditya Toshniwal <[email protected]>; +Cc: pgadmin-hackers; Akshay Joshi <[email protected]>

Hi Aditya/Akshay,

On Wed, Dec 9, 2020 at 2:59 PM Aditya Toshniwal <
[email protected]> wrote:

> Hello Rahul,
>
> Found below issues:
> 1) The data grid is not fetching more than 1000 records now, gives a
> console error:
> sqleditor.js:1264 Uncaught TypeError: self.enable_disable_download_btn is
> not a function
>     at child.fetch_next (sqleditor.js:1264)
>
 *Fixed*

>
> 2) The sqleditor test cases are failing.
>
 *This has been taken care of, and added more test cases. *

> 3) Fix pep8 issues.
> 4) Fix linter issues.
>
 *Sorry for these issues, last minute code changes* 😣

> 5) Please check the doc changes again, it's not clear to me.
>
 *This has been corrected now.*

>
> I didn't check but make sure the GUI tests for the sqleditor runs fine
> since the behaviour has changed now.
>
 *These are fixed now.*

>
> On Wed, Dec 9, 2020 at 1:53 PM Akshay Joshi <[email protected]>
> wrote:
>
>> Hi Aditya
>>
>> Can you please review this patch?
>>
>> On Mon, Dec 7, 2020 at 3:10 PM Rahul Shirsat <
>> [email protected]> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the updated patch below.
>>>
>>> On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <
>>> [email protected]> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the attached patch which resolves the issue of macros query
>>>> results download, have used async cursor to achieve this functionality,
>>>> where for downloading the results, cursor is scrolled back to 0 and end of
>>>> the records, and reset again while user scrolling on Data Output table.
>>>>
>>>> QA/Reviewer needs to observe below issues if it occurs:
>>>>
>>>>    1. If records are more like 5000 or 10000, try to fetch records by
>>>>    scrolling at least (2000), now save the results by clicking the download
>>>>    button, it should save the file, and now try scrolling again, the data
>>>>    should be shown continuously and not any abrupt end or unexpected records
>>>>    order.
>>>>    2. Also, the download button is now "Save results to CSV/TXT" where
>>>>    it will only get enabled when there are valid records in the Data Output.
>>>>
>>>>
>>>> Also a minor fix of the add folder icon issue is also added into this
>>>> patch.
>>>>
>>>> --
>>>> *Rahul Shirsat*
>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>
>>>
>>>
>>> --
>>> *Rahul Shirsat*
>>> Software Engineer | EnterpriseDB Corporation.
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
> <http://edbpostgres.com;
> "Don't Complain about Heat, Plant a TREE"
>


-- 
*Rahul Shirsat*
Software Engineer | EnterpriseDB Corporation.


Attachments:

  [application/octet-stream] RM5965_v3.patch (40.5K, 3-RM5965_v3.patch)
  download | inline diff:
diff --git a/docs/en_US/query_tool.rst b/docs/en_US/query_tool.rst
index 880ca4ffb..2d0530ea4 100644
--- a/docs/en_US/query_tool.rst
+++ b/docs/en_US/query_tool.rst
@@ -122,7 +122,7 @@ You can:
 * Select and copy from the displayed result set.
 * Use the *Execute/Refresh* options to retrieve query execution information and
   set query execution options.
-* Use the *Download as CSV/TXT* icon to download the content of the *Data Output*
+* Use the *Save results to file* icon to save the content of the *Data Output*
   tab as a comma-delimited file.
 * Edit the data in the result set of a SELECT query if it is updatable.
 
diff --git a/docs/en_US/query_tool_toolbar.rst b/docs/en_US/query_tool_toolbar.rst
index 2168c3930..3db3b563e 100644
--- a/docs/en_US/query_tool_toolbar.rst
+++ b/docs/en_US/query_tool_toolbar.rst
@@ -179,10 +179,10 @@ Query Execution
    |                      |                                                                                                   |                |
    |                      |  * Select *Clear History* to erase the content of the *History* tab.                              |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
-   | *Download as CSV/TXT*| Click the *Download as CSV/TXT* icon to download the result set of the current query as a *.csv*  | F8             |
-   |                      | or as a *.txt* file. if *CSV field seperator* set to comma(,) else as a *.txt* file.              |                |
-   |                      | You can specify the CSV/TXT settings through *Preferences -> SQL Editor -> CSV/TXT output*        |                |
-   |                      | dialogue.                                                                                         |                |
+   | *Save results to*    | Click the Save results to file icon to save the result set of the current query as a delimited    | F8             |
+   |  *file*              | text file (CSV, if the field separator is set to a comma). This button will only be enabled when  |                |
+   |                      | a query has been executed and there are results in the data grid. You can specify the CSV/TXT     |                |
+   |                      | settings in the Preference Dialogue under SQL Editor -> CSV/TXT output.                           |                |
    +----------------------+---------------------------------------------------------------------------------------------------+----------------+
    | *Macros*             | Click the *Macros* icon to manage the macros. You can create, edit or clear the macros through    |                |
    |                      | the *Manage Macros* option.                                                                       |                |
diff --git a/web/pgadmin/static/js/keyboard_shortcuts.js b/web/pgadmin/static/js/keyboard_shortcuts.js
index abda8d8f2..0e84aba3e 100644
--- a/web/pgadmin/static/js/keyboard_shortcuts.js
+++ b/web/pgadmin/static/js/keyboard_shortcuts.js
@@ -191,7 +191,7 @@ function keyboardShortcutsQueryTool(
   let executeKeys = sqlEditorController.preferences.execute_query;
   let explainKeys = sqlEditorController.preferences.explain_query;
   let explainAnalyzeKeys = sqlEditorController.preferences.explain_analyze_query;
-  let downloadCsvKeys = sqlEditorController.preferences.download_csv;
+  let downloadCsvKeys = sqlEditorController.preferences.download_results;
   let nextTabKeys = sqlEditorController.preferences.move_next;
   let previousTabKeys = sqlEditorController.preferences.move_previous;
   let switchPanelKeys = sqlEditorController.preferences.switch_panel;
@@ -211,8 +211,10 @@ function keyboardShortcutsQueryTool(
     this._stopEventPropagation(event);
     queryToolActions.explainAnalyze(sqlEditorController);
   } else if (this.validateShortcutKeys(downloadCsvKeys, event)) {
-    this._stopEventPropagation(event);
-    queryToolActions.download(sqlEditorController);
+    if(!sqlEditorController.is_save_results_to_file_disabled) {
+      this._stopEventPropagation(event);
+      queryToolActions.download(sqlEditorController);
+    }
   } else if (this.validateShortcutKeys(toggleCaseKeys, event)) {
     this._stopEventPropagation(event);
     queryToolActions.toggleCaseOfSelectedText(sqlEditorController);
diff --git a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
index d8d364722..2c52baf10 100644
--- a/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
+++ b/web/pgadmin/static/js/sqleditor/call_render_after_poll.js
@@ -43,6 +43,7 @@ export function callRenderAfterPoll(sqlEditor, alertify, res) {
     if (isNotificationEnabled(sqlEditor)) {
       alertify.success(msg, sqlEditor.info_notifier_timeout);
     }
+    sqlEditor.enable_disable_download_btn(true);
   }
 
   if (isQueryTool(sqlEditor)) {
diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js
index 6c4069d24..0135f5bc7 100644
--- a/web/pgadmin/static/js/sqleditor/execute_query.js
+++ b/web/pgadmin/static/js/sqleditor/execute_query.js
@@ -83,6 +83,7 @@ class ExecuteQuery {
         } else {
           self.loadingScreen.hide();
           self.enableSQLEditorButtons();
+          self.disableDownloadButton();
           // Enable/Disable commit and rollback button.
           if (result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INTRANS
             || result.data.data.transaction_status == queryTxnStatus.TRANSACTION_STATUS_INERROR) {
@@ -201,7 +202,7 @@ class ExecuteQuery {
     this.loadingScreen.show(gettext('Running query...'));
 
     $('#btn-flash').prop('disabled', true);
-    $('#btn-download').prop('disabled', true);
+    this.disableDownloadButton();
 
     this.sqlServerObject.query_start_time = new Date();
     if (typeof sqlStatement === 'object') {
@@ -281,6 +282,10 @@ class ExecuteQuery {
     }
   }
 
+  disableDownloadButton() {
+    this.sqlServerObject.enable_disable_download_btn(true);
+  }
+
   enableSQLEditorButtons() {
     this.sqlServerObject.disable_tool_buttons(false);
   }
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_actions.js b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
index 81937058c..a3e16bdff 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_actions.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
@@ -81,13 +81,7 @@ let queryToolActions = {
   },
 
   download: function (sqlEditorController) {
-    let sqlQuery = sqlEditorController.gridView.query_tool_obj.getSelection();
 
-    if (!sqlQuery) {
-      sqlQuery = sqlEditorController.gridView.query_tool_obj.getValue();
-    }
-
-    if (!sqlQuery) return;
     let extension = sqlEditorController.preferences.csv_field_separator === ',' ? '.csv': '.txt';
     let filename = 'data-' + new Date().getTime() + extension;
 
@@ -95,7 +89,7 @@ let queryToolActions = {
       filename = sqlEditorController.table_name + extension;
     }
 
-    sqlEditorController.trigger_csv_download(sqlQuery, filename);
+    sqlEditorController.trigger_csv_download(filename);
   },
 
   commentBlockCode: function (sqlEditorController) {
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
index ba4e7c882..a36328bdd 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_preferences.js
@@ -112,11 +112,11 @@ function updateUIPreferences(sqlEditor) {
     .attr('aria-label',
       shortcut_title(gettext('Explain Analyze'),preferences.explain_analyze_query));
 
-  $el.find('#btn-download')
+  $el.find('#btn-save-results-to-file')
     .attr('title',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv))
+      shortcut_title(gettext('Save results to file'),preferences.download_results))
     .attr('aria-label',
-      shortcut_title(gettext('Download as CSV/TXT'),preferences.download_csv));
+      shortcut_title(gettext('Save results to file'),preferences.download_results));
 
   $el.find('#btn-save-data')
     .attr('title',
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index cbee98222..d9d0d2254 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -374,9 +374,9 @@
                 </ul>
             </div>
             <div class="btn-group" role="group" aria-label="">
-                <button id="btn-download" type="button" class="btn btn-sm btn-primary-icon"
+                <button id="btn-save-results-to-file" type="button" class="btn btn-sm btn-primary-icon"
                         title=""
-                        tabindex="0">
+                        tabindex="0" disabled>
                     <i class="fa fa-download sql-icon-lg" aria-hidden="true" role="img"></i>
                 </button>
             </div>
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 0784c5468..a9f5627c6 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -1336,7 +1336,7 @@ def start_query_download_tool(trans_id):
         )
 
     data = request.values if request.values else None
-    if data is None or (data and 'query' not in data):
+    if data is None:
         return make_json_response(
             status=410,
             success=0,
@@ -1346,12 +1346,9 @@ def start_query_download_tool(trans_id):
         )
 
     try:
-        sql = data['query']
 
         # This returns generator of records.
-        status, gen = sync_conn.execute_on_server_as_csv(
-            sql, records=2000
-        )
+        status, gen = sync_conn.execute_on_server_as_csv(records=2000)
 
         if not status:
             return make_json_response(
@@ -1362,6 +1359,7 @@ def start_query_download_tool(trans_id):
 
         r = Response(
             gen(
+                trans_obj,
                 quote=blueprint.csv_quoting.get(),
                 quote_char=blueprint.csv_quote_char.get(),
                 field_separator=blueprint.csv_field_separator.get(),
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index c8d3cbcf3..522107cab 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -136,7 +136,7 @@ define('tools.querytool', [
       'click #btn-flash': 'on_flash',
       'click #btn-flash-menu': 'on_flash',
       'click #btn-cancel-query': 'on_cancel_query',
-      'click #btn-download': 'on_download',
+      'click #btn-save-results-to-file': 'on_download',
       'click #btn-clear': 'on_clear',
       'click #btn-auto-commit': 'on_auto_commit',
       'click #btn-auto-rollback': 'on_auto_rollback',
@@ -1358,7 +1358,7 @@ define('tools.querytool', [
       self.handler.fetching_rows = true;
 
       $('#btn-flash').prop('disabled', true);
-      $('#btn-download').prop('disabled', true);
+      $('#btn-save-results-to-file').prop('disabled', true);
 
       if (fetch_all) {
         self.handler.trigger(
@@ -1382,7 +1382,7 @@ define('tools.querytool', [
         .done(function(res) {
           self.handler.has_more_rows = res.data.has_more_rows;
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          $('#btn-save-results-to-file').prop('disabled', false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.update_grid_data(res.data.result);
           self.handler.fetching_rows = false;
@@ -1392,7 +1392,7 @@ define('tools.querytool', [
         })
         .fail(function(e) {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          $('#btn-save-results-to-file').prop('disabled', false);
           self.handler.trigger('pgadmin-sqleditor:loading-icon:hide');
           self.handler.has_more_rows = false;
           self.handler.fetching_rows = false;
@@ -2534,6 +2534,7 @@ define('tools.querytool', [
         self.server_type = url_params.server_type;
         self.url_params = url_params;
         self.is_transaction_buttons_disabled = true;
+        self.is_save_results_to_file_disabled = true;
 
         // We do not allow to call the start multiple times.
         if (self.gridView)
@@ -2783,7 +2784,7 @@ define('tools.querytool', [
         );
 
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
 
         self.trigger(
           'pgadmin-sqleditor:loading-icon:message',
@@ -3058,7 +3059,12 @@ define('tools.querytool', [
             // Hide the loading icon
             self_col.trigger('pgadmin-sqleditor:loading-icon:hide');
             $('#btn-flash').prop('disabled', false);
-            $('#btn-download').prop('disabled', false);
+            if (!_.isUndefined(data) && Array.isArray(data.result) && data.result.length > 0) {
+              self.enable_disable_download_btn(false);
+            }
+            else {
+              self.enable_disable_download_btn(true);
+            }
           }.bind(self)
         );
       },
@@ -3239,7 +3245,6 @@ define('tools.querytool', [
 
         if (status != 'Busy') {
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
           if(!self.total_time) {
@@ -3301,6 +3306,12 @@ define('tools.querytool', [
         return (self.get('can_edit'));
       },
 
+      enable_disable_download_btn: function(is_save_results_to_file_disabled) {
+        var self = this;
+        self.is_save_results_to_file_disabled = is_save_results_to_file_disabled;
+        $('#btn-save-results-to-file').prop('disabled', is_save_results_to_file_disabled);
+      },
+
       rows_to_delete: function(data) {
         let self = this;
         let tmp_keys = self.primary_keys;
@@ -4373,10 +4384,9 @@ define('tools.querytool', [
             }
             self.disable_tool_buttons(false);
             is_query_running = false;
-            if(!_.isUndefined(self.download_csv_obj)) {
-              self.download_csv_obj.abort();
+            if(!_.isUndefined(self.download_results_obj)) {
+              self.download_results_obj.abort();
               $('#btn-flash').prop('disabled', false);
-              $('#btn-download').prop('disabled', false);
               self.trigger(
                 'pgadmin-sqleditor:loading-icon:hide');
             }
@@ -4394,25 +4404,25 @@ define('tools.querytool', [
       },
 
       // Trigger query result download to csv.
-      trigger_csv_download: function(query, filename) {
+      trigger_csv_download: function(filename) {
         var self = this,
           url = url_for('sqleditor.query_tool_download', {
             'trans_id': self.transId,
           }),
-          data = { query: query, filename: filename };
+          data = { filename: filename };
 
         // Disable the Execute button
         $('#btn-flash').prop('disabled', true);
-        $('#btn-download').prop('disabled', true);
+        self.enable_disable_download_btn(true);
         self.disable_tool_buttons(true);
         self.set_sql_message('');
         self.trigger(
           'pgadmin-sqleditor:loading-icon:show',
-          gettext('Downloading CSV...')
+          gettext('Downloading Results...')
         );
 
         // Get the CSV file
-        self.download_csv_obj = $.ajax({
+        self.download_results_obj = $.ajax({
           type: 'POST',
           url: url,
           data: data,
@@ -4443,19 +4453,19 @@ define('tools.querytool', [
             }
 
             document.body.removeChild(link);
-            self.download_csv_obj = undefined;
+            self.download_results_obj = undefined;
           }
 
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
         }).fail(function(err) {
           let msg = '';
           // Enable the execute button
           $('#btn-flash').prop('disabled', false);
-          $('#btn-download').prop('disabled', false);
+          self.enable_disable_download_btn(false);
           self.disable_tool_buttons(false);
           self.trigger('pgadmin-sqleditor:loading-icon:hide');
 
diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py
index 37ee5f47e..61d369267 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py
+++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py
@@ -96,7 +96,7 @@ class TestDownloadCSV(BaseTestGenerator):
     ]
 
     def setUp(self):
-        self._db_name = 'download_csv_' + str(random.randint(10000, 65535))
+        self._db_name = 'download_results_' + str(random.randint(10000, 65535))
         self._sid = self.server_information['server_id']
 
         server_con = server_utils.connect_server(self, self._sid)
@@ -105,6 +105,24 @@ class TestDownloadCSV(BaseTestGenerator):
             self.server, self._db_name
         )
 
+    # This method is responsible for initiating query hit at least once,
+    # so that download csv works
+    def initiate_sql_query_tool(self, trans_id, sql_query):
+
+        # This code is to ensure to create a async cursor so that downloading
+        # csv can work.
+        # Start query tool transaction
+        url = '/sqleditor/query_tool/start/{0}'.format(trans_id)
+        response = self.tester.post(url, data=json.dumps({"sql": sql_query}),
+                                    content_type='html/json')
+
+        self.assertEqual(response.status_code, 200)
+
+        # Query tool polling
+        url = '/sqleditor/poll/{0}'.format(trans_id)
+        response = self.tester.get(url)
+        return response
+
     def runTest(self):
 
         db_con = database_utils.connect_database(self,
@@ -121,6 +139,8 @@ class TestDownloadCSV(BaseTestGenerator):
         response = self.tester.post(url)
         self.assertEqual(response.status_code, 200)
 
+        res = self.initiate_sql_query_tool(self.trans_id, self.sql)
+
         # If invalid tx test then make the Tx id invalid so that tests fails
         if not self.is_valid_tx:
             self.trans_id = self.trans_id + '007'
@@ -129,7 +149,11 @@ class TestDownloadCSV(BaseTestGenerator):
         url = self.donwload_url.format(self.trans_id)
         # Disable the console logging from Flask logger
         self.app.logger.disabled = True
-        if self.filename is None:
+        if not self.is_valid and self.is_valid_tx:
+            # When user enters wrong query, poll will throw 500, so expecting
+            # 500, as poll is never called for a wrong query.
+            self.assertEqual(res.status_code, 500)
+        elif self.filename is None:
             if self.download_as_txt:
                 with patch('pgadmin.tools.sqleditor.blueprint.'
                            'csv_field_separator.get', return_value=';'), patch(
diff --git a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py
index 4dc656ad3..e74a5f9c8 100644
--- a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py
+++ b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py
@@ -377,8 +377,8 @@ def register_query_tool_preferences(self):
 
     self.preference.register(
         'keyboard_shortcuts',
-        'download_csv',
-        gettext('Download CSV'),
+        'download_results',
+        gettext('Download Results'),
         'keyboardshortcut',
         {
             'alt': False,
diff --git a/web/pgadmin/utils/driver/psycopg2/connection.py b/web/pgadmin/utils/driver/psycopg2/connection.py
index 4f40e652b..000902cc8 100644
--- a/web/pgadmin/utils/driver/psycopg2/connection.py
+++ b/web/pgadmin/utils/driver/psycopg2/connection.py
@@ -728,27 +728,35 @@ WHERE db.datname = current_database()""")
         if self.async_ == 1:
             self._wait(cur.connection)
 
-    def execute_on_server_as_csv(self,
-                                 query, params=None,
-                                 formatted_exception_msg=False,
-                                 records=2000):
+    def execute_on_server_as_csv(self, params=None,
+                                 formatted_exception_msg=False, records=2000):
         """
         To fetch query result and generate CSV output
 
         Args:
-            query: SQL
             params: Additional parameters
             formatted_exception_msg: For exception
             records: Number of initial records
         Returns:
             Generator response
         """
-        status, cur = self.__cursor()
-        self.row_count = 0
+        cur = self.__async_cursor
+        if not cur:
+            return False, self.CURSOR_NOT_FOUND
 
-        if not status:
-            return False, str(cur)
-        query_id = random.randint(1, 9999999)
+        if self.conn.isexecuting():
+            return False, gettext(
+                "Asynchronous query execution/operation underway."
+            )
+
+        encoding = self.python_encoding
+        query = None
+        try:
+            query = str(cur.query, encoding) \
+                if cur and cur.query is not None else None
+        except Exception:
+            current_app.logger.warning('Error encoding query')
+            pass
 
         current_app.logger.log(
             25,
@@ -757,13 +765,14 @@ WHERE db.datname = current_database()""")
                 server_id=self.manager.sid,
                 conn_id=self.conn_id,
                 query=query,
-                query_id=query_id
+                query_id=self.__async_query_id
             )
         )
         try:
             # Unregistering type casting for large size data types.
             unregister_numeric_typecasters(self.conn)
-            self.__internal_blocking_execute(cur, query, params)
+            if self.async_ == 1:
+                self._wait(cur.connection)
         except psycopg2.Error as pe:
             cur.close()
             errmsg = self._formatted_exception_msg(pe, formatted_exception_msg)
@@ -775,7 +784,7 @@ WHERE db.datname = current_database()""")
                     server_id=self.manager.sid,
                     conn_id=self.conn_id,
                     errmsg=errmsg,
-                    query_id=query_id
+                    query_id=self.__async_query_id
                 )
             )
             return False, errmsg
@@ -809,13 +818,12 @@ WHERE db.datname = current_database()""")
 
             return results
 
-        def gen(quote='strings', quote_char="'", field_separator=',',
-                replace_nulls_with=None):
+        def gen(trans_obj, quote='strings', quote_char="'",
+                field_separator=',', replace_nulls_with=None):
 
+            cur.scroll(0, mode='absolute')
             results = cur.fetchmany(records)
             if not results:
-                if not cur.closed:
-                    cur.close()
                 yield gettext('The query executed did not return any data.')
                 return
 
@@ -857,8 +865,6 @@ WHERE db.datname = current_database()""")
                 results = cur.fetchmany(records)
 
                 if not results:
-                    if not cur.closed:
-                        cur.close()
                     break
                 res_io = StringIO()
 
@@ -874,6 +880,17 @@ WHERE db.datname = current_database()""")
                     results = handle_null_values(results, replace_nulls_with)
                 csv_writer.writerows(results)
                 yield res_io.getvalue()
+
+            try:
+                # try to reset the cursor scroll back to where it was,
+                # bypass error, if cannot scroll back
+                rows_fetched_from = trans_obj.get_fetched_row_cnt()
+                cur.scroll(rows_fetched_from, mode='absolute')
+            except psycopg2.Error:
+                # bypassing the error as cursor tried to scroll on the
+                # specified position, but end of records found
+                pass
+
         # Registering back type caster for large size data types to string
         # which was unregistered at starting
         register_string_typecasters(self.conn)
@@ -1224,6 +1241,7 @@ WHERE db.datname = current_database()""")
         Args:
           records: no of records to fetch. use -1 to fetchall.
           formatted_exception_msg:
+          for_download: if True, will fetch all records and reset the cursor
 
         Returns:
 
diff --git a/web/regression/javascript/sqleditor/call_render_after_poll_spec.js b/web/regression/javascript/sqleditor/call_render_after_poll_spec.js
index 18aeceac0..c7b2186a8 100644
--- a/web/regression/javascript/sqleditor/call_render_after_poll_spec.js
+++ b/web/regression/javascript/sqleditor/call_render_after_poll_spec.js
@@ -24,6 +24,7 @@ describe('#callRenderAfterPoll', () => {
       disable_tool_buttons: jasmine.createSpy('SQLEditor.disable_tool_buttons'),
       disable_transaction_buttons: jasmine.createSpy('SQLEditor.disable_transaction_buttons'),
       reset_data_store: jasmine.createSpy('SQLEditor.reset_data_store'),
+      enable_disable_download_btn: jasmine.createSpy('SQLEditor.enable_disable_download_btn'),
       query_start_time: new Date(),
     };
     alertify = jasmine.createSpyObj('alertify', ['success']);
@@ -115,6 +116,14 @@ describe('#callRenderAfterPoll', () => {
           );
         });
       });
+
+      it('disables the save results button', () => {
+        callRenderAfterPoll(sqlEditorSpy, alertify, queryResult);
+
+        expect(sqlEditorSpy.enable_disable_download_btn).toHaveBeenCalledWith(true);
+
+        expect(sqlEditorSpy.trigger).toHaveBeenCalledWith('pgadmin-sqleditor:loading-icon:hide');
+      });
     });
   });
 
@@ -212,6 +221,14 @@ describe('#callRenderAfterPoll', () => {
           );
         });
       });
+
+      it('disables the save results button', () => {
+        callRenderAfterPoll(sqlEditorSpy, alertify, queryResult);
+
+        expect(sqlEditorSpy.enable_disable_download_btn).toHaveBeenCalledWith(true);
+
+        expect(sqlEditorSpy.trigger).toHaveBeenCalledWith('pgadmin-sqleditor:loading-icon:hide');
+      });
     });
   });
 });
diff --git a/web/regression/javascript/sqleditor/execute_query_spec.js b/web/regression/javascript/sqleditor/execute_query_spec.js
index dd37342c9..c561bb610 100644
--- a/web/regression/javascript/sqleditor/execute_query_spec.js
+++ b/web/regression/javascript/sqleditor/execute_query_spec.js
@@ -45,6 +45,7 @@ describe('ExecuteQuery', () => {
       'handle_connection_lost',
       'update_notifications',
       'disable_transaction_buttons',
+      'enable_disable_download_btn',
     ]);
     sqlEditorMock.transId = 123;
     sqlEditorMock.rows_affected = 1000;
diff --git a/web/regression/javascript/sqleditor/keyboard_shortcuts_spec.js b/web/regression/javascript/sqleditor/keyboard_shortcuts_spec.js
index 04365bf48..358a7116c 100644
--- a/web/regression/javascript/sqleditor/keyboard_shortcuts_spec.js
+++ b/web/regression/javascript/sqleditor/keyboard_shortcuts_spec.js
@@ -71,7 +71,7 @@ describe('the keyboard shortcuts', () => {
           key_code: F7_KEY,
         },
       },
-      download_csv: {
+      download_results: {
         alt: false,
         shift: false,
         control: false,
diff --git a/web/regression/javascript/sqleditor/query_tool_actions_spec.js b/web/regression/javascript/sqleditor/query_tool_actions_spec.js
index 15021c463..66087bbd3 100644
--- a/web/regression/javascript/sqleditor/query_tool_actions_spec.js
+++ b/web/regression/javascript/sqleditor/query_tool_actions_spec.js
@@ -277,7 +277,7 @@ describe('queryToolActions', () => {
       it('does nothing', () => {
         queryToolActions.download(sqlEditorController);
 
-        expect(sqlEditorController.trigger_csv_download).not.toHaveBeenCalled();
+        expect(sqlEditorController.trigger_csv_download).toHaveBeenCalled();
       });
     });
 
@@ -298,21 +298,21 @@ describe('queryToolActions', () => {
           }));
         });
 
-        it('calls trigger_csv_download with the query and the filename with .csv extension', () => {
+        it('calls trigger_csv_download with filename having .csv extension', () => {
           let filename = 'data-' + time + '.csv';
 
           queryToolActions.download(sqlEditorController);
 
-          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(selectedQueryString, filename);
+          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(filename);
         });
 
-        it('calls trigger_csv_download with the query and the filename with .txt extension', () => {
+        it('calls trigger_csv_download filename having .txt extension', () => {
           sqlEditorController.preferences.csv_field_separator = ';';
           let filename = 'data-' + time + '.txt';
 
           queryToolActions.download(sqlEditorController);
 
-          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(selectedQueryString, filename);
+          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(filename);
         });
       });
 
@@ -333,12 +333,12 @@ describe('queryToolActions', () => {
           }));
         });
 
-        it('calls trigger_csv_download with the query and the filename', () => {
+        it('calls trigger_csv_download with filename', () => {
           let filename = 'data-' + time + '.csv';
 
           queryToolActions.download(sqlEditorController);
 
-          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(entireQueryString, filename);
+          expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(filename);
         });
       });
     });
@@ -351,7 +351,7 @@ describe('queryToolActions', () => {
 
         queryToolActions.download(sqlEditorController);
 
-        expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith(query, 'iAmATable' + '.csv');
+        expect(sqlEditorController.trigger_csv_download).toHaveBeenCalledWith('iAmATable' + '.csv');
       });
     });
 
diff --git a/web/regression/javascript/sqleditor/query_tool_preferences.js b/web/regression/javascript/sqleditor/query_tool_preferences.js
new file mode 100644
index 000000000..a36328bdd
--- /dev/null
+++ b/web/regression/javascript/sqleditor/query_tool_preferences.js
@@ -0,0 +1,241 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2020, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {shortcut_key, shortcut_accesskey_title, shortcut_title}
+  from 'sources/keyboard_shortcuts';
+import * as SqlEditorUtils from 'sources/sqleditor_utils';
+import $ from 'jquery';
+import gettext from 'sources/gettext';
+
+function updateUIPreferences(sqlEditor) {
+  let $el = sqlEditor.$el,
+    preferences = sqlEditor.preferences;
+
+  if(sqlEditor.handler.slickgrid) {
+    sqlEditor.handler.slickgrid.CSVOptions = {
+      quoting: sqlEditor.preferences.results_grid_quoting,
+      quote_char: sqlEditor.preferences.results_grid_quote_char,
+      field_separator: sqlEditor.preferences.results_grid_field_separator,
+    };
+  }
+
+  /* Accessed using accesskey direct w/o ctrl,atl,shift */
+  $el.find('#btn-load-file')
+    .attr('title', shortcut_accesskey_title(gettext('Open File'),preferences.btn_open_file))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Open File'),preferences.btn_open_file))
+    .attr('accesskey', shortcut_key(preferences.btn_open_file));
+
+  $el.find('#btn-save-file')
+    .attr('title', shortcut_accesskey_title(gettext('Save File'),preferences.btn_save_file))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Save File'),preferences.btn_save_file))
+    .attr('accesskey', shortcut_key(preferences.btn_save_file));
+
+  $el.find('#btn-find-menu-dropdown')
+    .attr('title', shortcut_accesskey_title(gettext('Find'),preferences.btn_find_options))
+    .attr('aria-label',shortcut_accesskey_title(gettext('Find'),preferences.btn_find_options))
+    .attr('accesskey', shortcut_key(preferences.btn_find_options));
+
+  $el.find('#btn-copy-row')
+    .attr('title', shortcut_accesskey_title(gettext('Copy'),preferences.btn_copy_row))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Copy'),preferences.btn_copy_row))
+    .attr('accesskey', shortcut_key(preferences.btn_copy_row));
+
+  $el.find('#btn-paste-row')
+    .attr('title', shortcut_accesskey_title(gettext('Paste'),preferences.btn_paste_row))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Paste'),preferences.btn_paste_row))
+    .attr('accesskey', shortcut_key(preferences.btn_paste_row));
+
+  $el.find('#btn-delete-row')
+    .attr('title', shortcut_accesskey_title(gettext('Delete'),preferences.btn_delete_row))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Delete'),preferences.btn_delete_row))
+    .attr('accesskey', shortcut_key(preferences.btn_delete_row));
+
+  $el.find('#btn-filter')
+    .attr('title', shortcut_accesskey_title(gettext('Filter'),preferences.btn_filter_dialog))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Filter'),preferences.btn_filter_dialog))
+    .attr('accesskey', shortcut_key(preferences.btn_filter_dialog));
+
+  $el.find('#btn-filter-dropdown')
+    .attr('title', shortcut_accesskey_title(gettext('Filter options'),preferences.btn_filter_options))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Filter options'),preferences.btn_filter_options))
+    .attr('accesskey', shortcut_key(preferences.btn_filter_options));
+
+  $el.find('#btn-rows-limit')
+    .attr('title', shortcut_accesskey_title(gettext('Rows limit'),preferences.btn_rows_limit))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Rows limit'),preferences.btn_rows_limit))
+    .attr('accesskey', shortcut_key(preferences.btn_rows_limit));
+
+  $el.find('#btn-query-dropdown')
+    .attr('title', shortcut_accesskey_title(gettext('Execute options'),preferences.btn_execute_options))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Execute options'),preferences.btn_execute_options))
+    .attr('accesskey', shortcut_key(preferences.btn_execute_options));
+
+  $el.find('#btn-cancel-query')
+    .attr('title', shortcut_accesskey_title(gettext('Cancel query'),preferences.btn_cancel_query))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Cancel query'),preferences.btn_cancel_query))
+    .attr('accesskey', shortcut_key(preferences.btn_cancel_query));
+
+  $el.find('#btn-clear-dropdown')
+    .attr('title', shortcut_accesskey_title(gettext('Clear'),preferences.btn_clear_options))
+    .attr('aria-label', shortcut_accesskey_title(gettext('Clear'),preferences.btn_clear_options))
+    .attr('accesskey', shortcut_key(preferences.btn_clear_options));
+
+  $el.find('#btn-conn-status')
+    .attr('accesskey', shortcut_key(preferences.btn_conn_status))
+    .find('i')
+    .attr('title',
+      shortcut_accesskey_title(gettext('Connection status (click for details)'),
+        preferences.btn_conn_status));
+
+  /* Accessed using ctrl,atl,shift and key */
+  $el.find('#btn-flash')
+    .attr('title',
+      shortcut_title(gettext('Execute/Refresh'),preferences.execute_query))
+    .attr('aria-label',
+      shortcut_title(gettext('Execute/Refresh'),preferences.execute_query));
+
+  $el.find('#btn-explain')
+    .attr('title',
+      shortcut_title(gettext('Explain'),preferences.explain_query))
+    .attr('aria-label',
+      shortcut_title(gettext('Explain'),preferences.explain_query));
+
+  $el.find('#btn-explain-analyze')
+    .attr('title',
+      shortcut_title(gettext('Explain Analyze'),preferences.explain_analyze_query))
+    .attr('aria-label',
+      shortcut_title(gettext('Explain Analyze'),preferences.explain_analyze_query));
+
+  $el.find('#btn-save-results-to-file')
+    .attr('title',
+      shortcut_title(gettext('Save results to file'),preferences.download_results))
+    .attr('aria-label',
+      shortcut_title(gettext('Save results to file'),preferences.download_results));
+
+  $el.find('#btn-save-data')
+    .attr('title',
+      shortcut_title(gettext('Save Data Changes'),preferences.save_data))
+    .attr('aria-label',
+      shortcut_title(gettext('Save Data Changes'),preferences.save_data));
+
+  $el.find('#btn-commit')
+    .attr('title',
+      shortcut_title(gettext('Commit'),preferences.commit_transaction))
+    .attr('aria-label',
+      shortcut_title(gettext('Commit'),preferences.commit_transaction));
+
+  $el.find('#btn-rollback')
+    .attr('title',
+      shortcut_title(gettext('Rollback'),preferences.rollback_transaction))
+    .attr('aria-label',
+      shortcut_title(gettext('Rollback'),preferences.rollback_transaction));
+
+  $el.find('#btn-show-query-tool')
+    .attr('title',
+      shortcut_title(gettext('Query tool'),preferences.show_query_tool))
+    .attr('aria-label',
+      shortcut_title(gettext('Query tool'),preferences.show_query_tool));
+
+  /* Set explain options on query editor */
+  if (preferences.explain_verbose){
+    $el.find('.explain-verbose').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-verbose').addClass('visibility-hidden');
+  }
+
+  if (preferences.explain_costs){
+    $el.find('.explain-costs').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-costs').addClass('visibility-hidden');
+  }
+
+  if (preferences.explain_buffers){
+    $el.find('.explain-buffers').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-buffers').addClass('visibility-hidden');
+  }
+
+  if (preferences.explain_timing) {
+    $el.find('.explain-timing').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-timing').addClass('visibility-hidden');
+  }
+
+  if (preferences.explain_summary) {
+    $el.find('.explain-summary').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-summary').addClass('visibility-hidden');
+  }
+
+  if (preferences.explain_settings) {
+    $el.find('.explain-settings').removeClass('visibility-hidden');
+  }
+  else {
+    $el.find('.explain-settings').addClass('visibility-hidden');
+  }
+
+  /* Connection status check */
+  /* remove the status checker if present */
+  if(sqlEditor.connIntervalId != null) {
+    clearInterval(sqlEditor.connIntervalId);
+    sqlEditor.connIntervalId = null;
+  }
+  if (preferences.connection_status) {
+    let $conn_status = $el.find('#btn-conn-status'),
+      $status_el = $conn_status.find('i');
+    $conn_status.popover();
+
+    $conn_status.removeClass('connection-status-hide');
+
+    // To set initial connection
+    SqlEditorUtils.fetchConnectionStatus(sqlEditor.handler, $conn_status, $status_el);
+
+    // Calling it again in specified interval
+    sqlEditor.connIntervalId =  setInterval(
+      SqlEditorUtils.fetchConnectionStatus.bind(null, sqlEditor.handler, $conn_status, $status_el),
+      preferences.connection_status_fetch_time * 1000
+    );
+  }
+  else {
+    $el.find('#btn-conn-status').addClass('connection-status-hide');
+  }
+
+  /* Code Mirror Preferences */
+  let sql_font_size = SqlEditorUtils.calcFontSize(preferences.sql_font_size);
+  $(sqlEditor.query_tool_obj.getWrapperElement()).css('font-size', sql_font_size);
+
+  if(preferences.plain_editor_mode) {
+    sqlEditor.query_tool_obj.setOption('mode', 'text/plain');
+    /* Although not required, setting explicitly as codemirror will remove code folding only on next edit */
+    sqlEditor.query_tool_obj.setOption('foldGutter', false);
+  } else {
+    sqlEditor.query_tool_obj.setOption('mode', sqlEditor.handler.server_type === 'gpdb' ? 'text/x-gpsql' : 'text/x-pgsql');
+    sqlEditor.query_tool_obj.setOption('foldGutter', preferences.code_folding);
+  }
+  sqlEditor.query_tool_obj.setOption('foldGutter', preferences.code_folding);
+  sqlEditor.query_tool_obj.setOption('indentWithTabs', !preferences.use_spaces);
+  sqlEditor.query_tool_obj.setOption('indentUnit', preferences.tab_size);
+  sqlEditor.query_tool_obj.setOption('tabSize', preferences.tab_size);
+  sqlEditor.query_tool_obj.setOption('lineWrapping', preferences.wrap_code);
+  sqlEditor.query_tool_obj.setOption('autoCloseBrackets', preferences.insert_pair_brackets);
+  sqlEditor.query_tool_obj.setOption('matchBrackets', preferences.brace_matching);
+  sqlEditor.query_tool_obj.refresh();
+
+  /* Render history to reflect Font size change */
+  sqlEditor.historyComponent.setEditorPref({
+    'sql_font_size' : sql_font_size,
+  });
+}
+
+export {updateUIPreferences};


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-14 06:25  Aditya Toshniwal <[email protected]>
  parent: Rahul Shirsat <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Aditya Toshniwal @ 2020-12-14 06:25 UTC (permalink / raw)
  To: Rahul Shirsat <[email protected]>; +Cc: pgadmin-hackers; Akshay Joshi <[email protected]>

Hi Rahul,

The file in the patch -
*web/regression/javascript/sqleditor/query_tool_preferences.js* is a copy
of *web/pgadmin/static/js/sqleditor/query_tool_preferences.js* and is not a
test case file.

@committer - Please remove the file
*web/regression/javascript/sqleditor/query_tool_preferences.js* before
committing.
Apart from this, the patch looks good to me.


On Thu, Dec 10, 2020 at 6:55 PM Rahul Shirsat <
[email protected]> wrote:

> Hi Aditya/Akshay,
>
> On Wed, Dec 9, 2020 at 2:59 PM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hello Rahul,
>>
>> Found below issues:
>> 1) The data grid is not fetching more than 1000 records now, gives a
>> console error:
>> sqleditor.js:1264 Uncaught TypeError: self.enable_disable_download_btn is
>> not a function
>>     at child.fetch_next (sqleditor.js:1264)
>>
>  *Fixed*
>
>>
>> 2) The sqleditor test cases are failing.
>>
>  *This has been taken care of, and added more test cases. *
>
>> 3) Fix pep8 issues.
>> 4) Fix linter issues.
>>
>  *Sorry for these issues, last minute code changes* 😣
>
>> 5) Please check the doc changes again, it's not clear to me.
>>
>  *This has been corrected now.*
>
>>
>> I didn't check but make sure the GUI tests for the sqleditor runs fine
>> since the behaviour has changed now.
>>
>  *These are fixed now.*
>
>>
>> On Wed, Dec 9, 2020 at 1:53 PM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Aditya
>>>
>>> Can you please review this patch?
>>>
>>> On Mon, Dec 7, 2020 at 3:10 PM Rahul Shirsat <
>>> [email protected]> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the updated patch below.
>>>>
>>>> On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please find the attached patch which resolves the issue of macros
>>>>> query results download, have used async cursor to achieve this
>>>>> functionality, where for downloading the results, cursor is scrolled back
>>>>> to 0 and end of the records, and reset again while user scrolling on Data
>>>>> Output table.
>>>>>
>>>>> QA/Reviewer needs to observe below issues if it occurs:
>>>>>
>>>>>    1. If records are more like 5000 or 10000, try to fetch records by
>>>>>    scrolling at least (2000), now save the results by clicking the download
>>>>>    button, it should save the file, and now try scrolling again, the data
>>>>>    should be shown continuously and not any abrupt end or unexpected records
>>>>>    order.
>>>>>    2. Also, the download button is now "Save results to CSV/TXT"
>>>>>    where it will only get enabled when there are valid records in the Data
>>>>>    Output.
>>>>>
>>>>>
>>>>> Also a minor fix of the add folder icon issue is also added into this
>>>>> patch.
>>>>>
>>>>> --
>>>>> *Rahul Shirsat*
>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>
>>>>
>>>>
>>>> --
>>>> *Rahul Shirsat*
>>>> Software Engineer | EnterpriseDB Corporation.
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>> *pgAdmin Hacker | Principal Software Architect*
>>> *EDB Postgres <http://edbpostgres.com>*
>>>
>>> *Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>> <http://edbpostgres.com;
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> *Rahul Shirsat*
> Software Engineer | EnterpriseDB Corporation.
>


-- 
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
<http://edbpostgres.com;
"Don't Complain about Heat, Plant a TREE"


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
@ 2020-12-14 07:05  Akshay Joshi <[email protected]>
  parent: Aditya Toshniwal <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Akshay Joshi @ 2020-12-14 07:05 UTC (permalink / raw)
  To: Aditya Toshniwal <[email protected]>; +Cc: Rahul Shirsat <[email protected]>; pgadmin-hackers

Thanks, patch applied.

On Mon, Dec 14, 2020 at 11:55 AM Aditya Toshniwal <
[email protected]> wrote:

> Hi Rahul,
>
> The file in the patch -
> *web/regression/javascript/sqleditor/query_tool_preferences.js* is a copy
> of *web/pgadmin/static/js/sqleditor/query_tool_preferences.js* and is not
> a test case file.
>
> @committer - Please remove the file
> *web/regression/javascript/sqleditor/query_tool_preferences.js* before
> committing.
> Apart from this, the patch looks good to me.
>
>
> On Thu, Dec 10, 2020 at 6:55 PM Rahul Shirsat <
> [email protected]> wrote:
>
>> Hi Aditya/Akshay,
>>
>> On Wed, Dec 9, 2020 at 2:59 PM Aditya Toshniwal <
>> [email protected]> wrote:
>>
>>> Hello Rahul,
>>>
>>> Found below issues:
>>> 1) The data grid is not fetching more than 1000 records now, gives a
>>> console error:
>>> sqleditor.js:1264 Uncaught TypeError: self.enable_disable_download_btn
>>> is not a function
>>>     at child.fetch_next (sqleditor.js:1264)
>>>
>>  *Fixed*
>>
>>>
>>> 2) The sqleditor test cases are failing.
>>>
>>  *This has been taken care of, and added more test cases. *
>>
>>> 3) Fix pep8 issues.
>>> 4) Fix linter issues.
>>>
>>  *Sorry for these issues, last minute code changes* 😣
>>
>>> 5) Please check the doc changes again, it's not clear to me.
>>>
>>  *This has been corrected now.*
>>
>>>
>>> I didn't check but make sure the GUI tests for the sqleditor runs fine
>>> since the behaviour has changed now.
>>>
>>  *These are fixed now.*
>>
>>>
>>> On Wed, Dec 9, 2020 at 1:53 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Aditya
>>>>
>>>> Can you please review this patch?
>>>>
>>>> On Mon, Dec 7, 2020 at 3:10 PM Rahul Shirsat <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please find the updated patch below.
>>>>>
>>>>> On Mon, Dec 7, 2020 at 2:55 PM Rahul Shirsat <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Please find the attached patch which resolves the issue of macros
>>>>>> query results download, have used async cursor to achieve this
>>>>>> functionality, where for downloading the results, cursor is scrolled back
>>>>>> to 0 and end of the records, and reset again while user scrolling on Data
>>>>>> Output table.
>>>>>>
>>>>>> QA/Reviewer needs to observe below issues if it occurs:
>>>>>>
>>>>>>    1. If records are more like 5000 or 10000, try to fetch records
>>>>>>    by scrolling at least (2000), now save the results by clicking the download
>>>>>>    button, it should save the file, and now try scrolling again, the data
>>>>>>    should be shown continuously and not any abrupt end or unexpected records
>>>>>>    order.
>>>>>>    2. Also, the download button is now "Save results to CSV/TXT"
>>>>>>    where it will only get enabled when there are valid records in the Data
>>>>>>    Output.
>>>>>>
>>>>>>
>>>>>> Also a minor fix of the add folder icon issue is also added into this
>>>>>> patch.
>>>>>>
>>>>>> --
>>>>>> *Rahul Shirsat*
>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Rahul Shirsat*
>>>>> Software Engineer | EnterpriseDB Corporation.
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>> <http://edbpostgres.com;
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> *Rahul Shirsat*
>> Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
> <http://edbpostgres.com;
> "Don't Complain about Heat, Plant a TREE"
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*


^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2020-12-14 07:05 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 09:25 [pgAdmin4] RM5965 Couldn't download file of Marcos query results Rahul Shirsat <[email protected]>
2020-12-07 09:39 ` Rahul Shirsat <[email protected]>
2020-12-09 08:23   ` Akshay Joshi <[email protected]>
2020-12-09 09:28     ` Aditya Toshniwal <[email protected]>
2020-12-10 13:24       ` Rahul Shirsat <[email protected]>
2020-12-14 06:25         ` Aditya Toshniwal <[email protected]>
2020-12-14 07:05           ` Akshay Joshi <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox