fix: %-escape identifiers only on the params path (no more %% wrong-target on DDL)

_quote_ident() always doubles % to %%, but aiomysql only runs query % args
substitution when args is not None - _run/_fetchall/_fetchone were passing
params or None, so no-params DDL (create_database/create_table/drop) and
no-condition get/delete/exists shipped literal %% to the server, silently
targeting the wrong object and making IF EXISTS no-op (regression 2e837da).

_run/_fetchall/_fetchone now forward None as-is (preserving layer-2's
documented "no params, no substitution" contract) but always pass a real
tuple otherwise, so layer-1's %%-escaped identifiers always collapse back
to a single % as intended. The three no-params DDL verbs now pass an
explicit empty tuple so substitution runs for them too.

connect()/close() had the same lock asymmetry as psql (close() wasn't
guarded by _connect_lock, so a close() racing an in-flight connect() could
no-op while the new pool went live) - fixed in lockstep with the psql twin
fix, and create_pool() is now inside the same try/log/re-raise as the
SELECT-1 validation.

Signed-off-by: disqualifier <dev@disqualifier.me>
This commit is contained in:
2026-07-03 19:11:39 -04:00
parent ebf1f80652
commit 0e01c4e77f
3 changed files with 47 additions and 24 deletions
+3 -3
View File
@@ -11,18 +11,18 @@ wire-compatible and share the driver, so this covers both.
`requirements.txt`: `requirements.txt`:
``` ```
mysql @ git+ssh://git@git.rethinkstudios.io/rethink-public/mysql.git@v0.1.5 mysql @ git+ssh://git@git.rethinkstudios.io/rethink-public/mysql.git@v0.1.6
``` ```
Direct: Direct:
```bash ```bash
pip install "mysql @ git+ssh://git@git.rethinkstudios.io/rethink-public/mysql.git@v0.1.5" pip install "mysql @ git+ssh://git@git.rethinkstudios.io/rethink-public/mysql.git@v0.1.6"
``` ```
Pulls `aiomysql` (which pulls `PyMySQL`). Pulls `aiomysql` (which pulls `PyMySQL`).
Drop the `@v0.1.5` suffix from the line above to install the latest unpinned. Drop the `@v0.1.6` suffix from the line above to install the latest unpinned.
## The two-layer API ## The two-layer API
+1 -1
View File
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
[project] [project]
name = "mysql" name = "mysql"
version = "0.1.5" version = "0.1.6"
description = "async mysql/mariadb wrapper over aiomysql: two-layer API (friendly verbs + raw escape hatch), fail-loud, config-free" description = "async mysql/mariadb wrapper over aiomysql: two-layer API (friendly verbs + raw escape hatch), fail-loud, config-free"
requires-python = ">=3.10" requires-python = ">=3.10"
dependencies = [ dependencies = [
+41 -18
View File
@@ -104,15 +104,22 @@ class MysqlDB:
""" """
async with self._connect_lock: async with self._connect_lock:
if self._pool is not None: if self._pool is not None:
await self.close() await self._close_locked()
pool = await aiomysql.create_pool(**self._config) pool = None
try: try:
pool = await aiomysql.create_pool(**self._config)
async with pool.acquire() as conn: async with pool.acquire() as conn:
async with conn.cursor() as cur: async with conn.cursor() as cur:
await cur.execute("SELECT 1") await cur.execute("SELECT 1")
await cur.fetchone() await cur.fetchone()
except _DRIVER_ERRORS:
log.exception("mysql.connect() failed")
if pool is not None:
pool.close()
await pool.wait_closed()
raise
except BaseException: except BaseException:
log.exception("mysql.connect() validation failed") if pool is not None:
pool.close() pool.close()
await pool.wait_closed() await pool.wait_closed()
raise raise
@@ -120,7 +127,17 @@ class MysqlDB:
return self return self
async def close(self) -> None: async def close(self) -> None:
"""close the pool and wait for it on shutdown""" """close the pool and wait for it on shutdown
guarded by the same lock as connect() - a close() racing an in-flight connect()
waits for it rather than no-opping against a not-yet-installed pool and leaving
the just-built one live.
"""
async with self._connect_lock:
await self._close_locked()
async def _close_locked(self) -> None:
"""close the pool and wait for it; caller must hold `_connect_lock`"""
if self._pool is None: if self._pool is None:
return return
try: try:
@@ -151,26 +168,32 @@ class MysqlDB:
# ------------------------------------------------------------------------- # -------------------------------------------------------------------------
# internal cursor helpers (autocommit pool) # internal cursor helpers (autocommit pool)
async def _run(self, query: str, params: Sequence = ()): async def _run(self, query: str, params: Optional[Sequence] = None):
"""execute a statement on a pooled cursor; return (rowcount, lastrowid)""" """execute a statement on a pooled cursor; return (rowcount, lastrowid)
`params=None` is forwarded as-is (aiomysql skips `query % args` entirely - the
layer-2 "no params, no substitution" contract); any other value, including an
empty sequence, is passed as a real tuple so `query % args` always runs and
collapses layer-1's `%%`-escaped identifiers back to `%` (see module note).
"""
async with self.pool.acquire() as conn: async with self.pool.acquire() as conn:
async with conn.cursor() as cur: async with conn.cursor() as cur:
await cur.execute(query, params or None) await cur.execute(query, params if params is None else tuple(params))
return cur.rowcount, cur.lastrowid return cur.rowcount, cur.lastrowid
async def _fetchall(self, query: str, params: Sequence = ()) -> List[dict]: async def _fetchall(self, query: str, params: Optional[Sequence] = None) -> List[dict]:
"""execute a query and return all rows as dicts""" """execute a query and return all rows as dicts"""
async with self.pool.acquire() as conn: async with self.pool.acquire() as conn:
async with conn.cursor() as cur: async with conn.cursor() as cur:
await cur.execute(query, params or None) await cur.execute(query, params if params is None else tuple(params))
rows = await cur.fetchall() rows = await cur.fetchall()
return list(rows) return list(rows)
async def _fetchone(self, query: str, params: Sequence = ()) -> Optional[dict]: async def _fetchone(self, query: str, params: Optional[Sequence] = None) -> Optional[dict]:
"""execute a query and return the first row dict, or None""" """execute a query and return the first row dict, or None"""
async with self.pool.acquire() as conn: async with self.pool.acquire() as conn:
async with conn.cursor() as cur: async with conn.cursor() as cur:
await cur.execute(query, params or None) await cur.execute(query, params if params is None else tuple(params))
return await cur.fetchone() return await cur.fetchone()
# ------------------------------------------------------------------------- # -------------------------------------------------------------------------
@@ -183,7 +206,7 @@ class MysqlDB:
lastrowid, or reach for the raw pool. lastrowid, or reach for the raw pool.
""" """
try: try:
rowcount, _ = await self._run(query, params or ()) rowcount, _ = await self._run(query, params)
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.execute(): %s", query) log.exception("mysql.execute(): %s", query)
raise raise
@@ -192,7 +215,7 @@ class MysqlDB:
async def fetch(self, query: str, params: Optional[Sequence] = None) -> List[dict]: async def fetch(self, query: str, params: Optional[Sequence] = None) -> List[dict]:
"""run a query and return all rows as plain dicts (empty list = no rows)""" """run a query and return all rows as plain dicts (empty list = no rows)"""
try: try:
return await self._fetchall(query, params or ()) return await self._fetchall(query, params)
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.fetch(): %s", query) log.exception("mysql.fetch(): %s", query)
raise raise
@@ -200,7 +223,7 @@ class MysqlDB:
async def fetchone(self, query: str, params: Optional[Sequence] = None) -> Optional[dict]: async def fetchone(self, query: str, params: Optional[Sequence] = None) -> Optional[dict]:
"""run a query and return the first row as a dict, or None if no rows""" """run a query and return the first row as a dict, or None if no rows"""
try: try:
return await self._fetchone(query, params or ()) return await self._fetchone(query, params)
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.fetchone(): %s", query) log.exception("mysql.fetchone(): %s", query)
raise raise
@@ -208,7 +231,7 @@ class MysqlDB:
async def fetchval(self, query: str, params: Optional[Sequence] = None) -> Any: async def fetchval(self, query: str, params: Optional[Sequence] = None) -> Any:
"""run a query and return the first column of the first row, or None""" """run a query and return the first column of the first row, or None"""
try: try:
row = await self._fetchone(query, params or ()) row = await self._fetchone(query, params)
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.fetchval(): %s", query) log.exception("mysql.fetchval(): %s", query)
raise raise
@@ -230,7 +253,7 @@ class MysqlDB:
async def create_database(self, name: str) -> None: async def create_database(self, name: str) -> None:
"""CREATE DATABASE IF NOT EXISTS name""" """CREATE DATABASE IF NOT EXISTS name"""
try: try:
await self._run(f"CREATE DATABASE IF NOT EXISTS {_quote_ident(name)}") await self._run(f"CREATE DATABASE IF NOT EXISTS {_quote_ident(name)}", ())
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.create_database(%s)", name) log.exception("mysql.create_database(%s)", name)
raise raise
@@ -245,7 +268,7 @@ class MysqlDB:
""" """
cols = ", ".join(f"{_quote_ident(col)} {decl}" for col, decl in schema.items()) cols = ", ".join(f"{_quote_ident(col)} {decl}" for col, decl in schema.items())
try: try:
await self._run(f"CREATE TABLE IF NOT EXISTS {_quote_ident(name)} ({cols})") await self._run(f"CREATE TABLE IF NOT EXISTS {_quote_ident(name)} ({cols})", ())
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.create_table(%s)", name) log.exception("mysql.create_table(%s)", name)
raise raise
@@ -254,7 +277,7 @@ class MysqlDB:
"""DROP a table (table=True, default) or database (table=False), IF EXISTS""" """DROP a table (table=True, default) or database (table=False), IF EXISTS"""
kind = "TABLE" if table else "DATABASE" kind = "TABLE" if table else "DATABASE"
try: try:
await self._run(f"DROP {kind} IF EXISTS {_quote_ident(name)}") await self._run(f"DROP {kind} IF EXISTS {_quote_ident(name)}", ())
except _DRIVER_ERRORS: except _DRIVER_ERRORS:
log.exception("mysql.drop(%s)", name) log.exception("mysql.drop(%s)", name)
raise raise