Skip to content

MDEV-31535: Add privilege-based fast path for SHOW DATABASES listing - #5457

Open
itzanway wants to merge 1 commit into
MariaDB:mainfrom
itzanway:MDEV-31535
Open

MDEV-31535: Add privilege-based fast path for SHOW DATABASES listing#5457
itzanway wants to merge 1 commit into
MariaDB:mainfrom
itzanway:MDEV-31535

Conversation

@itzanway

@itzanway itzanway commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

For users without global database-listing privileges, avoid a full data-directory
scan when building the database list for SHOW DATABASES and
INFORMATION_SCHEMA.SCHEMATA.

Instead of always calling find_files() in make_db_list(), restricted users
can now use a new helper get_acl_databases_for_user() that builds the list
from in-memory privilege tables (acl_dbs, column_priv_hash).

Problem

make_db_list() currently scans the entire data directory via find_files(),
even for users who only have privileges on a small, known set of databases.
This is unnecessary work or restricted users and does not scale well on
installations with many databases.

Solution

  1. Add get_acl_databases_for_user() in sql/sql_acl.cc.

    • Collect exact database names from mysql.db and table/column-level grants.
    • Match grantees the same way as the existing visibility checks:
      • db-level: priv_user, active role, and PUBLIC (mirrors acl_get_all3())
      • table-level: priv_user and active role (mirrors check_grant_db())
    • Use sctx->host / sctx->ip for host matching.

  2. Gate the fast path in make_db_list() (sql/sql_show.cc) when:

    • the caller is fill_schema_schemata() (SHOW DATABASES / SCHEMATA only)
    • the user lacks global DB-listing privileges
    • all applicable grants use exact database names (no % or _ wildcards)

  3. If any applicable grant contains a wildcard, decline the fast path and fall
    back to the existing find_files() scan.

  4. Preserve existing output ordering by applying the same sort used by
    find_files() (Discovered_table_list::sort() / sort_desc() in debug
    builds).

  5. On fast-path decline, restore the caller's database list to its entry size
    so partial results are not duplicated by the fallback scan.

@gemini-code-assist

gemini-code-assist Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

janlindstrom
janlindstrom requested changes Jul 27, 2026
View reviewed changes

@janlindstrom janlindstrom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks very promising but requires some cleanup and additional work so that
user can verify is fast path used or not.

Comment thread sql/sql_acl.cc

@janlindstrom janlindstrom Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please no white space changes.

Comment thread sql/sql_acl.cc Outdated

@janlindstrom janlindstrom Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please avoid lambda-functions and use inline functions instead.

Comment thread sql/sql_acl.cc Outdated

@janlindstrom janlindstrom Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar conditions in two places, consider using inline function

Comment thread mysql-test/main/mdev_31535.test
Comment thread sql/sql_acl.cc Outdated

@janlindstrom janlindstrom Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use strncmp instead

@janlindstrom janlindstrom self-assigned this Jul 27, 2026
@itzanway
itzanway force-pushed the MDEV-31535 branch 2 times, most recently from 0bd4a8a to cbb2b90 Compare August 1, 2026 15:36
janlindstrom
janlindstrom requested changes Aug 3, 2026
View reviewed changes

@janlindstrom janlindstrom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks very good. Please add some code comments on significant parts.

Comment thread sql/sql_show.cc Outdated
{
if (plan)
plan->db_list_method= SCHEMA_DB_LIST_ACL;
Discovered_table_list tl(thd, files, &null_clex_str);

@janlindstrom janlindstrom Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would move this before first if on this block

Comment thread sql/sql_show.cc
tl.sort();
#ifndef DBUG_OFF
else
tl.sort_desc();

@janlindstrom janlindstrom Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add comment before this line why this is done.

@janlindstrom

janlindstrom commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

/gemini review

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Aug 3, 2026
gkodinov
gkodinov requested changes Aug 5, 2026
View reviewed changes

@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.

I see that the final review is already in progress. That's great!

I'll just point out the obvious formal problems then:

  • please make sure the buildbot tests are passing. Currently your newly added test is failing.
  • please keep 1 commit per "feature" per "participant". In your case I believe it should be a single commit.

janlindstrom
janlindstrom reviewed Aug 5, 2026
View reviewed changes
Comment thread mysql-test/main/mdev_31535.test Outdated
--sorted_result
show databases;
--sorted_result
select schema_name from information_schema.schemata;

@janlindstrom janlindstrom Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use also ORDER BY

@itzanway itzanway Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added ORDER BY schema_name on SCHEMATA queries (fixes debug-build ordering)
Updated EXPLAIN expectations for filesort with ORDER BY

@itzanway
MDEV-31535: Add privilege-based fast path for SHOW DATABASES / SCHEMATA
f3a8106 
Use get_acl_databases_for_user() for restricted users on SCHEMATA queries
instead of scanning the datadir with find_files(). Fall back when grants
contain wildcards. Skip the fast path when NO_EMBEDDED_ACCESS_CHECKS is
defined (embedded server without privilege control). Add MTR coverage with
ORDER BY for deterministic output.
@itzanway

itzanway commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

@janlindstrom @sanja-byelkin @gkodinov

The remaining amd64-debian-12-debug-embedded failure is in the debug-view MTR suite, not in compilation.

After guarding the ACL fast path with #ifndef NO_EMBEDDED_ACCESS_CHECKS, the embedded link step succeeds .

On debian-12 embedded, MTR reports 3/1240 failures (99.76% pass):

These tests are unrelated to MDEV-31535. The change is covered by main.mdev_31535, which passes locally together with main.information_schema, and the required builders are green.

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

Reviewers

@janlindstrom janlindstrom janlindstrom requested changes
@gkodinov gkodinov gkodinov requested changes
@sanja-byelkin sanja-byelkin Awaiting requested review from sanja-byelkin

Assignees

@janlindstrom janlindstrom

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@itzanway @janlindstrom @gkodinov