Skip the result cache for qmark queries with parameters - #959
Conversation
With the qmark paramstyle the query text keeps its placeholders and the values go to Athena as ExecutionParameters, so the cache search matched an earlier execution that ran with different values. Athena does not return the ExecutionParameters of earlier executions, so the cache cannot compare them; a query with parameters now always starts a new execution. Closes #941 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| query_id = None | ||
| # Athena does not return the ExecutionParameters of earlier executions, | ||
| # so the cache cannot tell which parameters an execution ran with (#941). | ||
| if not request.get("ExecutionParameters"): |
There was a problem hiding this comment.
Self-review round 1 (implementation behavior) — FINDINGS (1, repaired)
Scope: base e0e85da, reviewed head 9126987; all 6 changed files.
Covered:
- Callers: every cursor (
cursor.py,async_cursor.py, pandas/arrow/polars/s3fs sync + async, aio cursor/pandas/arrow/polars/s3fs) reaches the cache only throughBaseCursor._execute()/AioBaseCursor._execute(), so the guard here and atpyathena/aio/common.py:92covers all of them. Spark cursors do not use the cache. - Request shape:
_build_start_query_execution_request()setsExecutionParametersonly when the list is non-empty, soqmarkwithNone/[]and allpyformatqueries keep the existing lookup; the globalpyathena.paramstyle = "qmark"takes the same_prepare_query()path. - Behavior: return value, cursor state,
on_start_query_execution, and interrupt handling are unchanged; a skipped lookup takes the same_start_execution()path as a cache miss._find_previous_query_id()signatures are unchanged (dbt-athena 1.x does not call them; the legacy_execute()kwargs are untouched and the passthrough tests still assert the lookup call). - Tests:
test_execute_qmark_parameters_skip_cache(sync/aio) fails with thepyathena/change reverted;test_cache_size_with_qmark_parametersruns the issue's reproduction against Athena.
Finding: the added docs/usage.md:289 sentence explained why ("because Athena does not return ..."); user docs state behavior only, the rationale stays in the PR. Repaired in 9f3557c.
Deferred (pre-existing, not introduced): a qmark query text with ? placeholders executed later without parameters could still match an earlier parameterized execution of the same text and return its result instead of Athena's missing-parameter error. Detecting this would need SQL parsing; such a call is invalid input either way.
| ) | ||
| query_id = None | ||
| # Athena does not return the ExecutionParameters of earlier executions, | ||
| # so the cache cannot tell which parameters an execution ran with (#941). |
There was a problem hiding this comment.
Self-review round 2 (claims, compatibility, operations) — FINDINGS (PR description only, corrected)
Scope: base e0e85da, head 9f3557c; claims in the PR body, commit messages, this comment, the _execute() / ExecuteOptions.cache_size docstrings, docs/usage.md:289, and the issue premise.
Claims checked:
- Issue premise "
AthenaQueryExecution.execution_parametersexposes them" — false in practice. Measured 2026-10-03 in work groupspyathenaandprimary: neitherGetQueryExecutionnorBatchGetQueryExecution(the API the cache search uses) returnsExecutionParametersfor a query started with them, although botocore 1.43.102'sQueryExecutionshape declares the field. This comment's claim holds for both measured work groups. - Round 1's deferral: measured,
SELECT ? AS vwithparamstyle="qmark"and no parameters fails withINVALID_PARAMETER_USAGE: Incorrect number of parameters: expected 1 but found 0, so the remaining pre-existing match only affects invalid calls. - "Queries without parameters, including
pyformat, keep the existing behavior":_build_start_query_execution_request()addsExecutionParametersonly for a non-empty list;pyformatpassesNone. - Docs reader: the parameterized-query section (
docs/usage.md:108-143) says nothing about caching; the cache section now states the rule; no other doc mentionsqmarkwith the cache. - Existing caller: no signature changes. Repeated
qmarkqueries with identical parameters used to get correct cache hits and now run again (more Athena queries for those callers). This was missing from the release note. - AWS operator: the change only removes
ListQueryExecutions/BatchGetQueryExecutioncalls; no retry layer changes. - Evidence: the local AWS run (21 passed) and offline revert check belong to 9126987; 9f3557c changes one docs sentence only.
Corrections to the PR description: named the second measured work group, added the identical-parameters behavior change to the release note, and stated which commit the test results belong to.
|
|
||
| @pytest.mark.parametrize("cursor", [{"work_group": ENV.work_group}], indirect=["cursor"]) | ||
| def test_cache_size_with_qmark_parameters(self, cursor): | ||
| query = f"SELECT ? AS v -- {datetime.now(UTC)!s}" |
There was a problem hiding this comment.
Independent review (relayed) — FINDINGS (2; 1 rejected with evidence, 1 pre-existing and deferred)
Reviewer: Codex CLI 0.160.0, model gpt-6-astra (OpenAI), codex exec --sandbox read-only, session 01a10064-26ca-7751-a71d-935256265a41. Static review only: no builds, tests, edits, or network access. The prompt gave the base/head SHAs and the repository conventions, without the PR number, description, commit messages, or self-review records.
Snapshot: detached worktree at head 9f3557c, base e0e85da; afterwards it was still clean at the same HEAD, and the PR worktree was unchanged.
Covered (reviewer): the full diff; all sync, threaded-async, and aio SQL cursor paths (Dict, pandas, Arrow, Polars, S3FS); UNLOAD; legacy private-method signatures; cache matching; changed docs; relevant tests. The reviewer confirmed that the guard reaches every applicable cursor, private signatures are preserved, both mocked tests fail against the original code, and botocore's service documentation says the parameters are not returned.
- Introduced, P2 (this line): "
SELECT ? AS vputs a parameter in the SELECT list, which Athena supports only in WHERE clauses, so the first execution fails." — Rejected. This test passed against Athena in the local run (-k "cache or qmark or legacy_kwargs": 21 passed) and returned[('2',)]/[('1',)]; the issue's live reproduction used the sameSELECT ? AS vform. - Pre-existing, P2 (
pyathena/common.py:1125): the same?SQL executed withparameters=None/[]andcache_sizecan reuse an earlier parameterized execution instead of letting Athena reject the missing binding. — Deferred, pre-existing (also present at the base SHA, as noted in round 1). Measured: Athena rejects such a call withINVALID_PARAMETER_USAGE: Incorrect number of parameters: expected 1 but found 0, so this only affects invalid calls. Skipping on unresolved?markers would need a heuristic over the SQL text, which is outside this fix's scope.
No code change resulted, so no follow-up review is needed. The PR description now also quotes botocore's field documentation.
WHAT
Client-side result caching (
cache_size/cache_expiration_time) no longer searches for a previous execution when aqmarkquery has parameters; such a query always starts a new execution.BaseCursor._execute()andAioBaseCursor._execute()skip_find_previous_query_id()when the request carriesExecutionParameters.Queries without parameters, including
pyformatqueries, keep the existing cache behavior.The
ExecuteOptions.cache_sizeand_execute()docstrings and the cache section ofdocs/usage.mdstate the new rule.Release note (behavior change):
cache_size/cache_expiration_timehave no effect onqmarkqueries with parameters.Before this change they could return the result of an execution that ran with different parameter values.
A repeated
qmarkquery with the same parameters, which the cache used to reuse, now starts a new execution as well.WHY
Closes #941.
With
qmark, the query text keeps its?placeholders and the values go to Athena asExecutionParameters, so the cache search matched an earlier execution with the same text but different values.The issue proposed comparing
AthenaQueryExecution.execution_parameterswith the current parameters.Athena does not return them: measured on 2026-10-03 (work groups
pyathenaandprimary, engine version 3), a query started withExecutionParameters=["'1'"]has noExecutionParameterskey in eitherGetQueryExecutionorBatchGetQueryExecution, also when read again minutes later.The botocore 1.43.102
QueryExecutionshape declares the field but documents it: "The list of parameters is not returned in the response."Every earlier execution therefore looks parameterless, so a comparison could never match a parameterized query; it would only spend
ListQueryExecutions/BatchGetQueryExecutioncalls.Skipping the search gives the same results without those calls.
TEST
Tested commit: 9126987 (the later 9f3557c changes only one
docs/usage.mdsentence;just docs lintre-run on it: 0 errors)just lint: passed.just docs lint: 0 errors.--noconftestwith dummy environment variables): the newtest_execute_qmark_parameters_skip_cache(sync and aio) and the existing cache-search and legacy-kwargs passthrough tests pass; with thepyathena/change reverted, both new tests fail.uv run --env-file .env pytest -n 1 tests/pyathena/test_cursor.py tests/pyathena/aio/test_cursor.py -k "cache or qmark or legacy_kwargs": 21 passed, including the newtest_cache_size_with_qmark_parameters, which runs the issue's reproduction (different parameters) and checks that the same parameters also start a new execution.just test pyathenasuite and the SQLAlchemy suites; the change is limited to_execute()and is covered by the AWS CI once Ready.🤖 Generated with Claude Code