From 371fc76d9ebee170441252694a28ded62ad22110 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 11:14:38 +0100 Subject: [PATCH 1/2] Fix syntax error editing SQL functions/procedures with a 'return' substring in the body. The SQL-standard body detection used a regex that matched 'return' anywhere in the body, so a plain SQL body containing a RETURNING clause (or an identifier like 'returned_value') was wrongly treated as a SQL-standard (BEGIN ATOMIC / RETURN) body. The generated CREATE OR REPLACE statement then dropped the AS $BODY$ ... $BODY$ wrapper, producing invalid SQL and a syntax error on save. Anchor the RETURN form to the start of the body so only genuine SQL-standard bodies are detected. Add unit tests for the detection. Closes #10059 --- docs/en_US/release_notes_9_16.rst | 1 + .../databases/schemas/functions/__init__.py | 7 +- .../test_is_function_def_sql_standard.py | 69 +++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py diff --git a/docs/en_US/release_notes_9_16.rst b/docs/en_US/release_notes_9_16.rst index 4f0740cdff0..45b7b519110 100644 --- a/docs/en_US/release_notes_9_16.rst +++ b/docs/en_US/release_notes_9_16.rst @@ -54,3 +54,4 @@ Bug fixes | `Issue #9984 `_ - Fix the Docker entrypoint mishandling a quoted PGADMIN_CONFIG_CONFIG_DATABASE_URI, which caused a SQLAlchemy parse error and silently skipped PGADMIN_DEFAULT_EMAIL/PASSWORD setup. | `Issue #9987 `_ - Fix "AttributeError: 'PgAdmin' object has no attribute 'login_manager'" crash when running setup.py user-management commands (add-user, update-user) from the CLI. | `Issue #9988 `_ - Provide an actionable error when 'openid' is in OAUTH2_SCOPE but OAUTH2_SERVER_METADATA_URL is not set, instead of a cryptic Authlib failure. + | `Issue #10059 `_ - Fix the generated SQL for editing a SQL-language function/procedure whose body contains the word "return" (e.g. a RETURNING clause), which was wrongly treated as a SQL-standard body and produced a statement without the AS $BODY$ wrapper. diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py index d1f76e24360..8607386625d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py @@ -1759,10 +1759,13 @@ def _is_function_def_sql_standard(resp_data): r"^.*(?:\"|\')(?=.*(atomic)).*$"] # valid regex, these combination a must in definition to detect a - # standard sql or pure sql + # standard sql or pure sql. The RETURN form must start the body + # (as a keyword) so that a plain SQL body which merely contains the + # substring "return" (e.g. a "RETURNING" clause or an identifier + # like "returned_value") is not mistaken for a SQL-standard body. valid_match = [ r"(?=.*begin)(.+?(\n)+)(?=.*atomic)|(?=.*begin)(?=.*atomic)", - r"(?=return)" + r"^\s*return\b" ] is_func_def_sql_std = False diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py new file mode 100644 index 00000000000..5ede0b80c84 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py @@ -0,0 +1,69 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from pgadmin.utils.route import BaseTestGenerator +from .. import FunctionView + + +class TestIsFunctionDefSqlStandard(BaseTestGenerator): + """ + Unit tests for FunctionView._is_function_def_sql_standard. + + This guards against a regression (issue #10059) where a plain SQL + function/procedure body that merely contained the substring "return" + (e.g. a "RETURNING" clause) was wrongly detected as a SQL-standard + body, causing the generated CREATE OR REPLACE statement to drop the + "AS $BODY$ ... $BODY$" wrapper and fail with a syntax error. + """ + + scenarios = [ + ('SQL-standard BEGIN ATOMIC body is detected', dict( + data=dict(lanname='sql', + prosrc='\nBEGIN ATOMIC\n INSERT INTO t VALUES (1);\nEND' + ), + expected=True + )), + ('SQL-standard RETURN body is detected', dict( + data=dict(lanname='sql', prosrc='RETURN $1 + $2'), + expected=True + )), + ('SQL-standard RETURN body with leading whitespace is detected', + dict( + data=dict(lanname='sql', prosrc='\n return a;'), + expected=True + )), + ('Plain SQL body with a RETURNING clause is not SQL-standard', dict( + data=dict(lanname='sql', + prosrc='INSERT INTO t(a) VALUES (1) RETURNING a;'), + expected=False + )), + ('Plain SQL body referencing a "returned" identifier is not ' + 'SQL-standard', dict( + data=dict(lanname='sql', + prosrc='SELECT my_returned_value FROM t;'), + expected=False + )), + ('Plain SQL body is not SQL-standard', dict( + data=dict(lanname='sql', prosrc='SELECT 1;'), + expected=False + )), + ('Non-sql language is never SQL-standard', dict( + data=dict(lanname='plpgsql', + prosrc='BEGIN\n RETURN;\nEND;'), + expected=False + )), + ('Empty body is not SQL-standard', dict( + data=dict(lanname='sql', prosrc=''), + expected=False + )), + ] + + def runTest(self): + result = FunctionView._is_function_def_sql_standard(self.data) + self.assertEqual(result, self.expected) From 693101da405d2a63a12576a396bfc93b77d841dd Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 11:21:02 +0100 Subject: [PATCH 2/2] Use start-of-string anchor for SQL-standard RETURN detection. Address review feedback: \A anchors the RETURN form to the start of the body so a multi-line plain SQL body whose later line begins with 'return' is not matched under re.MULTILINE. --- .../servers/databases/schemas/functions/__init__.py | 2 +- .../functions/tests/test_is_function_def_sql_standard.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py index 8607386625d..92844825089 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py @@ -1765,7 +1765,7 @@ def _is_function_def_sql_standard(resp_data): # like "returned_value") is not mistaken for a SQL-standard body. valid_match = [ r"(?=.*begin)(.+?(\n)+)(?=.*atomic)|(?=.*begin)(?=.*atomic)", - r"^\s*return\b" + r"\A\s*return\b" ] is_func_def_sql_std = False diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py index 5ede0b80c84..eca8a4d05f0 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_is_function_def_sql_standard.py @@ -53,6 +53,10 @@ class TestIsFunctionDefSqlStandard(BaseTestGenerator): data=dict(lanname='sql', prosrc='SELECT 1;'), expected=False )), + ('RETURN must start the body, not merely a later line', dict( + data=dict(lanname='sql', prosrc='SELECT 1;\nreturn x;'), + expected=False + )), ('Non-sql language is never SQL-standard', dict( data=dict(lanname='plpgsql', prosrc='BEGIN\n RETURN;\nEND;'),