public inbox for [email protected]
help / color / mirror / Atom feedFrom: Rahul Shirsat <[email protected]>
To: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
Date: Mon, 7 Dec 2020 15:09:53 +0530
Message-ID: <CAKtn9dPWeCXuToAyOSE3Mqdg-b0Qkx3vcRX30nuUrxabY-O-+Q@mail.gmail.com> (raw)
In-Reply-To: <CAKtn9dNGjVVoDCQUo0k_7hF1PKPivDmYTE1+2ds+g5wufNBgtQ@mail.gmail.com>
References: <CAKtn9dNGjVVoDCQUo0k_7hF1PKPivDmYTE1+2ds+g5wufNBgtQ@mail.gmail.com>
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:
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [pgAdmin4] RM5965 Couldn't download file of Marcos query results
In-Reply-To: <CAKtn9dPWeCXuToAyOSE3Mqdg-b0Qkx3vcRX30nuUrxabY-O-+Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox