Skip to content

MDEV-39993 Use CREATE OR REPLACE in sys schema to preserve grants dur…#5244

Open
akshatnehra wants to merge 1 commit into
MariaDB:mainfrom
akshatnehra:MDEV-39993
Open

MDEV-39993 Use CREATE OR REPLACE in sys schema to preserve grants dur…#5244
akshatnehra wants to merge 1 commit into
MariaDB:mainfrom
akshatnehra:MDEV-39993

Conversation

@akshatnehra

Copy link
Copy Markdown
Contributor

Description

This PR fixes MDEV-39993 where mariadb-upgrade drops existing EXECUTE grants on sys schema stored routines during upgrade because the installation scripts use DROP IF EXISTS + CREATE instead of CREATE OR REPLACE.

When a DBA grants EXECUTE on a sys schema routine (e.g., GRANT EXECUTE ON FUNCTION sys.quote_identifier TO appuser@host), the grant is stored in mysql.procs_priv. During upgrade, DROP IF EXISTS removes the routine and cascades to delete its grants. The subsequent CREATE recreates the routine but the grants are lost.

The fix involves:

  1. Replacing DROP FUNCTION/PROCEDURE IF EXISTS + CREATE with CREATE OR REPLACE in all 55 sys schema stored routine scripts (28 functions + 25 procedures + 2 GIS procedures in maria_add_gis_sp.sql.in)
  2. Updating the 2 template files (templates/function.sql, templates/procedure.sql) so future routines follow the same pattern
  3. CREATE OR REPLACE atomically replaces the routine body while preserving existing grants — matching the pattern already used by sys schema views

How can this PR be tested?

Run the MTR test:

build/mysql-test/mtr main.mdev_39993

Or manually:

-- Grant on a sys routine
GRANT EXECUTE ON FUNCTION sys.quote_identifier TO someuser@localhost;

-- Run upgrade
-- $ mariadb-upgrade --force

-- Check grant is preserved
SHOW GRANTS FOR someuser@localhost;

Results from my testing

  1. Before changes
MariaDB> GRANT EXECUTE ON FUNCTION sys.quote_identifier TO testuser@localhost;
MariaDB> GRANT EXECUTE ON PROCEDURE sys.table_exists TO testuser@localhost;

-- After running: mariadb-upgrade --force

MariaDB> SHOW GRANTS FOR testuser@localhost;
+----------------------------------------------+
| Grants for testuser@localhost                |
+----------------------------------------------+
| GRANT USAGE ON *.* TO `testuser`@`localhost` |
+----------------------------------------------+

Grants on sys.quote_identifier and sys.table_exists are gone after upgrade.

  1. After changes
-- After running: mariadb-upgrade --force

MariaDB> SHOW GRANTS FOR testuser@localhost;
+---------------------------------------------------------------------------------+
| Grants for testuser@localhost                                                   |
+---------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO `testuser`@`localhost`                                    |
| GRANT EXECUTE ON FUNCTION `sys`.`quote_identifier` TO `testuser`@`localhost`    |
| GRANT EXECUTE ON PROCEDURE `sys`.`table_exists` TO `testuser`@`localhost`       |
+---------------------------------------------------------------------------------+

-- Routine still works after replacement:
MariaDB> SELECT sys.quote_identifier('test');
+-----------------------------+
| sys.quote_identifier('test')|
+-----------------------------+
| `test`                      |
+-----------------------------+

Grants preserved, routines functional.

Basing the PR against the correct MariaDB version

  • This is a bug fix and the PR is based against the latest MariaDB development branch.

PR quality check

  • I have checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am fine with the reviewer making the changes themselves.

…ing upgrade

mariadb-upgrade re-installs sys schema routines using DROP IF EXISTS
followed by CREATE. This destroys any EXECUTE grants previously granted
on those routines in mysql.procs_priv, requiring DBAs to re-grant after
every upgrade.

Fix: replace DROP IF EXISTS + CREATE with CREATE OR REPLACE for all 53
sys schema stored functions and procedures. CREATE OR REPLACE atomically
replaces the routine body while preserving existing grants, matching the
pattern already used by sys schema views.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses MDEV-39993 by replacing the DROP and CREATE pattern with CREATE OR REPLACE for GIS and sys schema stored routines, which prevents existing EXECUTE grants from being dropped during a mariadb-upgrade. A new test case is also introduced to verify this behavior. The review feedback highlights that logging the full verbose output of mariadb-upgrade in the test makes it highly fragile to future schema changes or environmental differences. It is recommended to suppress this output using --disable_result_log and --enable_result_log around the upgrade executions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

SHOW GRANTS FOR testuser@localhost;

--echo # Run mariadb-upgrade (re-installs sys schema)
--exec $MYSQL_UPGRADE --force 2>&1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Logging the entire verbose output of mariadb-upgrade makes the test highly fragile and prone to failure. Any future changes to the mysql or sys schemas (such as adding, removing, or renaming tables/views), or running the test in environments with different storage engine configurations (e.g., with or without InnoDB enabled), will cause result mismatches and test failures.

It is highly recommended to wrap the $MYSQL_UPGRADE execution with --disable_result_log and --enable_result_log to suppress this output. The test's correctness is already verified by checking the grants and routine functionality before and after the upgrade.

--disable_result_log
--exec $MYSQL_UPGRADE --force 2>&1
--enable_result_log

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to agree here. it's a good suggestion.


--echo # Verify CREATE OR REPLACE works for fresh routine (simulate new install)
DROP FUNCTION IF EXISTS sys.quote_identifier;
--exec $MYSQL_UPGRADE --force 2>&1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similarly, suppress the verbose output of the second mariadb-upgrade execution to prevent test fragility due to schema or environment differences.

--disable_result_log
--exec $MYSQL_UPGRADE --force 2>&1
--enable_result_log

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too.

@akshatnehra

Copy link
Copy Markdown
Contributor Author

I disagree with the Gemini bot's suggestion here. Other mysql_upgrade tests use the same pattern of logging the full $MYSQL_UPGRADE --force 2>&1 output without --disable_result_log. Keeping it consistent with existing conventions.

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Jun 16, 2026
@gkodinov gkodinov self-assigned this Jun 16, 2026

@gkodinov gkodinov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This is a preliminary review.

Please rebase to 10.11 : this is a bug fix and it needs to go to the lowest affected version.

Also, please watch the buildbot tests so they complete without errors that look related.

SHOW GRANTS FOR testuser@localhost;

--echo # Run mariadb-upgrade (re-installs sys schema)
--exec $MYSQL_UPGRADE --force 2>&1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to agree here. it's a good suggestion.


--echo # Verify CREATE OR REPLACE works for fresh routine (simulate new install)
DROP FUNCTION IF EXISTS sys.quote_identifier;
--exec $MYSQL_UPGRADE --force 2>&1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too.

SHOW GRANTS FOR testuser@localhost;

--echo # Verify routines still work after replacement
SELECT sys.quote_identifier('test') AS quoted;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a valid test IMHO: I'd test it with the actual user that is being created for the test.

--exec $MYSQL_UPGRADE --force 2>&1

--echo # Routine recreated successfully
SELECT sys.quote_identifier('test') AS quoted;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also not what should be tested. Of course the procedure will be recreated. And this is probably tested someplace else already. I'd suggest dropping the whole test part: it doesn't contribute to more coverage IMHO.

Instead you should be testing all of the affected procedures and functions in SYS: there's quite a number of them and you're just testing the one.
I'd add grants to all of the procedures to the test user and verify if all of these survive the upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

2 participants