webapp/pgadmin4
1
0
Fork 0
mirror of synced 2026-10-23 07:08:55 +03:00
Code Activity

fix: stop Schema Diff injecting whitespace into applied function/procedure bodies (#10302) (#10317)

Browse Source 
Generating a script for a function or procedure that only exists on the
source ("Source Only") routed through FunctionView.sql(), which always
calls reformat_prosrc_code() to wrap the body with a leading and trailing
newline around $BODY$...$BODY$ purely for readability in the SQL panel.
That reformatted body was then executed verbatim against the target, so
PostgreSQL stored the extra newlines in prosrc: the target's body no
longer matched the source's byte for byte, and re-comparing kept
reporting a whitespace-only difference unless "Ignore whitespace" was
ticked.

Schema Diff's update path (CREATE OR REPLACE against an existing target
object) already suppressed this formatting via allow_code_formatting,
added for #7102; this extends the same guard to the create/"Source
Only" path so Schema Diff never adds whitespace of its own to the SQL
it actually applies.

Added a dedicated regression test covering both the "Source Only" and
"Different" cases for a function and a procedure: applying the
generated script now leaves every object comparing as Identical
without needing "Ignore whitespace".
This commit is contained in:
Dave Page
2026-09-07 12:11:35 +01:00
committed by GitHub
parent f2fdef5ba5
commit abf926428e
2 changed files with 250 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py
View File

@@ -913,6 +913,7 @@ class FunctionView(PGChildNodeView, DataTypeReader, SchemaDiffObjectCompare):
"""
json_resp = kwargs.get('json_resp', True)
target_schema = kwargs.get('target_schema', None)
allow_code_formatting = kwargs.get('allow_code_formatting', True)
resp_data = self._fetch_properties(gid, sid, did, scid, fnid)
# Most probably this is due to error
@@ -936,7 +937,8 @@ class FunctionView(PGChildNodeView, DataTypeReader, SchemaDiffObjectCompare):
resp_data['func_args_without'] = ', '.join(args_without_name)
self.reformat_prosrc_code(resp_data)
if allow_code_formatting:
self.reformat_prosrc_code(resp_data)
if self.node_type == 'procedure':
object_type = 'procedure'
@@ -1700,8 +1702,18 @@ class FunctionView(PGChildNodeView, DataTypeReader, SchemaDiffObjectCompare):
sql = self.delete(gid=gid, sid=sid, did=did,
scid=scid, fnid=oid, only_sql=True)
elif target_schema:
# This SQL is the one that will actually be executed
# against the target (e.g. to create a "Source Only"
# object there), so it must reproduce the body exactly
# as stored, without the cosmetic newlines added around
# $BODY$ for readability in the SQL panel. Otherwise the
# target's prosrc would differ from the source's by
# nothing but pgAdmin's own formatting, and a subsequent
# comparison would keep reporting a (whitespace-only)
# difference (see #10302).
sql = self.sql(gid=gid, sid=sid, did=did, scid=scid, fnid=oid,
target_schema=target_schema, json_resp=False)
target_schema=target_schema, json_resp=False,
allow_code_formatting=False)
else:
sql = self.sql(gid=gid, sid=sid, did=did, scid=scid, fnid=oid,
json_resp=False)

236
web/pgadmin/tools/schema_diff/tests/test_schema_diff_function_body.py Normal file
View File

@@ -0,0 +1,236 @@
##########################################################################
#
# pgAdmin 4 - PostgreSQL Tools
#
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
# This software is released under the PostgreSQL Licence
#
##########################################################################
"""Schema Diff tests for function/procedure body whitespace (#10302).
Schema Diff's own reverse-engineered CREATE OR REPLACE wraps a function or
procedure body in a leading and trailing newline around $BODY$...$BODY$ so
that it reads nicely in the SQL panel. That cosmetic reformatting must not
leak into the SQL that Schema Diff actually applies to the target: doing so
changes the stored prosrc (PostgreSQL keeps the body exactly as written
between the dollar quotes), so the target would forever compare as
different from the source by nothing but pgAdmin's own added whitespace,
even though "Ignore whitespace" is unticked.
This covers both ways a body ends up applied through Schema Diff: creating
an object that only exists in the source ("Source Only"), and replacing one
that exists in both but with a different body ("Different").
"""
import json
import secrets
import uuid
from pgadmin.utils.route import BaseSocketTestGenerator
from regression import parent_node_dict
from regression.python_test_utils import test_utils as utils
SCHEMA_NAME = 'test_func_body_diff'
# proc_added/func_added exist only in the source (Source Only status).
# proc_changed/func_changed exist in both, with a different body
# (Different status), so applying the diff is a CREATE OR REPLACE.
SOURCE_DDL = """
CREATE SCHEMA {0};
CREATE PROCEDURE {0}.proc_added(arg1 bigint)
LANGUAGE sql
AS $$select 1;$$;
CREATE FUNCTION {0}.func_added(arg1 bigint)
RETURNS bigint
LANGUAGE sql
AS $$select arg1;$$;
CREATE PROCEDURE {0}.proc_changed(arg1 bigint)
LANGUAGE sql
AS $$select 1;$$;
CREATE FUNCTION {0}.func_changed(arg1 bigint)
RETURNS bigint
LANGUAGE sql
AS $$select arg1;$$;
"""
TARGET_DDL = """
CREATE SCHEMA {0};
CREATE PROCEDURE {0}.proc_changed(arg1 bigint)
LANGUAGE sql
AS $$select 2;$$;
CREATE FUNCTION {0}.func_changed(arg1 bigint)
RETURNS bigint
LANGUAGE sql
AS $$select arg1 + 1;$$;
"""
class SchemaDiffFunctionBodyTestCase(BaseSocketTestGenerator):
""" This class will test Schema Diff for function/procedure bodies. """
scenarios = [
('Schema diff comparison of function/procedure bodies', dict())
]
SOCKET_NAMESPACE = '/schema_diff'
def setUp(self):
super().setUp()
self.src_database = "db_func_body_diff_src_%s" % str(
uuid.uuid4())[1:8]
self.tar_database = "db_func_body_diff_tar_%s" % str(
uuid.uuid4())[1:8]
self.src_db_id = utils.create_database(self.server, self.src_database)
self.tar_db_id = utils.create_database(self.server, self.tar_database)
self.server = parent_node_dict["server"][-1]["server"]
self.server_id = parent_node_dict["server"][-1]["server_id"]
self.execute_sql(self.src_database, SOURCE_DDL.format(SCHEMA_NAME))
self.execute_sql(self.tar_database, TARGET_DDL.format(SCHEMA_NAME))
def execute_sql(self, db_name, sql):
"""
Run a statement batch against one of the test databases.
:param db_name: Database to run against
:param sql: SQL to execute
"""
connection = utils.get_db_connection(db_name,
self.server['username'],
self.server['db_password'],
self.server['host'],
self.server['port'],
self.server['sslmode'])
old_isolation_level = connection.isolation_level
utils.set_isolation_level(connection, 0)
pg_cursor = connection.cursor()
pg_cursor.execute(sql)
utils.set_isolation_level(connection, old_isolation_level)
connection.commit()
connection.close()
def compare(self):
"""
Compare the two test databases and return the result.
:return: List of compared objects
"""
data = {
'trans_id': self.trans_id,
'source_sid': self.server_id,
'source_did': self.src_db_id,
'target_sid': self.server_id,
'target_did': self.tar_db_id,
'ignore_owner': 0,
'ignore_whitespaces': 0,
'ignore_tablespace': 0,
'ignore_grants': 0
}
self.socket_client.emit('compare_database', data,
namespace=self.SOCKET_NAMESPACE)
received = self.socket_client.get_received(self.SOCKET_NAMESPACE)
response_data = received[-1]['args'][0]
self.assertEqual(received[-1]['name'], "compare_database_success",
response_data)
return response_data
def find_object(self, response_data, node_type, title_prefix):
"""
Pick a single compared object out of the comparison result.
:param response_data: Result of compare()
:param node_type: Node type, e.g. 'procedure'
:param title_prefix: Object name (without argument list)
:return: The compared object
"""
for diff in response_data:
if diff.get('type') == node_type and \
diff.get('title', '').startswith(title_prefix + '('):
return diff
self.fail('{0} {1} was not compared'.format(node_type, title_prefix))
def assert_no_injected_whitespace(self, diff_ddl):
"""
The applied SQL must reproduce the body exactly as stored: no
leading/trailing newline injected around $BODY$ purely for
readability in the SQL panel (#10302).
"""
self.assertNotIn('$BODY$\n', diff_ddl,
'Schema Diff injected a leading newline into the '
'function/procedure body: {0}'.format(diff_ddl))
self.assertNotIn('\n$BODY$;', diff_ddl,
'Schema Diff injected a trailing newline into the '
'function/procedure body: {0}'.format(diff_ddl))
def runTest(self):
""" Test Schema Diff for function/procedure body whitespace. """
self.trans_id = str(secrets.choice(range(1, 99999)))
response = self.tester.get(
'schema_diff/initialize/{}'.format(self.trans_id))
self.assertEqual(response.status_code, 200)
received = self.socket_client.get_received(self.SOCKET_NAMESPACE)
self.assertEqual(received[0]['name'], 'connected')
self.tester.post(
'schema_diff/server/connect/{}'.format(self.server_id),
data=json.dumps({'password': self.server['db_password']}),
content_type='html/json')
self.tester.post('schema_diff/database/connect/{0}/{1}'.format(
self.server_id, self.src_db_id))
self.tester.post('schema_diff/database/connect/{0}/{1}'.format(
self.server_id, self.tar_db_id))
response_data = self.compare()
cases = [
('procedure', 'proc_added', 'Source Only'),
('function', 'func_added', 'Source Only'),
('procedure', 'proc_changed', 'Different'),
('function', 'func_changed', 'Different'),
]
diff_ddls = {}
for node_type, name, expected_status in cases:
diff = self.find_object(response_data, node_type, name)
self.assertEqual(diff['status'], expected_status,
'{0} {1} was reported as {2}, expected {3}'
.format(node_type, name, diff['status'],
expected_status))
self.assert_no_injected_whitespace(diff['diff_ddl'])
diff_ddls[name] = diff['diff_ddl']
# Apply every generated script to the target, then re-compare: all
# four objects must now settle as identical, without needing
# "Ignore whitespace" ticked.
for name in diff_ddls:
self.execute_sql(self.tar_database, diff_ddls[name])
response_data = self.compare()
for node_type, name, _ in cases:
diff = self.find_object(response_data, node_type, name)
self.assertEqual(diff['status'], 'Identical',
'{0} {1} still compares as {2} after applying '
'the generated script: {3}'
.format(node_type, name, diff['status'],
diff.get('diff_ddl')))
def tearDown(self):
"""This function drops the added databases"""
super().tearDown()
for db_name in (self.src_database, self.tar_database):
connection = utils.get_db_connection(self.server['db'],
self.server['username'],
self.server['db_password'],
self.server['host'],
self.server['port'],
self.server['sslmode'])
utils.drop_database(connection, db_name)