-
Notifications
You must be signed in to change notification settings - Fork 829
Added support to download binary data from result grid. #4011 #9645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| import secrets | ||
| from urllib.parse import unquote | ||
| from threading import Lock | ||
| from io import BytesIO | ||
| import threading | ||
| import math | ||
|
|
||
|
|
@@ -23,7 +24,8 @@ | |
|
|
||
| from config import PG_DEFAULT_DRIVER, ALLOW_SAVE_PASSWORD | ||
| from werkzeug.user_agent import UserAgent | ||
| from flask import Response, url_for, render_template, session, current_app | ||
| from flask import Response, url_for, render_template, session, current_app, \ | ||
| send_file | ||
| from flask import request | ||
| from flask_babel import gettext | ||
| from pgadmin.tools.sqleditor.utils.query_tool_connection_check \ | ||
|
|
@@ -70,6 +72,8 @@ | |
| from pgadmin.browser.server_groups.servers.utils import \ | ||
| convert_connection_parameter, get_db_disp_restriction | ||
| from pgadmin.misc.workspaces import check_and_delete_adhoc_server | ||
| from pgadmin.utils.driver.psycopg3.typecast import \ | ||
| register_binary_data_typecasters | ||
|
|
||
| MODULE_NAME = 'sqleditor' | ||
| TRANSACTION_STATUS_CHECK_FAILED = gettext("Transaction status check failed.") | ||
|
|
@@ -147,6 +151,7 @@ def get_exposed_url_endpoints(self): | |
| 'sqleditor.server_cursor', | ||
| 'sqleditor.nlq_chat_stream', | ||
| 'sqleditor.explain_analyze_stream', | ||
| 'sqleditor.download_binary_data', | ||
| ] | ||
|
|
||
| def on_logout(self): | ||
|
|
@@ -2182,6 +2187,77 @@ def start_query_download_tool(trans_id): | |
| return internal_server_error(errormsg=err_msg) | ||
|
|
||
|
|
||
| @blueprint.route( | ||
| '/download_binary_data/<int:trans_id>', | ||
| methods=["POST"], endpoint='download_binary_data' | ||
| ) | ||
| @pga_login_required | ||
| def download_binary_data(trans_id): | ||
| """ | ||
| This method is used to download binary data. | ||
| """ | ||
|
|
||
| (status, error_msg, conn, trans_obj, | ||
| session_obj) = check_transaction_status(trans_id) | ||
|
|
||
| if error_msg: | ||
| return internal_server_error( | ||
| errormsg=error_msg | ||
| ) | ||
|
|
||
| if not status or conn is None or trans_obj is None or \ | ||
| session_obj is None: | ||
| return internal_server_error( | ||
| errormsg=TRANSACTION_STATUS_CHECK_FAILED | ||
| ) | ||
|
|
||
| cur = conn._Connection__async_cursor | ||
| if cur is None: | ||
| return internal_server_error( | ||
| errormsg=gettext('No active result cursor.') | ||
| ) | ||
| register_binary_data_typecasters(cur) | ||
|
Comment on lines
+2219
to
+2224
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Locate and examine register_binary_data_typecasters implementation
fd "typecast" web/pgadmin/ --type f | head -20Repository: pgadmin-org/pgadmin4 Length of output: 109 🏁 Script executed: # Search for the function definition and usage
rg -n "def register_binary_data_typecasters" web/pgadmin/ -A 15Repository: pgadmin-org/pgadmin4 Length of output: 1378 🏁 Script executed: # Check the typecast module in psycopg3 driver
find web/pgadmin -name "*.py" -path "*/psycopg3/*" | xargs grep -l "ByteaDataLoader\|register.*typecaster" | head -5Repository: pgadmin-org/pgadmin4 Length of output: 157 🏁 Script executed: # Check how async_cursor is used in fetch operations
rg -n "__async_cursor" web/pgadmin/tools/sqleditor/__init__.py -B 3 -A 3 | head -80Repository: pgadmin-org/pgadmin4 Length of output: 739 🏁 Script executed: # Check if there's cleanup or restoration of adapters after binary download
rg -n "register_binary_data_typecasters" web/pgadmin/tools/sqleditor/__init__.py -B 10 -A 10Repository: pgadmin-org/pgadmin4 Length of output: 1740 🏁 Script executed: # Get the full function context around line 2224
sed -n '2200,2280p' web/pgadmin/tools/sqleditor/__init__.pyRepository: pgadmin-org/pgadmin4 Length of output: 2469 🏁 Script executed: # Check what function contains line 2224
rg -n "^def " web/pgadmin/tools/sqleditor/__init__.py | awk -F: '$1 <= 2224' | tail -1Repository: pgadmin-org/pgadmin4 Length of output: 104 🏁 Script executed: # Search for any adapter reset or restoration in the file
rg -n "adapters\|ByteaDataLoader\|ByteaBinaryDataLoader" web/pgadmin/tools/sqleditor/__init__.pyRepository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Check the typecast module to understand the loaders
rg -n "class ByteaDataLoader\|class ByteaBinaryDataLoader" web/pgadmin/utils/driver/psycopg3/typecast.py -A 10Repository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Get the full typecast.py file to understand ByteaDataLoader and ByteaBinaryDataLoader
wc -l web/pgadmin/utils/driver/psycopg3/typecast.pyRepository: pgadmin-org/pgadmin4 Length of output: 113 🏁 Script executed: # Check the loaders
sed -n '1,230p' web/pgadmin/utils/driver/psycopg3/typecast.pyRepository: pgadmin-org/pgadmin4 Length of output: 7820 🏁 Script executed: # Search for any documentation or comments about ByteaBinaryDataLoader
rg -n "ByteaBinaryDataLoader" web/pgadmin/utils/driver/psycopg3/typecast.py -B 5 -A 10Repository: pgadmin-org/pgadmin4 Length of output: 1118 🏁 Script executed: # Check if there are any other calls to register_binary_data_typecasters or adapter cleanup
rg -n "register_binary_data_typecasters\|adapters\.register\|adapters\.deregister" web/pgadmin/ -iRepository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Check the full ByteaDataLoader to understand what it returns
sed -n '258,276p' web/pgadmin/utils/driver/psycopg3/typecast.pyRepository: pgadmin-org/pgadmin4 Length of output: 688 🏁 Script executed: # Check if there's a separate cursor creation for binary download or cleanup after
rg -n "download_binary_data" web/pgadmin/tools/sqleditor/__init__.py -A 100 | grep -E "adapters|cursor|finally|cleanup" -iRepository: pgadmin-org/pgadmin4 Length of output: 348 🏁 Script executed: # Check if fetch_window or other grid operations might reuse the same cursor
rg -n "def fetch_window\|def async_fetchmany_2darray" web/pgadmin/tools/sqleditor/__init__.py -A 20 | head -80Repository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Get the complete download_binary_data function with context
sed -n '2195,2270p' web/pgadmin/tools/sqleditor/__init__.pyRepository: pgadmin-org/pgadmin4 Length of output: 2273 🏁 Script executed: # Check if there's any adapter cleanup or cursor reset logic elsewhere
rg -n "def query_tool\|def fetch_window\|adapters\." web/pgadmin/tools/sqleditor/__init__.py | head -40Repository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Check what the grid display expects for bytea columns
rg -n "bytea\|binary" web/pgadmin/tools/sqleditor/ -i --type js | head -30Repository: pgadmin-org/pgadmin4 Length of output: 46 🏁 Script executed: # Check if the async cursor is created fresh for each operation or persisted
rg -n "__async_cursor\s*=" web/pgadmin/utils/driver/psycopg3/ -B 3 -A 3Repository: pgadmin-org/pgadmin4 Length of output: 1859
Consider either: (a) saving and restoring the original adapters after the binary fetch, or (b) using a separate cursor for the binary download. 🤖 Prompt for AI Agents |
||
|
|
||
| data = request.values if request.values else request.get_json(silent=True) | ||
| if data is None: | ||
| return make_json_response( | ||
| status=410, | ||
| success=0, | ||
| errormsg=gettext( | ||
| "Could not find the required parameter (query)." | ||
| ) | ||
| ) | ||
|
Comment on lines
+2226
to
+2234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Misleading error message — copy-pasted from another endpoint. The error text ✏️ Suggested fix errormsg=gettext(
- "Could not find the required parameter (query)."
+ "Could not find the required parameters (rowpos, colpos)."
)🤖 Prompt for AI Agents |
||
| col_pos = data['colpos'] | ||
| cur.scroll(int(data['rowpos'])) | ||
| binary_data = cur.fetchone() | ||
| binary_data = binary_data[col_pos] | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+2235
to
+2238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Unvalidated duplicate code executes before the Lines 2235–2238 scroll the cursor and fetch a row without any validation or error handling, then lines 2240–2256 repeat the same logic properly inside a
Remove these four lines entirely. 🐛 Proposed fix register_binary_data_typecasters(cur)
data = request.values if request.values else request.get_json(silent=True)
if data is None:
return make_json_response(
status=410,
success=0,
errormsg=gettext(
"Could not find the required parameter (query)."
)
)
- col_pos = data['colpos']
- cur.scroll(int(data['rowpos']))
- binary_data = cur.fetchone()
- binary_data = binary_data[col_pos]
-
try:
row_pos = int(data['rowpos'])🤖 Prompt for AI Agents |
||
|
|
||
| try: | ||
| row_pos = int(data['rowpos']) | ||
| col_pos = int(data['colpos']) | ||
| if row_pos < 0 or col_pos < 0: | ||
| raise ValueError | ||
| cur.scroll(row_pos) | ||
| row = cur.fetchone() | ||
| if row is None or col_pos >= len(row): | ||
| return internal_server_error( | ||
| errormsg=gettext('Requested cell is out of range.') | ||
| ) | ||
| binary_data = row[col_pos] | ||
| except (ValueError, IndexError, TypeError) as e: | ||
| current_app.logger.error(e) | ||
| return internal_server_error( | ||
| errormsg='Invalid row/column position.' | ||
| ) | ||
|
|
||
| return send_file( | ||
| BytesIO(binary_data), | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| as_attachment=True, | ||
| download_name='binary_data', | ||
| mimetype='application/octet-stream' | ||
| ) | ||
|
|
||
|
|
||
| @blueprint.route( | ||
| '/status/<int:trans_id>', | ||
| methods=["GET"], | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.