From 647e90977ebccad4a7c43929ce2538d389b8c340 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 13:45:28 +0100 Subject: [PATCH 1/2] fix: allow deleting/adding UDF argument rows added in the current edit session canDeleteRow for the function/procedure Arguments grid checked whether the whole function was new rather than whether the individual row was new, so once a function was saved the delete icon was disabled for every argument row, including ones added but not yet saved (same bug pattern already fixed for enum values in #8208). canAdd had the same whole-object gate, hiding the "+" button entirely once a function was saved, so there was no way to add a row in the first place. Pre-existing (already persisted) arguments remain non-deletable, since PostgreSQL has no way to remove an argument from a function via CREATE OR REPLACE. Also fixes _update_arguments_for_get_sql, which only merged the 'changed' key of the arguments diff and silently dropped (or, without a 'changed' key at all, raised a 500) any newly added argument, so a row added via the now-enabled "+" button actually survives into the generated SQL. Closes #10252 --- .../databases/schemas/functions/__init__.py | 10 ++++- .../functions/static/js/function.ui.js | 12 +++--- .../functions/tests/test_function_get_msql.py | 40 +++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) 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 571afd7ef01..5d1dcd0772f 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 @@ -1029,17 +1029,23 @@ def msql(self, gid, sid, did, scid, fnid=None): def _update_arguments_for_get_sql(data, old_data): """ If Function Definition/Arguments are changed then merge old - Arguments with changed ones for Create/Replace Function SQL statement + Arguments with changed/added ones for Create/Replace Function SQL + statement :param data: :param old_data: :return: """ if 'arguments' in data and len(data['arguments']) > 0: - for arg in data['arguments']['changed']: + for arg in data['arguments'].get('changed', []): for old_arg in old_data['arguments']: if arg['argid'] == old_arg['argid']: old_arg.update(arg) break + # Newly added arguments (not yet saved) need to be appended, + # otherwise they would be silently dropped as only pre-existing + # arguments are present in old_data. + for arg in data['arguments'].get('added', []): + old_data['arguments'].append(arg) data['arguments'] = old_data['arguments'] elif data['change_func']: data['arguments'] = old_data['arguments'] diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.ui.js index c6f1b649441..1e57e9d92f8 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.ui.js @@ -304,16 +304,16 @@ export default class FunctionSchema extends BaseUISchema { }, { id: 'arguments', label: gettext('Arguments'), cell: 'string', - group: gettext('Definition'), type: 'collection', canAdd: function(){ - return obj.isNew(); - }, + group: gettext('Definition'), type: 'collection', canDelete: true, mode: ['create', 'edit'], columns: ['argtype', 'argmode', 'argname', 'argdefval'], schema : new DefaultArgumentSchema(this.node_info, this.fieldOptions.getTypes), disabled: obj.inCatalog(), - canDeleteRow: function() { - return obj.isNew(); - }, + // Existing (already saved) arguments cannot be removed here, as + // PostgreSQL has no way to drop an argument from a function via + // CREATE OR REPLACE. Only rows added in the current session (not + // yet saved) can be deleted. + canDeleteRow: (state) => (this.isNew(state)), },{ id: 'prosrc', label: gettext('Code'), cell: 'text', type: 'sql', mode: ['properties', 'create', 'edit'], diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py index d574f7d46a8..333d8bca270 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py @@ -142,6 +142,43 @@ class FunctionGetmsqlTestCase(BaseTestGenerator): } ) ), + ( + 'Fetch Function msql with newly added argument', + dict( + url='/browser/function/msql/', + is_positive_test=True, + mocking_required=False, + with_function_id=True, + is_mock_local_function=False, + test_data={ + "name": "Test Function", + "funcowner": "", + "pronamespace": 2200, + "prorettypename": "character varying", + "lanname": "sql", + "prosrc": "select '1'", + "probin": "$libdir/", + "variables": [], + "seclabels": [], + "acl": [], + # A newly added (not yet saved) argument must survive + # into the generated SQL, and not be silently dropped. + "arguments": json.dumps({ + "added": [{ + "argname": "new_arg", + "argtype": "integer", + "argmode": "IN", + "argdefval": "1" + }] + }) + }, + mock_data={}, + expected_data={ + "status_code": 200, + "check_string": "new_arg" + } + ), + ), ( 'Fetch Function msql fetch properties not found', dict( @@ -222,5 +259,8 @@ def _get_sql(self, **kwargs): self.assertEqual(response.status_code, self.expected_data['status_code']) + if 'check_string' in self.expected_data: + self.assertIn(self.expected_data['check_string'], + response.json['data']) # Disconnect the database database_utils.disconnect_database(self, self.server_id, self.db_id) From 817e13ea771f1bba012816be1e47e14135452a0f Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 20 Aug 2026 05:04:37 +0100 Subject: [PATCH 2/2] fix: reject adding a new argument to an existing function/procedure CREATE OR REPLACE FUNCTION cannot add an input argument to an existing routine: PostgreSQL treats a changed argument list as a distinct signature, so it creates a separate, orphaned overloaded routine instead of replacing this one, verified against a live PostgreSQL 18 instance. The previous commit's _update_arguments_for_get_sql change merged a newly added argument straight into the CREATE OR REPLACE statement, which would have silently done exactly that. Reject the edit explicitly instead, with a clear error, rather than letting it silently leave a phantom routine behind. Updates the msql test added in the previous commit to assert the rejection instead of successful SQL generation. --- .../databases/schemas/functions/__init__.py | 23 +++++++++++++------ .../functions/tests/test_function_get_msql.py | 16 +++++++++---- 2 files changed, 27 insertions(+), 12 deletions(-) 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 5d1dcd0772f..80ba1787bd6 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 @@ -1029,8 +1029,7 @@ def msql(self, gid, sid, did, scid, fnid=None): def _update_arguments_for_get_sql(data, old_data): """ If Function Definition/Arguments are changed then merge old - Arguments with changed/added ones for Create/Replace Function SQL - statement + Arguments with changed ones for Create/Replace Function SQL statement :param data: :param old_data: :return: @@ -1041,11 +1040,6 @@ def _update_arguments_for_get_sql(data, old_data): if arg['argid'] == old_arg['argid']: old_arg.update(arg) break - # Newly added arguments (not yet saved) need to be appended, - # otherwise they would be silently dropped as only pre-existing - # arguments are present in old_data. - for arg in data['arguments'].get('added', []): - old_data['arguments'].append(arg) data['arguments'] = old_data['arguments'] elif data['change_func']: data['arguments'] = old_data['arguments'] @@ -1200,6 +1194,21 @@ def _get_sql_for_edit_mode(self, data, parallel_dict, all_ids_dict, data[arg]) > 0) or arg in data: data['change_func'] = True + # PostgreSQL cannot add an input argument to an existing + # function/procedure via CREATE OR REPLACE: a changed argument + # list is a different signature, so PostgreSQL creates a new, + # separate overloaded routine instead of replacing this one. + # Reject such edits explicitly, rather than silently leaving an + # orphaned routine behind. + if 'arguments' in data and isinstance(data['arguments'], dict) \ + and data['arguments'].get('added'): + return False, gettext( + "Adding a new argument to an existing function/procedure " + "is not supported, as PostgreSQL would create a separate, " + "overloaded routine rather than replacing this one. " + "Please create a new function/procedure instead." + ), '' + # If Function Definition/Arguments are changed then merge old # Arguments with changed ones for Create/Replace Function # SQL statement diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py index 333d8bca270..8902052b23f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py @@ -143,7 +143,7 @@ class FunctionGetmsqlTestCase(BaseTestGenerator): ) ), ( - 'Fetch Function msql with newly added argument', + 'Fetch Function msql with newly added argument is rejected', dict( url='/browser/function/msql/', is_positive_test=True, @@ -161,8 +161,11 @@ class FunctionGetmsqlTestCase(BaseTestGenerator): "variables": [], "seclabels": [], "acl": [], - # A newly added (not yet saved) argument must survive - # into the generated SQL, and not be silently dropped. + # PostgreSQL cannot add an argument to an existing + # function via CREATE OR REPLACE (it would create a + # separate, overloaded routine instead), so this must + # be rejected with a clear error rather than silently + # producing SQL that orphans a routine. "arguments": json.dumps({ "added": [{ "argname": "new_arg", @@ -174,8 +177,8 @@ class FunctionGetmsqlTestCase(BaseTestGenerator): }, mock_data={}, expected_data={ - "status_code": 200, - "check_string": "new_arg" + "status_code": 500, + "check_errormsg": "not supported" } ), ), @@ -262,5 +265,8 @@ def _get_sql(self, **kwargs): if 'check_string' in self.expected_data: self.assertIn(self.expected_data['check_string'], response.json['data']) + if 'check_errormsg' in self.expected_data: + self.assertIn(self.expected_data['check_errormsg'], + response.json['errormsg']) # Disconnect the database database_utils.disconnect_database(self, self.server_id, self.db_id)