From 97c3865bf7872405ac12cd5ede0aaa5d056de4bf Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 11:13:27 +0100 Subject: [PATCH 1/7] Add Query Tool result export enhancements. Several long-standing requests around exporting/copying Query Tool results, all in the results download/copy path: - Save results as JSON or XML in addition to CSV, selectable from a drop-down on the "Save results to file" toolbar button. The download generator is now format-aware and streams JSON/XML as well as CSV. - New "Output file encoding" preference (CSV/TXT output) controlling the character encoding of saved results; defaults to utf-8. - New "Add byte order mark (BOM)?" preference that prepends a UTF BOM to saved CSV/TXT files for better interoperability with applications such as Microsoft Excel. - New "Copy with headers?" preference seeding the default state of the results grid "Copy with headers" toggle. Adds integration tests for the JSON/XML/BOM/encoding download paths and updates the preferences and Query Tool toolbar documentation. Closes #3205 Closes #4128 Closes #4129 Closes #6695 --- docs/en_US/preferences.rst | 10 ++ docs/en_US/query_tool_toolbar.rst | 10 +- web/pgadmin/tools/sqleditor/__init__.py | 59 ++++++-- .../js/components/sections/ResultSet.jsx | 14 +- .../components/sections/ResultSetToolbar.jsx | 28 +++- .../tests/test_download_csv_query_tool.py | 130 ++++++++++++++++++ .../sqleditor/utils/query_tool_preferences.py | 38 +++++ .../utils/driver/psycopg3/connection.py | 127 +++++++++++++---- 8 files changed, 364 insertions(+), 52 deletions(-) diff --git a/docs/en_US/preferences.rst b/docs/en_US/preferences.rst index 8342571ab5c..630d2070966 100644 --- a/docs/en_US/preferences.rst +++ b/docs/en_US/preferences.rst @@ -589,6 +589,13 @@ Use the fields on the *CSV/TXT Output* panel to control the CSV/TXT output. quoted in the CSV/TXT output; select *Strings*, *All*, or *None*. * Use the *Replace null values with* option to replace null values with specified string in the output file. Default is set to 'NULL'. +* Use the *Output file encoding* drop-down listbox to specify the character + encoding used when saving query results to a file. The default is utf-8; an + encoding that is not listed can also be typed in. +* Use the *Add byte order mark (BOM)?* switch to add a byte order mark at the + start of the saved file when a UTF encoding is used. This helps applications + such as Microsoft Excel detect the encoding correctly. This applies to the + CSV/TXT output only. .. image:: images/preferences_sql_display.png :alt: Preferences sqleditor display options @@ -754,6 +761,9 @@ preferences for copied data. character for copied data. * Use the *Result copy quoting* drop-down listbox to select which type of fields require quoting; select *All*, *None*, or *Strings*. +* When the *Copy with headers?* switch is set to true, the column headers are + included by default when copying data from the results grid. This can still + be toggled per-copy from the results grid copy options menu. * When the *Striped rows?* switch is set to true, the result grid will display rows with alternating background colors. diff --git a/docs/en_US/query_tool_toolbar.rst b/docs/en_US/query_tool_toolbar.rst index 9b03ca42ac9..848089a5659 100644 --- a/docs/en_US/query_tool_toolbar.rst +++ b/docs/en_US/query_tool_toolbar.rst @@ -210,10 +210,12 @@ Data Editing Options | *Save Data Changes* | Click the *Save Data Changes* icon to save data changes (insert, update, or delete) in the Data | F6 | | | Output Panel to the server. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ - | *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. | | + | *Save results to* | Click the Save results to file icon to save the result set of the current query. By | F8 | + | *file* | default it is saved as a delimited text file (CSV, if the field separator is set to a | | + | | comma). Use the adjacent drop-down list to instead save the results as JSON or XML. | | + | | This button is only enabled when a query has been executed and there are results in | | + | | the data grid. You can specify the CSV/TXT settings (including the output file encoding | | + | | and byte order mark) in the Preferences dialog under Query Tool -> CSV/TXT Output. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ | Graph Visualiser | Use the Graph Visualiser button to generate graphs of the query results. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 8080d220a54..29901272984 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2179,20 +2179,59 @@ def start_query_download_tool(trans_id): } ) + # Output format: csv (default), json or xml. + data_format = (data.get('format') or 'csv').lower() + if data_format not in ('csv', 'json', 'xml'): + data_format = 'csv' + + # Encoding and BOM apply to the CSV/text output only; the structured + # formats are always emitted as UTF-8. + if data_format == 'csv': + output_encoding = blueprint.csv_output_encoding.get() or 'utf-8' + add_bom = blueprint.csv_add_bom.get() + else: + output_encoding = 'utf-8' + add_bom = False + is_utf = output_encoding.lower().replace('-', '').replace( + '_', '').startswith('utf') + + str_gen = gen(conn_obj, + trans_obj, + quote=blueprint.csv_quoting.get(), + quote_char=blueprint.csv_quote_char.get(), + field_separator=blueprint.csv_field_separator.get(), + replace_nulls_with=blueprint.replace_nulls_with.get(), + data_format=data_format) + + def encoded_gen(text_gen): + is_first_chunk = True + for chunk in text_gen: + if is_first_chunk: + is_first_chunk = False + if add_bom and is_utf: + chunk = '\ufeff' + chunk + yield chunk.encode(output_encoding, errors='replace') + + if data_format == 'json': + base_mimetype = 'application/json' + elif data_format == 'xml': + base_mimetype = 'application/xml' + elif blueprint.csv_field_separator.get() == ',': + base_mimetype = 'text/csv' + else: + base_mimetype = 'text/plain' + r = Response( - gen(conn_obj, - trans_obj, - quote=blueprint.csv_quoting.get(), - quote_char=blueprint.csv_quote_char.get(), - field_separator=blueprint.csv_field_separator.get(), - replace_nulls_with=blueprint.replace_nulls_with.get()), - mimetype='text/csv' if - blueprint.csv_field_separator.get() == ',' - else 'text/plain' + encoded_gen(str_gen), + mimetype='{0}; charset={1}'.format(base_mimetype, output_encoding) ) import time - extn = 'csv' if blueprint.csv_field_separator.get() == ',' else 'txt' + if data_format == 'csv': + extn = 'csv' if blueprint.csv_field_separator.get() == ',' \ + else 'txt' + else: + extn = data_format filename = data['filename'] if data.get('filename', '') != "" else \ '{0}.{1}'.format(int(time.time()), extn) diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx index 639bd39598c..16ae2bd5d57 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx @@ -476,7 +476,8 @@ export class ResultSetUtils { }); } - async saveResultsToFile(fileName, onProgress) { + async saveResultsToFile(fileName, onProgress, dataFormat='csv') { + const mimeTypes = {csv: 'text/csv', json: 'application/json', xml: 'application/xml'}; try { await DownloadUtils.downloadFileStream({ url: url_for('sqleditor.query_tool_download', { @@ -484,8 +485,8 @@ export class ResultSetUtils { }), options: { method: 'POST', - body: JSON.stringify({filename: fileName, query_commited: this.hasQueryCommitted}) - }}, fileName, 'text/csv', onProgress); + body: JSON.stringify({filename: fileName, query_commited: this.hasQueryCommitted, format: dataFormat}) + }}, fileName, mimeTypes[dataFormat] ?? 'text/csv', onProgress); this.eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS_END); } catch (error) { this.eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS_END); @@ -1052,8 +1053,9 @@ export function ResultSet() { setLoaderText(null); }); - eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async ()=>{ - let extension = queryToolCtx.preferences?.sqleditor?.csv_field_separator === ',' ? '.csv': '.txt'; + eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async (dataFormat='csv')=>{ + const csvExtension = queryToolCtx.preferences?.sqleditor?.csv_field_separator === ',' ? '.csv': '.txt'; + let extension = {csv: csvExtension, json: '.json', xml: '.xml'}[dataFormat] ?? csvExtension; let fileName = 'data-' + new Date().getTime() + extension; if(!queryToolCtx.params.is_query_tool) { fileName = queryToolCtx.params.node_name + extension; @@ -1061,7 +1063,7 @@ export function ResultSet() { setLoaderText(gettext('Downloading results...')); await rsu.current.saveResultsToFile(fileName, (p)=>{ setLoaderText(gettext('Downloading results(%s)...', p)); - }); + }, dataFormat); setLoaderText(''); }); diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx index 793ca45dcc4..36daa8b1131 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx @@ -282,6 +282,7 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all /* Menu button refs */ const copyMenuRef = React.useRef(null); const pasetMenuRef = React.useRef(null); + const downloadMenuRef = React.useRef(null); const queryToolPref = queryToolCtx.preferences.sqleditor; @@ -309,8 +310,8 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all const addRow = useCallback(()=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_ADD_ROWS, [[]], {isNewRow: true}); }, []); - const downloadResult = useCallback(()=>{ - eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS); + const downloadResult = useCallback((fmt='csv')=>{ + eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, fmt); }, []); const showGraphVisualiser = useCallback(()=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_GRAPH_VISUALISER); @@ -348,6 +349,14 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all setDisableButton('save-result', (totalRowCount||0) < 1); }, [totalRowCount]); + useEffect(()=>{ + // Seed the "Copy with headers" toggle default from the user preference. + setCheckedMenuItems((prev)=>({ + ...prev, + copy_with_headers: queryToolPref.copy_column_headers, + })); + }, [queryToolPref.copy_column_headers]); + useEffect(()=>{ eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_COPY_DATA, copyData); return ()=>eventBus.deregisterListener(QUERY_TOOL_EVENTS.TRIGGER_COPY_DATA, copyData); @@ -432,7 +441,10 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all } - onClick={downloadResult} shortcut={queryToolPref.download_results} + onClick={()=>downloadResult('csv')} shortcut={queryToolPref.download_results} + disabled={buttonsDisabled['save-result']} /> + } splitButton + name="menu-downloadoptions" ref={downloadMenuRef} onClick={openMenu} disabled={buttonsDisabled['save-result']} /> @@ -490,6 +502,16 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all > {gettext('Paste with SERIAL/IDENTITY values?')} + + downloadResult('csv')}>{gettext('Save as CSV/Text')} + downloadResult('json')}>{gettext('Save as JSON')} + downloadResult('xml')}>{gettext('Save as XML')} + ); } 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 f4157dbdc7f..17a1bd28f44 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 @@ -240,3 +240,133 @@ def tearDown(self): self.server['sslmode'] ) test_utils.drop_database(main_conn, self._db_name) + + +class TestDownloadResultFormats(BaseTestGenerator): + """ + Validates downloading query results as JSON and XML, the UTF BOM option + and the output file encoding option. + """ + SQL = 'SELECT 1 as "A", 2 as "B", \'x\' as "C"' + INIT_URL = '/sqleditor/initialize/sqleditor/{0}/{1}/{2}/{3}' + DOWNLOAD_URL = '/sqleditor/query_tool/download/{0}' + + scenarios = [ + ( + 'Download results as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json') + ), + ( + 'Download results as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml') + ), + ( + 'Download CSV with a UTF BOM', + dict(data_format='csv', add_bom=True, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + 'Download CSV with a non-UTF output encoding', + dict(data_format='csv', add_bom=True, encoding='latin-1', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ] + + def setUp(self): + self._db_name = 'download_results_fmt_' + str( + secrets.choice(range(10000, 65535))) + self._sid = self.server_information['server_id'] + server_utils.connect_server(self, self._sid) + self._did = test_utils.create_database(self.server, self._db_name) + + def initiate_sql_query_tool(self, trans_id, sql_query): + 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) + return async_poll(tester=self.tester, + poll_url='/sqleditor/poll/{0}'.format(trans_id)) + + def runTest(self): + db_con = database_utils.connect_database(self, + test_utils.SERVER_GROUP, + self._sid, + self._did) + if not db_con["info"] == "Database connected.": + raise Exception("Could not connect to the database.") + + self.trans_id = str(secrets.choice(range(1, 9999999))) + url = self.INIT_URL.format( + self.trans_id, test_utils.SERVER_GROUP, self._sid, self._did) + response = self.tester.post(url, data=json.dumps({ + "dbname": self._db_name + })) + self.assertEqual(response.status_code, 200) + + self.initiate_sql_query_tool(self.trans_id, self.SQL) + + url = self.DOWNLOAD_URL.format(self.trans_id) + self.app.logger.disabled = True + filename = 'test{0}'.format(self.expected_extension) + with patch('pgadmin.tools.sqleditor.blueprint.' + 'csv_add_bom.get', return_value=self.add_bom), \ + patch('pgadmin.tools.sqleditor.blueprint.' + 'csv_output_encoding.get', return_value=self.encoding): + response = self.tester.post(url, data={ + "query": self.SQL, + "filename": filename, + "format": self.data_format, + "query_commited": True, + }) + self.app.logger.disabled = False + + headers = dict(response.headers) + self.assertEqual(response.status_code, 200) + self.assertIn(self.expected_content_type, headers['Content-Type']) + self.assertIn('charset={0}'.format(self.encoding), + headers['Content-Type']) + self.assertIn(filename, headers['Content-Disposition']) + + raw = response.data + if self.add_bom and self.encoding.lower().startswith('utf'): + self.assertTrue(raw.startswith(b'\xef\xbb\xbf')) + else: + self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) + + body = raw.decode(self.encoding) + + if self.data_format == 'json': + parsed = json.loads(body) + self.assertIsInstance(parsed, list) + self.assertEqual(parsed[0]['A'], 1) + self.assertEqual(parsed[0]['B'], 2) + self.assertEqual(parsed[0]['C'], 'x') + elif self.data_format == 'xml': + self.assertIn('', body) + self.assertIn('1', body) + self.assertIn('x', body) + self.assertIn('', body) + else: + self.assertIn('"A","B","C"', body) + + url = '/sqleditor/close/{0}'.format(self.trans_id) + response = self.tester.delete(url) + self.assertEqual(response.status_code, 200) + database_utils.disconnect_database(self, self._sid, self._did) + + def tearDown(self): + main_conn = test_utils.get_db_connection( + self.server['db'], + self.server['username'], + self.server['db_password'], + self.server['host'], + self.server['port'], + self.server['sslmode'] + ) + test_utils.drop_database(main_conn, self._db_name) diff --git a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py index 5b6e8941f24..a037b833707 100644 --- a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py +++ b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py @@ -307,6 +307,34 @@ def register_query_tool_preferences(self): allow_blanks=True ) + self.csv_output_encoding = self.preference.register( + 'CSV_output', 'csv_output_encoding', + gettext("Output file encoding"), 'options', 'utf-8', + category_label=PREF_LABEL_CSV_TXT, + options=[{'label': 'utf-8', 'value': 'utf-8'}, + {'label': 'utf-16', 'value': 'utf-16'}, + {'label': 'latin-1', 'value': 'latin-1'}, + {'label': 'windows-1252', 'value': 'windows-1252'}], + control_props={ + 'allowClear': False, + 'tags': False, + 'creatable': True + }, + help_str=gettext('The character encoding used when saving query ' + 'results to a file. Defaults to utf-8. A different ' + 'encoding can be typed in if it is not listed.') + ) + + self.csv_add_bom = self.preference.register( + 'CSV_output', 'csv_add_bom', + gettext("Add byte order mark (BOM)?"), 'boolean', + False, category_label=PREF_LABEL_CSV_TXT, + help_str=gettext('If set to True, a byte order mark (BOM) is added at ' + 'the start of the saved file when a UTF encoding is ' + 'used. This helps applications such as Microsoft ' + 'Excel detect the encoding correctly.') + ) + self.results_grid_quoting = self.preference.register( 'Results_grid', 'results_grid_quoting', gettext("Result copy quoting"), 'options', 'strings', @@ -347,6 +375,16 @@ def register_query_tool_preferences(self): } ) + self.copy_column_headers = self.preference.register( + 'Results_grid', 'copy_column_headers', + gettext("Copy with headers?"), 'boolean', + False, category_label=PREF_LABEL_RESULTS_GRID, + help_str=gettext('If set to True, the column headers are included by ' + 'default when copying data from the results grid. ' + 'This can still be toggled per-copy from the results ' + 'grid copy menu.') + ) + self.column_data_auto_resize = self.preference.register( 'Results_grid', 'column_data_auto_resize', gettext("Columns sized by"), 'radioModern', 'by_data', diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index d07a16cefcd..deb2867ea8e 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -17,7 +17,9 @@ import secrets import datetime import asyncio +import json from collections import deque +from xml.sax.saxutils import escape as xml_escape, quoteattr as xml_quoteattr import psycopg from flask import g, current_app from flask_babel import gettext @@ -54,6 +56,62 @@ _ = gettext + +def _json_default(value): + """Fallback serialiser for values that json cannot encode natively + (dates, Decimals, intervals, etc.).""" + return str(value) + + +def _generate_json(cur, records, results, header, replace_nulls_with, + handle_null_values): + """Stream the result set as a JSON array of row objects. + + The first batch of rows (``results``) has already been fetched by the + caller; subsequent batches are pulled with ``fetchmany(records)``. + """ + yield '[' + is_first_row = True + while results: + if replace_nulls_with is not None: + results = handle_null_values(results, replace_nulls_with) + for row in results: + row_json = json.dumps(dict(row), default=_json_default) + yield row_json if is_first_row else ',' + row_json + is_first_row = False + results = cur.fetchmany(records) + yield ']' + + +def _generate_xml(cur, records, results, header, replace_nulls_with, + handle_null_values): + """Stream the result set as XML. + + Column names are emitted as escaped ``name`` attributes (rather than + element names) so that column names which are not valid XML element + names are handled safely. + """ + yield '\n' + while results: + if replace_nulls_with is not None: + results = handle_null_values(results, replace_nulls_with) + for row in results: + row_io = [''] + for column in header: + value = row.get(column) + if value is None: + row_io.append( + ''.format( + xml_quoteattr(column))) + else: + row_io.append('{1}'.format( + xml_quoteattr(column), xml_escape(str(value)))) + row_io.append('') + yield ''.join(row_io) + results = cur.fetchmany(records) + yield '' + + # Register global type caster which will be applicable to all connections. register_global_typecasters() configure_driver_encodings(encodings) @@ -927,7 +985,8 @@ def handle_null_values(results, replace_nulls_with): return results def gen(conn_obj, trans_obj, quote='strings', quote_char="'", - field_separator=',', replace_nulls_with=None): + field_separator=',', replace_nulls_with=None, + data_format='csv'): try: cur.scroll(0, mode='absolute') @@ -950,37 +1009,24 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", if c.to_dict()['type_code'] in ALL_JSON_TYPES: json_columns.append(column_name) - res_io = StringIO() - - if quote == 'strings': - quote = csv.QUOTE_NONNUMERIC - elif quote == 'all': - quote = csv.QUOTE_ALL + if data_format == 'json': + yield from _generate_json(cur, records, results, header, + replace_nulls_with, + handle_null_values) + elif data_format == 'xml': + yield from _generate_xml(cur, records, results, header, + replace_nulls_with, + handle_null_values) else: - quote = csv.QUOTE_NONE - - csv_writer = csv.DictWriter( - res_io, fieldnames=header, delimiter=field_separator, - quoting=quote, - quotechar=quote_char, - replace_nulls_with=replace_nulls_with - ) - - csv_writer.writeheader() - # Replace the null values with given string if configured. - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) - csv_writer.writerows(results) - - yield res_io.getvalue() - - while True: - results = cur.fetchmany(records) - - if not results: - break res_io = StringIO() + if quote == 'strings': + quote = csv.QUOTE_NONNUMERIC + elif quote == 'all': + quote = csv.QUOTE_ALL + else: + quote = csv.QUOTE_NONE + csv_writer = csv.DictWriter( res_io, fieldnames=header, delimiter=field_separator, quoting=quote, @@ -988,12 +1034,35 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", replace_nulls_with=replace_nulls_with ) + csv_writer.writeheader() # Replace the null values with given string if configured. if replace_nulls_with is not None: results = handle_null_values(results, replace_nulls_with) csv_writer.writerows(results) + yield res_io.getvalue() + while True: + results = cur.fetchmany(records) + + if not results: + break + res_io = StringIO() + + csv_writer = csv.DictWriter( + res_io, fieldnames=header, delimiter=field_separator, + quoting=quote, + quotechar=quote_char, + replace_nulls_with=replace_nulls_with + ) + + # Replace the null values with given string if configured. + if replace_nulls_with is not None: + 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 From 66c39ef1429bcd7044d819f67e5b230fc8c9bc8a Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 11 Jun 2026 10:41:34 +0100 Subject: [PATCH 2/7] Fix double BOM and validate output encoding for result export. Two fixes from code review of the Query Tool result export feature: - Avoid emitting a double byte-order mark for the 'utf-16' and 'utf-32' output encodings. Those codecs (without an explicit endianness suffix) self-emit a BOM, so hand-prepending another produced two BOMs and a corrupt file. We now only hand-write the BOM for codecs that do not emit one themselves (utf-8 and the explicit-endian utf-16/32-le/-be forms), guaranteeing exactly one BOM for every utf-* encoding. - Validate the user-configurable output encoding up front with codecs.lookup() before building the streaming Response, returning a clean 400 instead of raising LookupError mid-stream (which produced a truncated 200 with a raw traceback). Adds test scenarios asserting utf-16 output carries exactly one BOM and that an invalid encoding returns a 400. --- web/pgadmin/tools/sqleditor/__init__.py | 30 +++++++++++-- .../tests/test_download_csv_query_tool.py | 45 ++++++++++++++++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 29901272984..9eb608bcfd9 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -8,6 +8,7 @@ ########################################################################## """A blueprint module implementing the sqleditor frame.""" +import codecs import os import pickle import re @@ -2192,8 +2193,29 @@ def start_query_download_tool(trans_id): else: output_encoding = 'utf-8' add_bom = False - is_utf = output_encoding.lower().replace('-', '').replace( - '_', '').startswith('utf') + # Validate the (free-text, user-configurable) encoding up front so + # an invalid codec returns a clean 400 here, rather than raising a + # LookupError mid-stream after the 200 Response has been returned. + try: + codecs.lookup(output_encoding) + except LookupError: + return make_json_response( + status=400, + success=0, + errormsg=gettext( + "Unknown output encoding '{0}'." + ).format(output_encoding) + ) + + normalized_encoding = output_encoding.lower().replace( + '-', '').replace('_', '') + is_utf = normalized_encoding.startswith('utf') + # The 'utf-16' and 'utf-32' codecs (without an explicit endianness + # suffix) emit their own BOM, so we must not hand-prepend one too; + # doing so would produce two BOMs and corrupt the output. The + # explicit-endian forms (utf-16-le/-be, utf-32-le/-be) and utf-8 do + # not self-emit a BOM, so for those we keep writing it ourselves. + codec_self_emits_bom = normalized_encoding in ('utf16', 'utf32') str_gen = gen(conn_obj, trans_obj, @@ -2208,7 +2230,9 @@ def encoded_gen(text_gen): for chunk in text_gen: if is_first_chunk: is_first_chunk = False - if add_bom and is_utf: + # Only hand-prepend a BOM when the codec does not emit + # one itself, otherwise we'd end up with two BOMs. + if add_bom and is_utf and not codec_self_emits_bom: chunk = '\ufeff' + chunk yield chunk.encode(output_encoding, errors='replace') 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 17a1bd28f44..70550a60580 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 @@ -7,6 +7,7 @@ # This software is released under the PostgreSQL Licence # ########################################################################## +import codecs from unittest.mock import patch from pgadmin.utils.route import BaseTestGenerator @@ -276,6 +277,23 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_content_type='text/csv', expected_extension='.csv') ), + ( + # utf-16 (without endianness) self-emits a BOM, so the result + # must contain exactly one BOM, not two (a hand-prepended one + # plus the codec's own). + 'Download CSV as utf-16 has exactly one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-16', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + # A bogus, non-existent codec must be rejected up front with a + # clean 400, rather than blowing up mid-stream after a 200. + 'Download CSV with an invalid output encoding returns 400', + dict(data_format='csv', add_bom=False, encoding='not-a-codec', + expected_status=400, expected_content_type=None, + expected_extension='.csv') + ), ] def setUp(self): @@ -327,6 +345,18 @@ def runTest(self): self.app.logger.disabled = False headers = dict(response.headers) + + # An invalid encoding must be rejected up front with a clean error + # status, before the streaming Response is constructed. + expected_status = getattr(self, 'expected_status', 200) + if expected_status != 200: + self.assertEqual(response.status_code, expected_status) + url = '/sqleditor/close/{0}'.format(self.trans_id) + response = self.tester.delete(url) + self.assertEqual(response.status_code, 200) + database_utils.disconnect_database(self, self._sid, self._did) + return + self.assertEqual(response.status_code, 200) self.assertIn(self.expected_content_type, headers['Content-Type']) self.assertIn('charset={0}'.format(self.encoding), @@ -334,8 +364,19 @@ def runTest(self): self.assertIn(filename, headers['Content-Disposition']) raw = response.data - if self.add_bom and self.encoding.lower().startswith('utf'): - self.assertTrue(raw.startswith(b'\xef\xbb\xbf')) + normalized = self.encoding.lower().replace('-', '').replace('_', '') + if self.add_bom and normalized.startswith('utf'): + # The output must carry exactly one BOM for the encoding, never + # two (which happened when a BOM was hand-prepended for codecs + # that already self-emit one, e.g. utf-16/utf-32). + bom = { + 'utf8': codecs.BOM_UTF8, + 'utf16': codecs.BOM_UTF16, + 'utf32': codecs.BOM_UTF32, + }[normalized] + self.assertTrue(raw.startswith(bom)) + # No second, redundant BOM immediately after the first. + self.assertFalse(raw[len(bom):].startswith(bom)) else: self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) From b3ab8f44f2424b4fc94356be5abedefac63260e5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Mon, 17 Aug 2026 13:46:53 +0100 Subject: [PATCH 3/7] Make the JSON and XML output parseable whatever the data is Four things were wrong with the new formats, and I checked each against the running server rather than taking them on trust, which is worth saying because two of the four turned out differently from the review. XML was genuinely broken: xml.sax.saxutils.escape() handles the three markup characters and passes everything else through, but XML 1.0 forbids most C0 control characters outright, and they cannot be escaped as character references either. A text column holding chr(1), which PostgreSQL is perfectly happy to store, produced a document that ElementTree rejects with "not well-formed (invalid token)". Those characters are now replaced with U+FFFD, in element text and in the column name attributes alike. NaN and Infinity were genuinely broken too: json.dumps writes them as bare tokens, which Python reads back but most other parsers refuse, so they now become the strings PostgreSQL itself uses. Containers are walked on the way out, since a float8[] or a json column can hold them nested. The bytea case reported in review does not arise on this path: the query tool registers a loader that reports the placeholder "binary data" for bytea, as the grid and the existing CSV export both show, so nothing here ever sees a memoryview. The isinstance handling is still there, cheap insurance if that loader is ever changed, but it is not fixing a live bug. Content-Disposition needed the quoting the review asked for. A name with a space was truncated by the client, so it is quoted now, and where a name cannot be encoded as latin-1 the real name is sent as RFC 5987 filename* rather than being discarded in favour of a hardcoded download.csv, whose extension was wrong for JSON and XML anyway. One further problem the review did not reach: both new formats applied the "Replace null values with" preference, so every NULL arrived as the string "NULL". That preference exists because CSV cannot distinguish an empty field from a NULL; JSON has null and the XML here has null="true", so both now report NULLs natively and the preference applies to CSV only. Tests cover a row containing a forbidden control character, a bytea value, NaN, both infinities and a NULL, asserting that the output parses with a strict parser in each format, plus the two filename cases. --- web/pgadmin/tools/sqleditor/__init__.py | 31 +++-- .../tests/test_download_csv_query_tool.py | 121 +++++++++++++++++- .../utils/driver/psycopg3/connection.py | 93 +++++++++++--- 3 files changed, 211 insertions(+), 34 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 9eb608bcfd9..864a55bec44 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -13,7 +13,7 @@ import pickle import re import secrets -from urllib.parse import unquote +from urllib.parse import quote as url_quote, unquote from threading import Lock from io import BytesIO import threading @@ -2259,18 +2259,27 @@ def encoded_gen(text_gen): filename = data['filename'] if data.get('filename', '') != "" else \ '{0}.{1}'.format(int(time.time()), extn) - # We will try to encode report file name with latin-1 - # If it fails then we will fallback to default ascii file name - # werkzeug only supports latin-1 encoding supported values + # Werkzeug will only put latin-1 in a header, so a name it cannot + # encode needs an ASCII stand-in. RFC 6266 lets us send the real name + # alongside it as filename*, so rather than losing the name entirely + # we offer both and let the client prefer the latter. The fallback + # follows the chosen format rather than always claiming to be a CSV. + ascii_filename = filename try: - tmp_file_name = filename - tmp_file_name.encode('latin-1', 'strict') + filename.encode('latin-1', 'strict') except UnicodeEncodeError: - filename = "download.csv" - - r.headers[ - "Content-Disposition" - ] = "attachment;filename={0}".format(filename) + ascii_filename = 'download.{0}'.format(extn) + + # RFC 6266 requires the quoted form for anything with a space or a + # separator character in it, which a user supplied name can easily + # have. + disposition = 'attachment; filename="{0}"'.format( + ascii_filename.replace('\\', '\\\\').replace('"', '\\"')) + if ascii_filename != filename: + disposition += "; filename*=UTF-8''{0}".format( + url_quote(filename, safe='')) + + r.headers["Content-Disposition"] = disposition return r except (ConnectionLost, SSHTunnelConnectionLost): 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 70550a60580..ca33e8e8ba5 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 @@ -9,6 +9,8 @@ ########################################################################## import codecs from unittest.mock import patch +from urllib.parse import quote as url_quote +from xml.etree import ElementTree from pgadmin.utils.route import BaseTestGenerator from pgadmin.browser.server_groups.servers.databases.tests import utils as \ @@ -243,6 +245,19 @@ def tearDown(self): test_utils.drop_database(main_conn, self._db_name) +# A control character that XML 1.0 does not allow at all (not even as a +# character reference), a bytea value, the two non-finite floats that JSON +# has no syntax for, and a NULL. +AWKWARD_SQL = ( + 'SELECT E\'ctl\\x01char\' as "Ctl", ' + '\'\\x48656c6c6f\'::bytea as "Bytes", ' + '\'NaN\'::float8 as "NotANumber", ' + '\'Infinity\'::float8 as "Inf", ' + '\'-Infinity\'::float8 as "NegInf", ' + 'NULL::text as "Nothing"' +) + + class TestDownloadResultFormats(BaseTestGenerator): """ Validates downloading query results as JSON and XML, the UTF BOM option @@ -294,8 +309,49 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_status=400, expected_content_type=None, expected_extension='.csv') ), + ( + # RFC 6266 requires the quoted form once the name contains a + # space, or the client sees a truncated filename. + 'Download with a filename containing spaces', + dict(data_format='csv', add_bom=False, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv', + filename_override='my query results.csv') + ), + ( + # A name werkzeug cannot put in a latin-1 header still has to + # reach the client, via the RFC 5987 filename* form, rather than + # being thrown away. + 'Download with a filename outside latin-1', + dict(data_format='csv', add_bom=False, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv', + filename_override='ohms-\u03a9.csv') + ), + ( + # Data that the naive serialisers get wrong: a control character + # that XML 1.0 forbids outright, a bytea column, the non-finite + # floats that are not valid JSON, and a NULL. + 'Download awkward data as JSON stays valid JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', sql=AWKWARD_SQL, + awkward_data=True) + ), + ( + 'Download awkward data as XML stays well formed', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', sql=AWKWARD_SQL, + awkward_data=True) + ), ] + # Set per scenario; the scenarios above override these as needed. + sql = None + awkward_data = False + filename_override = None + def setUp(self): self._db_name = 'download_results_fmt_' + str( secrets.choice(range(10000, 65535))) @@ -311,6 +367,46 @@ def initiate_sql_query_tool(self, trans_id, sql_query): return async_poll(tester=self.tester, poll_url='/sqleditor/poll/{0}'.format(trans_id)) + def _assert_awkward_data(self, body): + """The output must be parseable, whatever the data contained. + + A strict parser is the point here: XML 1.0 forbids most control + characters outright, and NaN/Infinity are not JSON tokens, so an + exporter that passes them straight through produces a file the + user's next tool refuses to open. + """ + if self.data_format == 'json': + def reject_constant(constant): + # json.loads accepts NaN and Infinity by default even though + # they are not JSON; most other parsers do not, so treat them + # as the failure they are. + raise AssertionError( + '{0} is not a JSON token'.format(constant)) + + parsed = json.loads(body, parse_constant=reject_constant) + self.assertEqual(len(parsed), 1) + row = parsed[0] + # A NULL must survive as JSON null rather than a string. + self.assertIsNone(row['Nothing']) + # NaN and Infinity have to arrive as something a parser will + # accept, i.e. not as bare NaN/Infinity tokens. + self.assertEqual(row['NotANumber'], 'NaN') + self.assertEqual(row['Inf'], 'Infinity') + self.assertEqual(row['NegInf'], '-Infinity') + # bytea is deliberately reported as a placeholder rather than its + # contents, as it is in the grid and in CSV output, but it must + # never leak a Python repr such as ''. + self.assertNotIn('memory at', str(row['Bytes'])) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + columns = {c.get('name'): c for c in root.find('row')} + self.assertEqual(columns['Nothing'].get('null'), 'true') + self.assertNotIn('memory at', columns['Bytes'].text or '') + # The control character must not have been passed through verbatim. + self.assertNotIn('\x01', body) + def runTest(self): db_con = database_utils.connect_database(self, test_utils.SERVER_GROUP, @@ -327,17 +423,19 @@ def runTest(self): })) self.assertEqual(response.status_code, 200) - self.initiate_sql_query_tool(self.trans_id, self.SQL) + sql = self.sql or self.SQL + self.initiate_sql_query_tool(self.trans_id, sql) url = self.DOWNLOAD_URL.format(self.trans_id) self.app.logger.disabled = True - filename = 'test{0}'.format(self.expected_extension) + filename = self.filename_override or \ + 'test{0}'.format(self.expected_extension) with patch('pgadmin.tools.sqleditor.blueprint.' 'csv_add_bom.get', return_value=self.add_bom), \ patch('pgadmin.tools.sqleditor.blueprint.' 'csv_output_encoding.get', return_value=self.encoding): response = self.tester.post(url, data={ - "query": self.SQL, + "query": sql, "filename": filename, "format": self.data_format, "query_commited": True, @@ -361,7 +459,18 @@ def runTest(self): self.assertIn(self.expected_content_type, headers['Content-Type']) self.assertIn('charset={0}'.format(self.encoding), headers['Content-Type']) - self.assertIn(filename, headers['Content-Disposition']) + disposition = headers['Content-Disposition'] + try: + filename.encode('latin-1', 'strict') + except UnicodeEncodeError: + # The stand-in must follow the format, and the real name must + # still be there in percent-encoded form. + self.assertIn('filename="download{0}"'.format( + self.expected_extension), disposition) + self.assertIn("filename*=UTF-8''", disposition) + self.assertIn(url_quote(filename, safe=''), disposition) + else: + self.assertIn('filename="{0}"'.format(filename), disposition) raw = response.data normalized = self.encoding.lower().replace('-', '').replace('_', '') @@ -382,7 +491,9 @@ def runTest(self): body = raw.decode(self.encoding) - if self.data_format == 'json': + if self.awkward_data: + self._assert_awkward_data(body) + elif self.data_format == 'json': parsed = json.loads(body) self.assertIsInstance(parsed, list) self.assertEqual(parsed[0]['A'], 1) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index deb2867ea8e..175c7735078 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -18,7 +18,9 @@ import datetime import asyncio import json +import re from collections import deque +from math import isfinite, isnan from xml.sax.saxutils import escape as xml_escape, quoteattr as xml_quoteattr import psycopg from flask import g, current_app @@ -63,38 +65,97 @@ def _json_default(value): return str(value) -def _generate_json(cur, records, results, header, replace_nulls_with, - handle_null_values): +# XML 1.0 permits tab, newline, carriage return and nothing else below U+0020, +# and forbids the surrogate range and U+FFFE/U+FFFF. Those characters cannot +# even be written as character references, so a parser rejects the whole +# document: a text column legally holding chr(1) would otherwise produce a +# file nothing can open. +_XML_ILLEGAL_CHARS = re.compile( + '[^\u0009\u000a\u000d\u0020-\ud7ff\ue000-\ufffd' + '\U00010000-\U0010ffff]' +) + +# Substituted for anything XML cannot carry. +_XML_REPLACEMENT = '\ufffd' + + +def _to_text(value): + """Render a value as text for the structured output formats.""" + if isinstance(value, (memoryview, bytes, bytearray)): + # Match the hex form PostgreSQL itself uses for bytea, rather than + # letting str() produce something like ''. + return '\\x' + bytes(value).hex() + if isinstance(value, float) and not isfinite(value): + return 'NaN' if isnan(value) else \ + ('Infinity' if value > 0 else '-Infinity') + return str(value) + + +def _xml_text(value): + """Escape a value for XML, dropping characters XML cannot represent.""" + return xml_escape( + _XML_ILLEGAL_CHARS.sub(_XML_REPLACEMENT, _to_text(value))) + + +def _xml_attr(value): + """Quote an attribute value for XML, with the same sanitising.""" + return xml_quoteattr(_XML_ILLEGAL_CHARS.sub(_XML_REPLACEMENT, str(value))) + + +def _json_safe(value): + """Convert a value into something json can encode, and validly. + + NaN and Infinity are not JSON tokens: json.dumps emits them bare by + default, which Python itself will read back but most other parsers + reject, so they become the strings PostgreSQL uses for them. bytea is + rendered in the same hex form as elsewhere. Containers are walked + because a float8[] or a json column can hold either case nested. + """ + if isinstance(value, (memoryview, bytes, bytearray)): + return _to_text(value) + if isinstance(value, float) and not isfinite(value): + return _to_text(value) + if isinstance(value, dict): + return {key: _json_safe(val) for key, val in value.items()} + if isinstance(value, (list, tuple)): + return [_json_safe(val) for val in value] + return value + + +def _generate_json(cur, records, results): """Stream the result set as a JSON array of row objects. The first batch of rows (``results``) has already been fetched by the caller; subsequent batches are pulled with ``fetchmany(records)``. + + The 'Replace null values with' preference is deliberately not applied: + it exists because CSV has no way to distinguish an empty field from a + NULL, whereas JSON has null, and substituting the placeholder string + would turn every NULL into ordinary text. """ yield '[' is_first_row = True while results: - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) for row in results: - row_json = json.dumps(dict(row), default=_json_default) + row_json = json.dumps( + {key: _json_safe(value) for key, value in dict(row).items()}, + default=_json_default, allow_nan=False) yield row_json if is_first_row else ',' + row_json is_first_row = False results = cur.fetchmany(records) yield ']' -def _generate_xml(cur, records, results, header, replace_nulls_with, - handle_null_values): +def _generate_xml(cur, records, results, header): """Stream the result set as XML. Column names are emitted as escaped ``name`` attributes (rather than element names) so that column names which are not valid XML element - names are handled safely. + names are handled safely. As with JSON, NULLs are reported natively, + via null="true", rather than through the CSV placeholder preference. """ yield '\n' while results: - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) for row in results: row_io = [''] for column in header: @@ -102,10 +163,10 @@ def _generate_xml(cur, records, results, header, replace_nulls_with, if value is None: row_io.append( ''.format( - xml_quoteattr(column))) + _xml_attr(column))) else: row_io.append('{1}'.format( - xml_quoteattr(column), xml_escape(str(value)))) + _xml_attr(column), _xml_text(value))) row_io.append('') yield ''.join(row_io) results = cur.fetchmany(records) @@ -1010,13 +1071,9 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", json_columns.append(column_name) if data_format == 'json': - yield from _generate_json(cur, records, results, header, - replace_nulls_with, - handle_null_values) + yield from _generate_json(cur, records, results) elif data_format == 'xml': - yield from _generate_xml(cur, records, results, header, - replace_nulls_with, - handle_null_values) + yield from _generate_xml(cur, records, results, header) else: res_io = StringIO() From 97f351c7e82f47fbf56efb52744c57256032f2a3 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 20 Aug 2026 09:16:34 +0100 Subject: [PATCH 4/7] Emit direct single-value output and fix empty-result content type for JSON/XML export. Addresses the remaining CodeRabbit findings on #10062: - A genuine single-row, single-column result is now written as the bare value (a JSON scalar, or an XML document with no / wrapper), per #3205, instead of always being wrapped in a one-element array or a element. - An empty (zero-row) result now yields an empty JSON array or an empty XML document for those formats, rather than the CSV-era plain-text "did not return any data" message under an application/json or application/xml content type. - Added regression coverage for both cases (including the NULL single-value shape), and extended the Latin-1 output-encoding test to use a character Latin-1 cannot represent (previously ASCII-only, so it could not have caught silent character loss), asserting the existing errors='replace' contract is preserved end to end. --- .../tests/test_download_csv_query_tool.py | 110 +++++++++++++++++- .../utils/driver/psycopg3/connection.py | 50 +++++++- 2 files changed, 156 insertions(+), 4 deletions(-) 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 ca33e8e8ba5..b2bf87f0ad9 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 @@ -287,10 +287,17 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_extension='.csv') ), ( + # '€' (Euro sign) cannot be represented in Latin-1. The + # exporter's errors='replace' contract must survive the whole + # request/response round trip: an ASCII-only fixture would let a + # regression that silently drops or mis-encodes the character + # pass unnoticed. 'Download CSV with a non-UTF output encoding', dict(data_format='csv', add_bom=True, encoding='latin-1', expected_content_type='text/csv', - expected_extension='.csv') + expected_extension='.csv', + sql='SELECT 1 as "A", 2 as "B", \'€\' as "C"', + non_latin1_char='€') ), ( # utf-16 (without endianness) self-emits a BOM, so the result @@ -345,12 +352,69 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_extension='.xml', sql=AWKWARD_SQL, awkward_data=True) ), + ( + # A genuine single-row, single-column result must be written as + # the bare value, not wrapped in the usual array/row structure, + # per #3205. + 'Download a single-value result as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT 42 as "Value"', single_value=True) + ), + ( + 'Download a single-value result as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT 42 as "Value"', single_value=True) + ), + ( + # A single-row, single-column NULL is still the direct-value + # shape, i.e. a bare JSON null / an empty element with + # null="true", not a row containing one null column. + 'Download a single-value NULL result as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT NULL::text as "Value"', single_value=True, + single_value_is_null=True) + ), + ( + 'Download a single-value NULL result as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT NULL::text as "Value"', single_value=True, + single_value_is_null=True) + ), + ( + # Zero rows must still come back as a (empty) document of the + # requested format, not the CSV-era plain-text message under an + # application/json or application/xml content type. + 'Download an empty result as JSON stays valid JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT 1 as "A" WHERE false', empty_result=True) + ), + ( + 'Download an empty result as XML stays well formed', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT 1 as "A" WHERE false', empty_result=True) + ), ] # Set per scenario; the scenarios above override these as needed. sql = None awkward_data = False + single_value = False + single_value_is_null = False + empty_result = False filename_override = None + non_latin1_char = None def setUp(self): self._db_name = 'download_results_fmt_' + str( @@ -407,6 +471,40 @@ def reject_constant(constant): # The control character must not have been passed through verbatim. self.assertNotIn('\x01', body) + def _assert_single_value(self, body): + """A genuine single-row, single-column result must be the bare + value, per #3205, not a one-element array / one-row document. + """ + if self.data_format == 'json': + parsed = json.loads(body) + if self.single_value_is_null: + self.assertIsNone(parsed) + else: + self.assertEqual(parsed, 42) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + # No row/column wrapper, and no column name anywhere in sight. + self.assertIsNone(root.find('row')) + self.assertIsNone(root.find('column')) + if self.single_value_is_null: + self.assertEqual(root.get('null'), 'true') + else: + self.assertEqual(root.text, '42') + + def _assert_empty_result(self, body): + """Zero rows must still come back as an (empty) document of the + requested format, not the CSV-era plain-text message. + """ + if self.data_format == 'json': + self.assertEqual(json.loads(body), []) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + self.assertEqual(list(root), []) + def runTest(self): db_con = database_utils.connect_database(self, test_utils.SERVER_GROUP, @@ -493,6 +591,10 @@ def runTest(self): if self.awkward_data: self._assert_awkward_data(body) + elif self.single_value: + self._assert_single_value(body) + elif self.empty_result: + self._assert_empty_result(body) elif self.data_format == 'json': parsed = json.loads(body) self.assertIsInstance(parsed, list) @@ -506,6 +608,12 @@ def runTest(self): self.assertIn('', body) else: self.assertIn('"A","B","C"', body) + if self.non_latin1_char: + # errors='replace' must turn the character Latin-1 cannot + # encode into the codec's standard replacement rather than + # silently dropping it or corrupting the row. + self.assertNotIn(self.non_latin1_char, body) + self.assertIn('?', body) url = '/sqleditor/close/{0}'.format(self.trans_id) response = self.tester.delete(url) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index 175c7735078..a770e1c6955 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -173,6 +173,23 @@ def _generate_xml(cur, records, results, header): yield '' +def _generate_single_value(data_format, value): + """Render a genuine single-row, single-column result directly, per + issue #3205: no array wrapper for JSON, no / wrapper for + XML, just the value itself. NULL is reported the same way it is + elsewhere: JSON null, or an empty element with null="true". + """ + if data_format == 'json': + return json.dumps( + _json_safe(value), default=_json_default, allow_nan=False) + + if value is None: + return ('\n' + '') + return ('\n' + '{0}'.format(_xml_text(value))) + + # Register global type caster which will be applicable to all connections. register_global_typecasters() configure_driver_encodings(encodings) @@ -1056,9 +1073,6 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", # Make sure numeric values will be fetched without quoting register_numeric_typecasters(cur) results = cur.fetchmany(records) - if not results: - yield gettext('The query executed did not return any data.') - return header = [] json_columns = [] @@ -1070,6 +1084,36 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", if c.to_dict()['type_code'] in ALL_JSON_TYPES: json_columns.append(column_name) + if not results: + # An empty result must still come back in the requested + # format: JSON/XML consumers expect a (empty) document of + # that type, not the CSV-era plain-text message under an + # application/json or application/xml content type. + if data_format == 'json': + yield '[]' + elif data_format == 'xml': + yield ('\n' + '') + else: + yield gettext( + 'The query executed did not return any data.') + return + + if data_format in ('json', 'xml') and len(header) == 1 and \ + len(results) == 1: + # A genuine single-row, single-column result is written as + # the bare value, without the usual array/row wrapper, per + # #3205. Confirm there really is only one row before + # committing to that shape: a batch boundary can make the + # first fetchmany() return exactly one row even though more + # follow. + more = cur.fetchmany(records) + if not more: + yield _generate_single_value( + data_format, results[0].get(header[0])) + return + results = results + more + if data_format == 'json': yield from _generate_json(cur, records, results) elif data_format == 'xml': From e3d2f38cd3136714da4724496ac2b28dca78e4db Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 1 Sep 2026 12:13:13 +0100 Subject: [PATCH 5/7] Emit one BOM per file rather than one per chunk, and clean up in tearDown The exporter fetches ten rows at a time and encoded each chunk with its own chunk.encode() call. The 'utf-16' and 'utf-32' codecs emit their BOM on every such call, so any export large enough to span more than one chunk carried a BOM at the head of every chunk rather than once at the head of the file. The existing utf-16 scenario could not catch it: its fixture is a single row, which never spans chunks. Encoding now goes through an incremental encoder, which emits the BOM once and then keeps going, with a final flush for anything the codec is holding. The utf-16 scenario is joined by a 25 row one, and the assertion no longer just checks that a second BOM does not immediately follow the first: it counts them across the whole payload. The fixture data is ASCII, so the BOM byte sequence cannot occur in the encoded rows and the count is exact. Closing the transaction and disconnecting have moved into tearDown, so a failed assertion no longer skips them and leaves the connection open against a database that is about to be dropped. --- web/pgadmin/tools/sqleditor/__init__.py | 14 +++++++++- .../tests/test_download_csv_query_tool.py | 27 ++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 864a55bec44..24b2ffb34f9 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2225,6 +2225,14 @@ def start_query_download_tool(trans_id): replace_nulls_with=blueprint.replace_nulls_with.get(), data_format=data_format) + # Encode incrementally rather than a chunk at a time. The codecs + # that emit their own BOM do so on every encode() call, so encoding + # each chunk on its own would put a BOM at the head of every chunk + # rather than once at the head of the file; an incremental encoder + # emits it once and then keeps going. + encoder = codecs.getincrementalencoder(output_encoding)( + errors='replace') + def encoded_gen(text_gen): is_first_chunk = True for chunk in text_gen: @@ -2234,7 +2242,11 @@ def encoded_gen(text_gen): # one itself, otherwise we'd end up with two BOMs. if add_bom and is_utf and not codec_self_emits_bom: chunk = '\ufeff' + chunk - yield chunk.encode(output_encoding, errors='replace') + yield encoder.encode(chunk) + + trailing = encoder.encode('', True) + if trailing: + yield trailing if data_format == 'json': base_mimetype = 'application/json' 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 b2bf87f0ad9..c5edf46a995 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 @@ -308,6 +308,18 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_content_type='text/csv', expected_extension='.csv') ), + ( + # The exporter fetches ten rows at a time, and the codecs that + # self-emit a BOM do so on every encode() call, so anything + # spanning more than one chunk is where a BOM per chunk would + # show up. A single row cannot catch that. + 'Download multi-chunk CSV as utf-16 has exactly one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-16', + expected_content_type='text/csv', + expected_extension='.csv', + sql='SELECT g as "A", g * 2 as "B", \'x\' as "C" ' + 'FROM generate_series(1, 25) g') + ), ( # A bogus, non-existent codec must be rejected up front with a # clean 400, rather than blowing up mid-stream after a 200. @@ -584,6 +596,12 @@ def runTest(self): self.assertTrue(raw.startswith(bom)) # No second, redundant BOM immediately after the first. self.assertFalse(raw[len(bom):].startswith(bom)) + # And none further in either: the exporter encodes in chunks, + # and a codec that emits its own BOM emits one per encode() + # call, so a payload spanning several chunks is where a stray + # BOM would otherwise appear. The fixture data is ASCII, so the + # BOM byte sequence cannot occur in the encoded rows. + self.assertEqual(raw.count(bom), 1) else: self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) @@ -615,12 +633,15 @@ def runTest(self): self.assertNotIn(self.non_latin1_char, body) self.assertIn('?', body) + def tearDown(self): + # Closing the transaction and disconnecting belong here rather than + # at the end of runTest: a failed assertion would otherwise skip + # them and leave the connection open against a database that is + # about to be dropped. url = '/sqleditor/close/{0}'.format(self.trans_id) - response = self.tester.delete(url) - self.assertEqual(response.status_code, 200) + self.tester.delete(url) database_utils.disconnect_database(self, self._sid, self._did) - def tearDown(self): main_conn = test_utils.get_db_connection( self.server['db'], self.server['username'], From e635faa8e0a23bfc9742b11beffbb6aa550f0516 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 23 Sep 2026 16:18:00 +0100 Subject: [PATCH 6/7] Address the rest of Ashesh's review of the result export These are the points from his review comment that the earlier rounds had not reached. - utf-8-sig writes its own BOM, just as utf-16 and utf-32 do, so asking for a BOM as well produced two of them. It now joins those codecs in the set we never hand-prepend a BOM for, with a test scenario. - The encoding and BOM preference help text, and the matching docs, now say what happens to a character the chosen encoding cannot represent (it becomes '?') and that utf-16, utf-32 and utf-8-sig always carry a BOM whatever the BOM switch says. - JSON and XML are streamed one fetched batch at a time, as CSV already was, rather than one row at a time, with multi-batch scenarios for both confirming every row arrives in one well-formed document. - The json_columns list built in gen() was never read, so it and the now unused ALL_JSON_TYPES import are gone. - The 'csv' default for the download format is a single constant in QueryToolConstants.js rather than being repeated in three places. --- docs/en_US/preferences.rst | 6 ++-- web/pgadmin/tools/sqleditor/__init__.py | 12 ++++--- .../js/components/QueryToolConstants.js | 8 +++++ .../js/components/sections/ResultSet.jsx | 6 ++-- .../components/sections/ResultSetToolbar.jsx | 12 +++---- .../tests/test_download_csv_query_tool.py | 35 +++++++++++++++++++ .../sqleditor/utils/query_tool_preferences.py | 8 +++-- .../utils/driver/psycopg3/connection.py | 35 ++++++++++--------- 8 files changed, 87 insertions(+), 35 deletions(-) diff --git a/docs/en_US/preferences.rst b/docs/en_US/preferences.rst index 630d2070966..57a68692b63 100644 --- a/docs/en_US/preferences.rst +++ b/docs/en_US/preferences.rst @@ -591,11 +591,13 @@ Use the fields on the *CSV/TXT Output* panel to control the CSV/TXT output. specified string in the output file. Default is set to 'NULL'. * Use the *Output file encoding* drop-down listbox to specify the character encoding used when saving query results to a file. The default is utf-8; an - encoding that is not listed can also be typed in. + encoding that is not listed can also be typed in. Any character that the + chosen encoding cannot represent is written as a question mark (?). * Use the *Add byte order mark (BOM)?* switch to add a byte order mark at the start of the saved file when a UTF encoding is used. This helps applications such as Microsoft Excel detect the encoding correctly. This applies to the - CSV/TXT output only. + CSV/TXT output only. The utf-16, utf-32 and utf-8-sig encodings always + include a BOM, whatever this is set to. .. image:: images/preferences_sql_display.png :alt: Preferences sqleditor display options diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 24b2ffb34f9..1122ee3c676 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2211,11 +2211,13 @@ def start_query_download_tool(trans_id): '-', '').replace('_', '') is_utf = normalized_encoding.startswith('utf') # The 'utf-16' and 'utf-32' codecs (without an explicit endianness - # suffix) emit their own BOM, so we must not hand-prepend one too; - # doing so would produce two BOMs and corrupt the output. The - # explicit-endian forms (utf-16-le/-be, utf-32-le/-be) and utf-8 do - # not self-emit a BOM, so for those we keep writing it ourselves. - codec_self_emits_bom = normalized_encoding in ('utf16', 'utf32') + # suffix) and 'utf-8-sig' emit their own BOM, so we must not + # hand-prepend one too; doing so would produce two BOMs and corrupt + # the output. The explicit-endian forms (utf-16-le/-be, + # utf-32-le/-be) and utf-8 do not self-emit a BOM, so for those we + # keep writing it ourselves. + codec_self_emits_bom = normalized_encoding in ( + 'utf16', 'utf32', 'utf8sig') str_gen = gen(conn_obj, trans_obj, diff --git a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js b/web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js index 50ea29d870a..ee2277aae4d 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js +++ b/web/pgadmin/tools/sqleditor/static/js/components/QueryToolConstants.js @@ -123,4 +123,12 @@ export const PANELS = { export const MAX_QUERY_LENGTH = 1000000; +export const RESULT_DOWNLOAD_FORMATS = { + CSV: 'csv', + JSON: 'json', + XML: 'xml', +}; + +export const DEFAULT_RESULT_DOWNLOAD_FORMAT = RESULT_DOWNLOAD_FORMATS.CSV; + export const OS_EOL = getPlatform() === 'Windows' ? 'crlf' : 'lf'; diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx index 16ae2bd5d57..ab2c2cbd259 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx @@ -10,7 +10,7 @@ import _ from 'lodash'; import { styled } from '@mui/material/styles'; import React, { useContext, useEffect, useRef, useState } from 'react'; import QueryToolDataGrid, { GRID_ROW_SELECT_KEY } from '../QueryToolDataGrid'; -import {CONNECTION_STATUS, PANELS, QUERY_TOOL_EVENTS, MODAL_DIALOGS} from '../QueryToolConstants'; +import {CONNECTION_STATUS, PANELS, QUERY_TOOL_EVENTS, MODAL_DIALOGS, DEFAULT_RESULT_DOWNLOAD_FORMAT} from '../QueryToolConstants'; import url_for from 'sources/url_for'; import getApiInstance, { parseApiError } from '../../../../../../static/js/api_instance'; import { QueryToolContext, QueryToolEventsContext } from '../QueryToolComponent'; @@ -476,7 +476,7 @@ export class ResultSetUtils { }); } - async saveResultsToFile(fileName, onProgress, dataFormat='csv') { + async saveResultsToFile(fileName, onProgress, dataFormat=DEFAULT_RESULT_DOWNLOAD_FORMAT) { const mimeTypes = {csv: 'text/csv', json: 'application/json', xml: 'application/xml'}; try { await DownloadUtils.downloadFileStream({ @@ -1053,7 +1053,7 @@ export function ResultSet() { setLoaderText(null); }); - eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async (dataFormat='csv')=>{ + eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async (dataFormat=DEFAULT_RESULT_DOWNLOAD_FORMAT)=>{ const csvExtension = queryToolCtx.preferences?.sqleditor?.csv_field_separator === ',' ? '.csv': '.txt'; let extension = {csv: csvExtension, json: '.json', xml: '.xml'}[dataFormat] ?? csvExtension; let fileName = 'data-' + new Date().getTime() + extension; diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx index 36daa8b1131..26b5e4d27ee 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx @@ -26,7 +26,7 @@ import EditOffRoundedIcon from '@mui/icons-material/EditOffRounded'; import CheckRoundedIcon from '@mui/icons-material/CheckRounded'; import AllInboxRoundedIcon from '@mui/icons-material/AllInboxRounded'; -import {QUERY_TOOL_EVENTS} from '../QueryToolConstants'; +import {QUERY_TOOL_EVENTS, RESULT_DOWNLOAD_FORMATS, DEFAULT_RESULT_DOWNLOAD_FORMAT} from '../QueryToolConstants'; import { QueryToolContext, QueryToolEventsContext } from '../QueryToolComponent'; import { PgMenu, PgMenuItem } from '../../../../../../static/js/components/Menu'; import gettext from 'sources/gettext'; @@ -310,7 +310,7 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all const addRow = useCallback(()=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_ADD_ROWS, [[]], {isNewRow: true}); }, []); - const downloadResult = useCallback((fmt='csv')=>{ + const downloadResult = useCallback((fmt=DEFAULT_RESULT_DOWNLOAD_FORMAT)=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, fmt); }, []); const showGraphVisualiser = useCallback(()=>{ @@ -441,7 +441,7 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all } - onClick={()=>downloadResult('csv')} shortcut={queryToolPref.download_results} + onClick={()=>downloadResult(DEFAULT_RESULT_DOWNLOAD_FORMAT)} shortcut={queryToolPref.download_results} disabled={buttonsDisabled['save-result']} /> } splitButton name="menu-downloadoptions" ref={downloadMenuRef} onClick={openMenu} @@ -508,9 +508,9 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all onClose={handleMenuClose} label={gettext('Save Results Options Menu')} > - downloadResult('csv')}>{gettext('Save as CSV/Text')} - downloadResult('json')}>{gettext('Save as JSON')} - downloadResult('xml')}>{gettext('Save as XML')} + downloadResult(RESULT_DOWNLOAD_FORMATS.CSV)}>{gettext('Save as CSV/Text')} + downloadResult(RESULT_DOWNLOAD_FORMATS.JSON)}>{gettext('Save as JSON')} + downloadResult(RESULT_DOWNLOAD_FORMATS.XML)}>{gettext('Save as XML')} ); 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 c5edf46a995..821d502e1bc 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 @@ -320,6 +320,33 @@ class TestDownloadResultFormats(BaseTestGenerator): sql='SELECT g as "A", g * 2 as "B", \'x\' as "C" ' 'FROM generate_series(1, 25) g') ), + ( + # utf-8-sig self-emits a BOM too, so asking for one as well + # must still produce exactly one. + 'Download CSV as utf-8-sig has exactly one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-8-sig', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + # JSON and XML are streamed a fetched batch at a time, so a + # result spanning several batches must still join up into one + # well-formed document holding every row. + 'Download multi-chunk results as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', expected_rows=25, + sql='SELECT g as "A", g * 2 as "B", \'x\' as "C" ' + 'FROM generate_series(1, 25) g') + ), + ( + 'Download multi-chunk results as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', expected_rows=25, + sql='SELECT g as "A", g * 2 as "B", \'x\' as "C" ' + 'FROM generate_series(1, 25) g') + ), ( # A bogus, non-existent codec must be rejected up front with a # clean 400, rather than blowing up mid-stream after a 200. @@ -427,6 +454,7 @@ class TestDownloadResultFormats(BaseTestGenerator): empty_result = False filename_override = None non_latin1_char = None + expected_rows = None def setUp(self): self._db_name = 'download_results_fmt_' + str( @@ -590,6 +618,7 @@ def runTest(self): # that already self-emit one, e.g. utf-16/utf-32). bom = { 'utf8': codecs.BOM_UTF8, + 'utf8sig': codecs.BOM_UTF8, 'utf16': codecs.BOM_UTF16, 'utf32': codecs.BOM_UTF32, }[normalized] @@ -619,11 +648,17 @@ def runTest(self): self.assertEqual(parsed[0]['A'], 1) self.assertEqual(parsed[0]['B'], 2) self.assertEqual(parsed[0]['C'], 'x') + if self.expected_rows is not None: + self.assertEqual(len(parsed), self.expected_rows) elif self.data_format == 'xml': self.assertIn('', body) self.assertIn('1', body) self.assertIn('x', body) self.assertIn('', body) + if self.expected_rows is not None: + root = ElementTree.fromstring(body) + self.assertEqual(len(root.findall('row')), + self.expected_rows) else: self.assertIn('"A","B","C"', body) if self.non_latin1_char: diff --git a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py index a037b833707..8521c94a649 100644 --- a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py +++ b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py @@ -322,7 +322,9 @@ def register_query_tool_preferences(self): }, help_str=gettext('The character encoding used when saving query ' 'results to a file. Defaults to utf-8. A different ' - 'encoding can be typed in if it is not listed.') + 'encoding can be typed in if it is not listed. ' + 'Any character that the chosen encoding cannot ' + 'represent is written as a question mark (?).') ) self.csv_add_bom = self.preference.register( @@ -332,7 +334,9 @@ def register_query_tool_preferences(self): help_str=gettext('If set to True, a byte order mark (BOM) is added at ' 'the start of the saved file when a UTF encoding is ' 'used. This helps applications such as Microsoft ' - 'Excel detect the encoding correctly.') + 'Excel detect the encoding correctly. The utf-16, ' + 'utf-32 and utf-8-sig encodings always include a ' + 'BOM, whatever this is set to.') ) self.results_grid_quoting = self.preference.register( diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index a770e1c6955..baef6304c78 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -39,7 +39,7 @@ from .typecast import register_binary_data_typecasters,\ register_global_typecasters, register_string_typecasters,\ register_binary_typecasters, register_array_to_string_typecasters,\ - register_numeric_typecasters, ALL_JSON_TYPES + register_numeric_typecasters from .encoding import get_encoding, configure_driver_encodings from pgadmin.utils.text_sanitize import sanitize_external_text from pgadmin.utils import csv_lib as csv @@ -133,15 +133,18 @@ def _generate_json(cur, records, results): NULL, whereas JSON has null, and substituting the placeholder string would turn every NULL into ordinary text. """ - yield '[' - is_first_row = True + # One chunk per fetched batch rather than per row, as the CSV path does, + # so a large export is not streamed as a very long run of tiny pieces. + separator = '[' while results: + batch = [] for row in results: - row_json = json.dumps( + batch.append(separator) + batch.append(json.dumps( {key: _json_safe(value) for key, value in dict(row).items()}, - default=_json_default, allow_nan=False) - yield row_json if is_first_row else ',' + row_json - is_first_row = False + default=_json_default, allow_nan=False)) + separator = ',' + yield ''.join(batch) results = cur.fetchmany(records) yield ']' @@ -156,19 +159,21 @@ def _generate_xml(cur, records, results, header): """ yield '\n' while results: + # As with JSON, one chunk per fetched batch rather than per row. + batch = [] for row in results: - row_io = [''] + batch.append('') for column in header: value = row.get(column) if value is None: - row_io.append( + batch.append( ''.format( _xml_attr(column))) else: - row_io.append('{1}'.format( + batch.append('{1}'.format( _xml_attr(column), _xml_text(value))) - row_io.append('') - yield ''.join(row_io) + batch.append('') + yield ''.join(batch) results = cur.fetchmany(records) yield '' @@ -1075,14 +1080,10 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", results = cur.fetchmany(records) header = [] - json_columns = [] for c in cur.ordered_description(): # This is to handle the case in which column name is non-ascii - column_name = c.to_dict()['name'] - header.append(column_name) - if c.to_dict()['type_code'] in ALL_JSON_TYPES: - json_columns.append(column_name) + header.append(c.to_dict()['name']) if not results: # An empty result must still come back in the requested From 458dbc353ea5ad321c32558dd43da956f3cc3eba Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 24 Sep 2026 12:23:49 +0100 Subject: [PATCH 7/7] Use the canonical codec name, and write single JSON/XML documents as is The output encoding preference is free text, and codecs.lookup() accepts aliases, any case and stray whitespace, whilst the BOM decisions and the charset header were made on the raw text. 'utf-16 ' therefore got a hand-prepended BOM on top of the codec's own, 'u8' got none, and odd whitespace went straight into the Content-Type header. Everything now uses the name lookup() resolves to. Issue #3205 asks for a single JSON or XML value to be written as it is, but a lone json or jsonb value saved as JSON came out as a quoted string, since the loaders hand those types over as text, and a lone xml value saved as XML was escaped inside a wrapper. Both are now written directly. --- web/pgadmin/tools/sqleditor/__init__.py | 13 ++-- .../tests/test_download_csv_query_tool.py | 73 ++++++++++++++++--- .../utils/driver/psycopg3/connection.py | 26 ++++++- 3 files changed, 93 insertions(+), 19 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 1122ee3c676..b7b5bca9087 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2196,8 +2196,11 @@ def start_query_download_tool(trans_id): # Validate the (free-text, user-configurable) encoding up front so # an invalid codec returns a clean 400 here, rather than raising a # LookupError mid-stream after the 200 Response has been returned. + # Carry on with the codec's canonical name rather than the raw text: + # lookup() accepts aliases ('u8'), case and stray whitespace, and + # the BOM decisions and the charset header below need the real name. try: - codecs.lookup(output_encoding) + output_encoding = codecs.lookup(output_encoding).name except LookupError: return make_json_response( status=400, @@ -2207,17 +2210,15 @@ def start_query_download_tool(trans_id): ).format(output_encoding) ) - normalized_encoding = output_encoding.lower().replace( - '-', '').replace('_', '') - is_utf = normalized_encoding.startswith('utf') + is_utf = output_encoding.startswith('utf') # The 'utf-16' and 'utf-32' codecs (without an explicit endianness # suffix) and 'utf-8-sig' emit their own BOM, so we must not # hand-prepend one too; doing so would produce two BOMs and corrupt # the output. The explicit-endian forms (utf-16-le/-be, # utf-32-le/-be) and utf-8 do not self-emit a BOM, so for those we # keep writing it ourselves. - codec_self_emits_bom = normalized_encoding in ( - 'utf16', 'utf32', 'utf8sig') + codec_self_emits_bom = output_encoding in ( + 'utf-16', 'utf-32', 'utf-8-sig') str_gen = gen(conn_obj, trans_obj, 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 821d502e1bc..077aed16448 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 @@ -328,6 +328,23 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_content_type='text/csv', expected_extension='.csv') ), + ( + # The encoding is free text, and codecs.lookup() accepts aliases + # and stray whitespace, so the BOM decision must be made on the + # codec's real name: 'u8' is utf-8 and still gets its BOM... + 'Download CSV with an aliased utf-8 encoding has a BOM', + dict(data_format='csv', add_bom=True, encoding='u8', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + # ...and 'utf-16 ' is utf-16, which self-emits its BOM, so it + # must not be given a second one. + 'Download CSV as utf-16 with trailing space has one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-16 ', + expected_content_type='text/csv', + expected_extension='.csv') + ), ( # JSON and XML are streamed a fetched batch at a time, so a # result spanning several batches must still join up into one @@ -408,6 +425,33 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_extension='.xml', sql='SELECT 42 as "Value"', single_value=True) ), + ( + # A single json, jsonb or xml value is already a document in + # the requested format, so it is written as it is rather than + # being quoted as a string or escaped as text. + 'Download a single json value as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT \'{"a": [1, 2]}\'::json as "Value"', + single_value=True, single_value_expected={'a': [1, 2]}) + ), + ( + 'Download a single jsonb value as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT \'{"a": [1, 2]}\'::jsonb as "Value"', + single_value=True, single_value_expected={'a': [1, 2]}) + ), + ( + 'Download a single xml value as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT \'x\'::xml as "Value"', + single_value=True, single_xml_document=True) + ), ( # A single-row, single-column NULL is still the direct-value # shape, i.e. a bare JSON null / an empty element with @@ -451,6 +495,8 @@ class TestDownloadResultFormats(BaseTestGenerator): awkward_data = False single_value = False single_value_is_null = False + single_value_expected = 42 + single_xml_document = False empty_result = False filename_override = None non_latin1_char = None @@ -520,10 +566,17 @@ def _assert_single_value(self, body): if self.single_value_is_null: self.assertIsNone(parsed) else: - self.assertEqual(parsed, 42) + self.assertEqual(parsed, self.single_value_expected) return root = ElementTree.fromstring(body) + if self.single_xml_document: + # The xml value is the whole document, with no wrapper. + self.assertEqual(root.tag, 'item') + self.assertEqual(root.get('id'), '1') + self.assertEqual(root.text, 'x') + return + self.assertEqual(root.tag, 'data_output') # No row/column wrapper, and no column name anywhere in sight. self.assertIsNone(root.find('row')) @@ -595,7 +648,10 @@ def runTest(self): self.assertEqual(response.status_code, 200) self.assertIn(self.expected_content_type, headers['Content-Type']) - self.assertIn('charset={0}'.format(self.encoding), + # The header carries the codec's canonical name, not the raw + # preference text, which may be an alias or hold stray whitespace. + canonical = codecs.lookup(self.encoding).name + self.assertIn('charset={0}'.format(canonical), headers['Content-Type']) disposition = headers['Content-Disposition'] try: @@ -611,17 +667,16 @@ def runTest(self): self.assertIn('filename="{0}"'.format(filename), disposition) raw = response.data - normalized = self.encoding.lower().replace('-', '').replace('_', '') - if self.add_bom and normalized.startswith('utf'): + if self.add_bom and canonical.startswith('utf'): # The output must carry exactly one BOM for the encoding, never # two (which happened when a BOM was hand-prepended for codecs # that already self-emit one, e.g. utf-16/utf-32). bom = { - 'utf8': codecs.BOM_UTF8, - 'utf8sig': codecs.BOM_UTF8, - 'utf16': codecs.BOM_UTF16, - 'utf32': codecs.BOM_UTF32, - }[normalized] + 'utf-8': codecs.BOM_UTF8, + 'utf-8-sig': codecs.BOM_UTF8, + 'utf-16': codecs.BOM_UTF16, + 'utf-32': codecs.BOM_UTF32, + }[canonical] self.assertTrue(raw.startswith(bom)) # No second, redundant BOM immediately after the first. self.assertFalse(raw[len(bom):].startswith(bom)) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index baef6304c78..afcc7bd545f 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -39,7 +39,7 @@ from .typecast import register_binary_data_typecasters,\ register_global_typecasters, register_string_typecasters,\ register_binary_typecasters, register_array_to_string_typecasters,\ - register_numeric_typecasters + register_numeric_typecasters, PSYCOPG_SUPPORTED_JSON_TYPES from .encoding import get_encoding, configure_driver_encodings from pgadmin.utils.text_sanitize import sanitize_external_text from pgadmin.utils import csv_lib as csv @@ -178,12 +178,26 @@ def _generate_xml(cur, records, results, header): yield '' -def _generate_single_value(data_format, value): +# The xml type's OID. +PG_XML_TYPE = 142 + + +def _generate_single_value(data_format, value, type_code=None): """Render a genuine single-row, single-column result directly, per issue #3205: no array wrapper for JSON, no / wrapper for XML, just the value itself. NULL is reported the same way it is elsewhere: JSON null, or an empty element with null="true". + + A json or jsonb value saved as JSON, or an xml value saved as XML, is + already a document in that format (the loaders hand them over as + text), so it is written as it is rather than quoted or escaped. """ + if value is not None and ( + (data_format == 'json' and + type_code in PSYCOPG_SUPPORTED_JSON_TYPES) or + (data_format == 'xml' and type_code == PG_XML_TYPE)): + return value + if data_format == 'json': return json.dumps( _json_safe(value), default=_json_default, allow_nan=False) @@ -1080,10 +1094,13 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", results = cur.fetchmany(records) header = [] + type_codes = [] for c in cur.ordered_description(): # This is to handle the case in which column name is non-ascii - header.append(c.to_dict()['name']) + column = c.to_dict() + header.append(column['name']) + type_codes.append(column['type_code']) if not results: # An empty result must still come back in the requested @@ -1111,7 +1128,8 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", more = cur.fetchmany(records) if not more: yield _generate_single_value( - data_format, results[0].get(header[0])) + data_format, results[0].get(header[0]), + type_codes[0]) return results = results + more