fix(db): run SQL on a parameterless DBAPI cursor — % broke every Postgres query

`execute_readonly` went through `exec_driver_sql`, which hands the driver an
(empty) parameter set. psycopg speaks pyformat, so it then scanned the statement
for placeholders and rejected `LIKE '%variance%'`:

    only '%s', '%b', '%t' are allowed as placeholders, got '%v'

That is the single most common shape a text-to-SQL model emits, plus the `%`
modulo operator — and it hit BIRD's own gold SQL too, so affected questions
could not even be scored (qids 586/587 on the codebase_community eval). The
Postgres backend has been advertised since day one; nothing had ever actually
run a LIKE against it.

A DBAPI cursor executed with no parameters interprets nothing, which is the only
path that survives both traps at once: psycopg's `%` and SQLAlchemy `text()`'s
`:__` (the latter is why the code left `text()` in the first place — BIRD qids
959/989/990). Driver-native exceptions are re-wrapped into `sqlalchemy.exc` so
that callers' error classification (execution/runner.py) keeps working.

Live-Postgres tests pin LIKE (prefix/suffix/contains), the modulo operator, and
colon literals. SQLite path is unchanged: re-measured n=200 → still 57.5%.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: JuliaEdom

src/nl_sql/db/connection.py

@@ -20,6 +20,7 @@
 
 from sqlalchemy import Connection, Engine, create_engine, text
 from sqlalchemy.engine import make_url
+from sqlalchemy.exc import DBAPIError
 
 Dialect = Literal["sqlite", "postgresql"]
 
@@ -126,13 +127,36 @@ def execute_readonly(
     started = time.perf_counter()
     with engine.connect() as conn:
         _apply_runtime_limits(conn, statement_timeout_ms)
-        # exec_driver_sql bypasses SQLAlchemy's :name bind-parameter parsing,
-        # which would otherwise misinterpret colons inside string literals such
-        # as `LIKE '_:%:__.___'` (BIRD qids 959 / 989 / 990 — formula_1 time patterns).
-        cursor = conn.exec_driver_sql(sql)
-        columns = list(cursor.keys())
-        rows = cursor.fetchmany(row_cap + 1)
-        cursor.close()  # drain any remaining rows so SQLite/Postgres release resources
+        # Run on a DBAPI cursor with NO parameters, because every parameterised
+        # path mangles a literal that real SQL contains:
+        #   - SQLAlchemy `text()` reads `:__` as a bind parameter, breaking gold
+        #     like `LIKE '_:%:__.___'` (BIRD qids 959/989/990, formula_1 times).
+        #   - `exec_driver_sql` passes an (empty) parameter set to the driver, so
+        #     psycopg's pyformat parser scans for placeholders and rejects `%`:
+        #     `LIKE '%variance%'` → "only '%s', '%b', '%t' are allowed as
+        #     placeholders". That killed LIKE filters *and* the `%` modulo
+        #     operator on Postgres — see tests/test_postgres_readonly.py.
+        # A cursor executed with no parameters interprets neither, on either driver.
+        cursor = conn.connection.cursor()
+        try:
+            cursor.execute(sql)
+            columns = [str(d[0]) for d in cursor.description or ()]
+            rows = cursor.fetchmany(row_cap + 1)
+        except Exception as raw_exc:
+            # Going through the raw cursor also means driver-native exceptions
+            # (sqlite3.OperationalError, psycopg.Error) instead of SQLAlchemy's.
+            # Callers classify failures on sqlalchemy.exc types — timeouts on
+            # OperationalError, everything else on SQLAlchemyError (see
+            # execution/runner.py) — so re-wrap and keep that contract intact.
+            raise DBAPIError.instance(
+                sql,
+                None,
+                raw_exc,
+                engine.dialect.loaded_dbapi.Error,
+                dialect=engine.dialect,
+            ) from raw_exc
+        finally:
+            cursor.close()  # release server-side resources / drain remaining rows
     truncated = len(rows) > row_cap
     if truncated:
         rows = rows[:row_cap]

tests/test_postgres_readonly.py

@@ -108,3 +108,44 @@ def test_execute_readonly_runs_a_real_select(readonly_engine: Engine) -> None:
         assert result.columns == ["id", "v"]
         assert result.rows == [(1, "seed")]
         assert result.truncated is False
+
+
+@pytest.mark.parametrize(
+    ("sql", "expected"),
+    [
+        pytest.param(
+            f"SELECT v FROM {_TABLE} WHERE v LIKE '%see%'", [("seed",)], id="like-contains"
+        ),
+        pytest.param(f"SELECT v FROM {_TABLE} WHERE v LIKE 'see%'", [("seed",)], id="like-prefix"),
+        pytest.param(f"SELECT v FROM {_TABLE} WHERE v LIKE '%eed'", [("seed",)], id="like-suffix"),
+        pytest.param(f"SELECT 7 % 2 FROM {_TABLE}", [(1,)], id="modulo-operator"),
+        pytest.param(f"SELECT ':__' FROM {_TABLE}", [(":__",)], id="colon-literal"),
+        pytest.param(
+            f"SELECT v FROM {_TABLE} WHERE 'a:1' LIKE '%:%'", [("seed",)], id="colon-and-percent"
+        ),
+    ],
+)
+def test_execute_readonly_passes_literals_through_verbatim(
+    readonly_engine: Engine, sql: str, expected: list[tuple[object, ...]]
+) -> None:
+    """Neither `%` nor `:` inside the SQL may be read as a bind placeholder.
+
+    Two drivers, two opposite traps, and the statement has to survive both:
+
+    - psycopg speaks `pyformat`, so anything routed through a *parameterised*
+      execute makes the driver scan for `%s`-style placeholders. `LIKE '%see%'`
+      then dies with "only '%s', '%b', '%t' are allowed as placeholders, got
+      '%s'e" — and a LIKE filter is the single most common shape a text-to-SQL
+      model emits. This is not hypothetical: BIRD qids 586/587 (`LIKE
+      '%variance%'`, `LIKE '%<humor>%'`) failed on the Postgres eval, and the
+      failure hit BIRD's own *gold* SQL too, so the question could not even be
+      scored.
+    - SQLAlchemy's `text()` reads `:__` as a bind parameter, which breaks BIRD
+      gold like `LIKE '_:%:__.___'` (qids 959/989/990) — see the SQLite twin of
+      this test in tests/test_db_connection.py.
+
+    The only path that interprets neither is a DBAPI cursor executed with no
+    parameters at all, which is what `execute_readonly` must do.
+    """
+    with execute_readonly(readonly_engine, sql) as result:
+        assert result.rows == expected