Summary
MySQLDestination already has a _quote_ident helper that correctly handles schema-qualified table names (db.table → `db`.`table`), but it's only used in the swap path. The non-swap replace / insert / upsert paths still use raw f-string interpolation, so schema-qualified table names break in those modes.
Background
#442 / PR #498 closed the equivalent gap on the Postgres side (psycopg2.sql.Identifier with (schema, relation) parts). The MySQL audit was listed in #442's "Proposed Approach" but explicitly out of scope — this issue captures it as a follow-up.
The good news: unlike Postgres, the MySQL fix needs no new helpers. _quote_ident already exists in drt/destinations/mysql.py and does the right thing:
@staticmethod
def _quote_ident(table: str) -> str:
if "." in table:
return "`" + "`.`".join(table.split(".")) + "`"
return f"`{table}`"
The work is purely mechanical: replace the remaining raw-backtick f-strings with _quote_ident().
Call sites that still use raw f-strings
| Method |
Line |
Current |
Needs |
_load_replace |
L190 |
f"TRUNCATE TABLE \{table}`"` |
f"TRUNCATE TABLE {self._quote_ident(table)}" |
_build_insert_sql |
L353 |
f"INSERT INTO \{table}` ..."` |
_quote_ident(table) |
_build_upsert_sql |
L368 |
f"INSERT INTO \{table}` ..."` |
_quote_ident(table) |
_build_upsert_sql |
L372 |
f"INSERT IGNORE INTO \{table}` ..."` |
_quote_ident(table) |
get_row_count |
L144-149 |
inline split+backtick logic |
reuse _quote_ident |
(_load_replace_swap and finalize_sync already use _quote_ident — leave them.)
Reference PR
PR #498 for the equivalent Postgres work is a near-perfect template — see especially the test patterns in tests/unit/test_postgres_destination.py::TestQualifiedIdentifiers and the test_*_uses_schema_qualified_identifier cases. The MySQL test file (tests/unit/test_mysql_destination.py) follows the same mock_connect pattern, so the diff should look very similar.
Acceptance Criteria
Out of Scope
- Adding support for arbitrary quoted identifiers (embedded
., `, etc.) — not worth supporting until requested.
- ClickHouse audit — tracked separately.
Good First Issue
This is well-scoped for first-time contributors:
Summary
MySQLDestinationalready has a_quote_identhelper that correctly handles schema-qualified table names (db.table→`db`.`table`), but it's only used in the swap path. The non-swap replace / insert / upsert paths still use raw f-string interpolation, so schema-qualified table names break in those modes.Background
#442 / PR #498 closed the equivalent gap on the Postgres side (
psycopg2.sql.Identifierwith(schema, relation)parts). The MySQL audit was listed in #442's "Proposed Approach" but explicitly out of scope — this issue captures it as a follow-up.The good news: unlike Postgres, the MySQL fix needs no new helpers.
_quote_identalready exists indrt/destinations/mysql.pyand does the right thing:The work is purely mechanical: replace the remaining raw-backtick f-strings with
_quote_ident().Call sites that still use raw f-strings
_load_replacef"TRUNCATE TABLE \{table}`"`f"TRUNCATE TABLE {self._quote_ident(table)}"_build_insert_sqlf"INSERT INTO \{table}` ..."`_quote_ident(table)_build_upsert_sqlf"INSERT INTO \{table}` ..."`_quote_ident(table)_build_upsert_sqlf"INSERT IGNORE INTO \{table}` ..."`_quote_ident(table)get_row_count_quote_ident(
_load_replace_swapandfinalize_syncalready use_quote_ident— leave them.)Reference PR
PR #498 for the equivalent Postgres work is a near-perfect template — see especially the test patterns in
tests/unit/test_postgres_destination.py::TestQualifiedIdentifiersand thetest_*_uses_schema_qualified_identifiercases. The MySQL test file (tests/unit/test_mysql_destination.py) follows the samemock_connectpattern, so the diff should look very similar.Acceptance Criteria
_load_replace/_build_insert_sql/_build_upsert_sqlcall sites use_quote_identget_row_countreuses_quote_identinstead of duplicating the split logicmydb.scores) for replace, upsert, andget_row_countpaths — mirroring the cases added in PR fix: quote qualified Postgres table identifiers #498type: ignore;ruff checkandmypy drtpassOut of Scope
.,`, etc.) — not worth supporting until requested.Good First Issue
This is well-scoped for first-time contributors: