Skip to content

fix(config): round out default SQL denylists and apply MySQL defaults to MariaDB - #46148

Open
sha174n wants to merge 1 commit into
apache:masterfrom
sha174n:fix/default-sql-denylist-coverage
Open

sha174n wants to merge 1 commit into
apache:masterfrom
sha174n:fix/default-sql-denylist-coverage

Conversation

@sha174n

@sha174n sha174n commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Fills gaps in the default per-engine SQL function/table denylists and makes MySQL-derived engine specs pick up the MySQL defaults. These are defaults-completeness and cross-engine-consistency changes to superset/config.py; the lists remain fully operator-overridable.

  • PostgreSQL DISALLOWED_SQL_FUNCTIONS: add the sequence functions setval, nextval, currval, lastval, next to the existing SELECT-callable state-changing entries.
  • ClickHouse DISALLOWED_SQL_FUNCTIONS: add the table functions that read from locations outside the configured database — remote, remoteSecure, s3, s3Cluster, file, fileCluster, hdfs, mysql, postgresql, mongodb, jdbc, odbc — matching the external-data functions already covered for other engines. url was already listed.
  • MariaDB coverage: the denylists are keyed by db_engine_spec.engine. MariaDBEngineSpec derives from MySQLEngineSpec but reports engine="mariadb", so it did not pick up the "mysql" entries. The MySQL function and table defaults are now mirrored onto "mariadb" — copied, not aliased, so overriding one engine's list in a deployment's config does not mutate the other's.

TESTING INSTRUCTIONS

Added unit tests:

  • tests/unit_tests/config_test.py — the new PostgreSQL/ClickHouse defaults are present, and "mariadb" mirrors "mysql" for both dicts (equal but distinct objects).
  • tests/unit_tests/db_engine_specs/test_mariadb.py — the db_engine_spec.engine lookup resolves the MySQL defaults for MariaDB.
pytest tests/unit_tests/config_test.py tests/unit_tests/db_engine_specs/test_mariadb.py
# 88 passed

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
  • Introduces new feature or API
  • Removes existing feature or API

@codeant-ai-for-open-source

codeant-ai-for-open-source Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

CodeAnt  CodeAnt PR Risk

🟢  Low Risk

  • The PR appears safe to merge; no actionable issues were identified in the review.

Assessed commit: e5f108897f80

@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.24%. Comparing base (a6f30aa) to head (e5f1088).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #46148      +/-   ##
==========================================
- Coverage   65.35%   65.24%   -0.11%     
==========================================
  Files        3083     3083              
  Lines      211333   211335       +2     
  Branches    49823    49823              
==========================================
- Hits       138110   137884     -226     
- Misses      70041    70259     +218     
- Partials     3182     3192      +10     
Flag Coverage Δ
hive 34.05% <100.00%> (+<0.01%) ⬆️
mysql ?
postgres 51.84% <100.00%> (+<0.01%) ⬆️
presto 35.69% <100.00%> (+<0.01%) ⬆️
python 51.95% <100.00%> (-0.18%) ⬇️
sqlite ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… to MariaDB

Fill gaps in the default per-engine SQL function/table denylists and make
MySQL-derived engine specs pick up the MySQL defaults.

- PostgreSQL: add the sequence functions (setval, nextval, currval, lastval)
  to DISALLOWED_SQL_FUNCTIONS, next to the other SELECT-callable
  state-changing entries.
- ClickHouse: add the table functions that read from outside the configured
  database (remote, remoteSecure, s3, s3Cluster, file, fileCluster, hdfs,
  mysql, postgresql, mongodb, jdbc, odbc), matching the external-data
  functions already covered for other engines. url was already listed.
- The denylists are keyed by db_engine_spec.engine. MariaDBEngineSpec derives
  from MySQLEngineSpec but reports engine="mariadb", so it did not pick up the
  "mysql" entries. Mirror the MySQL function and table defaults onto "mariadb"
  (copied, not aliased, so overriding one engine does not mutate the other).
- Add unit tests for the new defaults and the MariaDB lookup resolution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sha174n
sha174n force-pushed the fix/default-sql-denylist-coverage branch from d54cbb8 to e5f1088 Compare November 11, 2026 11:39

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant