Skip to content

Fix UDF/procedure argument grid delete (and add) in edit mode #10333

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.

Sign up for GitHub

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

Open
dpage wants to merge 3 commits into
base: master
Choose a base branch
from
+202 −17
Open

Fix UDF/procedure argument grid delete (and add) in edit mode #10333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a46a16
fix: allow deleting/adding UDF argument rows added in the current edi…
dpage Aug 19, 2026
abb615d
fix: reject adding a new argument to an existing function/procedure
dpage Aug 20, 2026
f0bfe43
fix: distinguish argmode in the added-argument edit-mode guard
dpage Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion web/pgadmin/browser/server_groups/servers/databases/schemas/functions/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ def _update_arguments_for_get_sql(data, 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)
Expand Down Expand Up @@ -1194,6 +1194,36 @@ 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 argument to an existing function/
# procedure via CREATE OR REPLACE. Adding an IN/INOUT/VARIADIC
# argument changes the routine's signature, so PostgreSQL
# creates a new, separate overloaded routine instead of
# replacing this one. Adding an OUT argument does not affect
# the signature, but it changes the shape of the returned row,
# which PostgreSQL rejects outright (SQLSTATE 42P13). Reject
# both cases explicitly, rather than silently leaving an
# orphaned routine behind or letting the database error surface.
if 'arguments' in data and isinstance(data['arguments'], dict) \
and data['arguments'].get('added'):
added_args = data['arguments']['added']
if any(
(a.get('argmode') or 'IN') != 'OUT' for a in added_args
):
return False, gettext(
"Adding a new IN/INOUT/VARIADIC 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."
), ''
else:
return False, gettext(
"Adding a new OUT argument to an existing function/"
"procedure is not supported, as it would change the "
"shape of the returned row. 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
Expand Down
12 changes: 6 additions & 6 deletions ...gadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
87 changes: 87 additions & 0 deletions ...browser/server_groups/servers/databases/schemas/functions/tests/test_function_get_msql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,87 @@ class FunctionGetmsqlTestCase(BaseTestGenerator):
}
)
),
(
'Fetch Function msql with newly added IN argument is rejected',
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": [],
# PostgreSQL cannot add an IN 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",
"argtype": "integer",
"argmode": "IN",
"argdefval": "1"
}]
})
},
mock_data={},
expected_data={
"status_code": 500,
"check_errormsg": "overloaded"
}
),
),
(
'Fetch Function msql with newly added OUT argument is '
'rejected',
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": [],
# Unlike an added IN/INOUT/VARIADIC argument, an added
# OUT argument does not change the function's
# identity/signature, but it does change the shape of
# the returned row, which PostgreSQL rejects outright
# (SQLSTATE 42P13). This must be rejected with a
# distinct, accurate error message.
"arguments": json.dumps({
"added": [{
"argname": "new_out_arg",
"argtype": "integer",
"argmode": "OUT"
}]
})
},
mock_data={},
expected_data={
"status_code": 500,
"check_errormsg": "returned row"
}
),
),
(
'Fetch Function msql fetch properties not found',
dict(
Expand Down Expand Up @@ -222,5 +303,11 @@ 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'])
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)
88 changes: 78 additions & 10 deletions ...min/browser/server_groups/servers/databases/schemas/functions/tests/test_procedure_put.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,77 @@ class ProcedurePutTestCase(BaseTestGenerator):
""" This class will update new procedure under schema node. """
scenarios = [
# Fetching default URL for procedure node.
('Fetch Procedure Node URL',
dict(url='/browser/procedure/obj/'))
('Fetch Procedure Node URL', dict(
url='/browser/procedure/obj/',
is_add_argument=False,
expected_data={
"status_code": 200
}
)),
(
'Fetch Procedure update with newly added IN argument is '
'rejected',
dict(
url='/browser/procedure/obj/',
# PostgreSQL cannot add an IN argument to an existing
# procedure 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.
is_add_argument=True,
test_data={
"arguments": {
"added": [{
"argname": "new_arg",
"argtype": "integer",
"argmode": "IN",
}]
}
},
expected_data={
"status_code": 500,
"check_errormsg": "overloaded"
}
),
),
(
'Fetch Procedure update with newly added OUT argument is '
'rejected',
dict(
url='/browser/procedure/obj/',
# Unlike an added IN/INOUT/VARIADIC argument, an added
# OUT argument does not change the procedure's
# identity/signature, but it does change the shape of the
# returned row, which PostgreSQL rejects outright
# (SQLSTATE 42P13). This must be rejected with a
# distinct, accurate error message.
is_add_argument=True,
test_data={
"arguments": {
"added": [{
"argname": "new_out_arg",
"argtype": "integer",
"argmode": "OUT",
}]
}
},
expected_data={
"status_code": 500,
"check_errormsg": "returned row"
}
),
),
]

def update_procedure(self, proc_id, data):
return self.tester.put(
self.url + str(utils.SERVER_GROUP) +
'/' + str(self.server_id) + '/' + str(self.db_id) + '/' +
str(self.schema_id) + '/' +
str(proc_id),
data=json.dumps(data),
follow_redirects=True)

def runTest(self):
""" This function will update procedure under database node. """
super().setUp()
Expand All @@ -47,14 +114,15 @@ def runTest(self):
"dependsonextensions": ["plpgsql"]
}

put_response = self.tester.put(
self.url + str(utils.SERVER_GROUP) +
'/' + str(self.server_id) + '/' + str(self.db_id) + '/' +
str(self.schema_id) + '/' +
str(proc_id),
data=json.dumps(data),
follow_redirects=True)
self.assertEqual(put_response.status_code, 200)
if getattr(self, 'is_add_argument', False):
data['arguments'] = self.test_data['arguments']

response = self.update_procedure(proc_id, data)
self.assertEqual(response.status_code,
self.expected_data['status_code'])
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)

Expand Down
Loading