Skip to content

hygiene: unify BigQuery executor onto the DB-API shim + document why it diverges #99

Description

@sandeep-agami

Context

execute_sql.py runs generated SQL against ~10 engine families. Nine of them (Postgres/Redshift, MySQL,
Snowflake, SQL Server, Oracle, Databricks, Trino, DuckDB, SQLite) are DB-API 2.0 (PEP 249) drivers, so they all
funnel through one shared row-materialization sink, _write_cursor_csv (cur.description + cur.fetchall()).

BigQuery is the exception. _execute_bigquery (packages/agami-core/src/execute_sql.py:453, and the mirror
in plugins/agami/lib/execute_sql.py:454) uses BigQuery's native job model — client.query(sql)
job.result()RowIterator — plus IAM auth (service-account key / ADC + a private-key chmod check +
default_dataset), none of which is DB-API-shaped. So it has its own emit loop and does not share
_write_cursor_csv.

Why this matters: every executor-level guardrail has to be applied twice, and BigQuery can silently drift
from the shared safety path. Concretely, the incoming ACE-038 row cap (safe-sql-execution / F9) lands as
fetchmany(cap + 1) in _write_cursor_csv for the nine shared engines, but needs a separate
job.result(max_results=cap + 1) in the BigQuery path.

Tasks

  • Document the divergence. Add a short comment to _execute_bigquery in both copies
    (packages/agami-core/src/execute_sql.py and plugins/agami/lib/execute_sql.py) noting it diverges because
    BigQuery's client isn't DB-API 2.0-shaped (job / RowIterator model + IAM auth), unlike the other engines that
    share _write_cursor_csv.
  • Unify onto the DB-API shim (cleanup). Migrate BigQuery onto google.cloud.bigquery.dbapi: keep the
    existing bigquery.Client(...) construction (SA key / ADC + chmod check + default_dataset) and pass it to
    dbapi.connect(client=client). Per Google's docs, a client explicitly passed to the Connection
    constructor is not closed by the connection, so the auth path stays intact. BigQuery then flows through
    _write_cursor_csv and inherits executor-level guardrails (the ACE-038 row cap, and any future gate) in one
    place — collapsing the special case.

Notes / references

  • Keep both execute_sql.py copies in sync (the recurring "both surfaces" concern).
  • Hygiene, not a safety blocker — ACE-038 handles the BigQuery cap in its own path regardless; this issue is
    about removing the per-engine special case so future guardrails land once.
  • Refs: PEP 249 (DB-API 2.0) ·
    google.cloud.bigquery.dbapi ·
    QueryJob.result(max_results=…)
  • Related: F9 safe-sql-execution — ACE-035 (shared Verdict/Envelope), ACE-038 (resource limits),
    ACE-040 (both-surfaces safety corpus).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions