Backport bug fixes and the CI test policy to 3.x for v3.36.1 - #891
laughingman7743 wants to merge 7 commits into
Conversation
Port the policies of #789, #837, #862, and #863 to the 3.x workflows without the parallel suites or the per-change suite selection: - A lint job runs for every pull request, including Draft and fork ones. - Draft and external-fork pull requests run no AWS suites; a ready pull request runs them on the newest Python version only. - workflow_dispatch accepts a python-versions input and otherwise runs every supported version. - The Release workflow runs every suite on every supported Python version for the tagged commit through workflow_call before building and publishing, so a failing tag publishes nothing. The fixed S3 Tables namespace no longer exists in the test account, so AWS_ATHENA_S3_TABLES_NAMESPACE is dropped and the SQLAlchemy S3 Tables tests skip on this branch. The weekly schedule is removed because scheduled runs only use the default branch's workflow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit ad1232b) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ermination fails (cherry picked from commit 2a107ad) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 37999c3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SQLAlchemy's generic Double and DOUBLE_PRECISION are not DOUBLE subclasses, so CAST fell through to the Float branch and rendered REAL, losing precision. This backports only the compiler fix from #826 (cherry picked from commit 3ddb868), with a regression test that runs on SQLAlchemy 2.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CREATE TABLE rendered AthenaStruct columns as ROW(...), which Athena rejects in table DDL. That affected top-level STRUCT columns and STRUCT values inside MAP and ARRAY. Column DDL now renders STRUCT<name:type, ...> at every nesting depth, and quotes field names with the DDL identifier preparer. This reimplements #870 (cherry picked from commit cf673c2) for 3.x, which lacks the ARRAY DDL context from #774. Integer spelling inside MAP and STRUCT stays INTEGER, which Athena's DDL accepts. Direct type compilation, CAST output, and an empty AthenaStruct() are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| def visit_enum(self, type_, **kw): | ||
| return self.visit_string(type_, **kw) | ||
|
|
||
| def _enable_hive_column_ddl(self, kw: dict[str, Any]) -> bool: |
There was a problem hiding this comment.
Self-review round one (behavior and implementation), by the authoring model; not an independent review.
Scope: base 4ef4d3e, head 610d6c3, all 13 changed files.
Result: CLEAN. No actionable findings within this scope.
Checked:
- STRUCT DDL (fix: render Hive STRUCT syntax in table column DDL #870 reimplementation):
- Only
get_column_specificationpasses aColumnastype_expression.CASTpasses aTypeClause, so CAST output is unchanged. - A
TypeDecoratorkeepskwwhen it resolves to its implementation. - Field names use
AthenaDDLIdentifierPreparer, sodaterenders as`date`and backticks double. - Empty
AthenaStruct()and direct type compilation are unchanged. - The new DDL tests fail with the 3.x compiler (2 failed).
- Only
CASTDouble (SQLAlchemy compliance: audit numeric range and float precision #826): the check requiresDouble, which exists only in SQLAlchemy 2.0. On older SQLAlchemy versions,CASTrenders as before.DOUBLEsubclassesDouble, and theDoublecheck runs before theFloatbranch.- Spark (Shut down the AsyncSparkCursor executor when session termination fails #830 Stop Spark session readiness polling on failure states #831 Terminate a newly started Spark session when cursor setup fails #832):
- 3.x
SparkBaseCursor.close()calls_terminate_session()on every call, so a failed termination is retried. - Startup cleanup calls the synchronous
_terminate_session_by_id, soAioSparkCursor's coroutine override is never invoked from the constructor. - The executor and the S3 client are created before any session starts.
- 3.x
to_sql(Find an existing table in to_sql whatever the name's case #800):schemadefaults to"default"and is never None. Quotes in both literals are doubled.- CI:
- On this Draft PR, only lint ran;
versions,test,test-sqla, andtest-sqla-asyncwere skipped. - A tag push calls
test.yamlwithevent_name == 'push', which selects every Python version. - The S3 Tables tests (test_base.py:2041, :2082, :2117) are all gated by
requires_s3_tables, which needs the namespace, so they skip.
- On this Draft PR, only lint ran;
Out of scope (pre-existing on 3.x, not changed here):
CASTto STRUCT/MAP/ARRAY rendersROW(date STRING, ...)andMAP<...>, which Trino rejects. master fixed this in the ARRAY work (Support native SQLAlchemy ARRAY types and typed round trips #774) and later changes.- The generic
sqlalchemy.types.ARRAYrendersARRAY<STRING>.
| # requests run none. A ready pull request tests the newest Python version; a | ||
| # dispatch tests the requested versions or every version, and the Release | ||
| # workflow every version. | ||
| versions: |
There was a problem hiding this comment.
Self-review round two (claims, callers, and operations), by the authoring model; not an independent review.
Scope: base 4ef4d3e, head 610d6c3, covering the PR body, the commit messages, the changed docstrings and comments, and docs/sqlalchemy.md.
Result: FINDINGS. There were two gaps in the PR description; both are corrected, and the code is unchanged.
Claims checked:
- The fixed S3 Tables namespace no longer exists.
s3tables:GetNamespace(pyathena)returns NotFound. All three S3 Tables tests are gated on the namespace, so they skip. - Draft and fork PRs run no AWS suites. For Draft, this was observed on this PR's run: only lint ran. For forks, it follows from the conditions on
versionsand ontest-suite.yaml'srunjob. - A tag runs every version before publishing. This holds in the code:
releaseneedstest, and the called workflow'sevent_nameispush, so it selects the full list. It is not exercised until a tag is pushed, and neither is master's Run the full test matrix in the Release workflow before publishing #862. - "Scheduled runs only use the default branch's workflow" is GitHub's documented behavior, so the 3.x schedule never ran.
- The reorder changed only the workflows:
git diff 7c1db8f 610d6c3touches only.github/workflows/*, so the local test results still apply. - STRUCT DDL, Double CAST, and INTEGER being accepted in DDL: covered by the round-one evidence and the Athena probe on fix: render Hive STRUCT syntax in table column DDL #870.
Existing callers:
- 3.x allows
sqlalchemy>=1.0.0. The compiler code only usesColumnand ahasattr(types, "Double")guard, and the new Double test skips on SQLAlchemy versions below 2.0. - No other docs describe DDL with
ROW(...).docs/usage.md:596is a SELECT with CAST.
Operations:
- A ready 3.x PR now runs the three suites on Python 3.14 only.
- A tag runs the three suites on five Python versions, with 3.x's retry behavior and without master's rerun-once (Rerun tests once on Athena service-side query failures #811). A transient failure blocks publishing until the failed jobs are re-run.
Corrections: the PR body's TEST section now records the Draft CI observation, the namespace check, and the fact that the release gate has not run yet.
Separate finding, not in this PR: the test table bucket holds 539 namespaces, mostly pyathena_test_*. Per-session namespaces from master (#815) are accumulating. To be reported separately.
WHAT
Backport bug fixes from master to the
3.xmaintenance branch for v3.36.1, and port the CI test policy and release gate so that this and later3.xpull requests stop running every suite on every Python version.Each commit is one change:
workflow_dispatchacceptspython-versions.AWS_ATHENA_S3_TABLES_NAMESPACEis dropped and the SQLAlchemy S3 Tables tests skip on3.x.to_sqlfinds an existing table whatever the case of its name (clean cherry-pick).AsyncSparkCursor.close()shuts down its executor when session termination fails (clean cherry-pick).TERMINATED,DEGRADED, andFAILEDinstead of polling forever._terminate_session_by_id, as on master after Replace name-mangled helpers with single-underscore methods #881.CASTtosa.DoubleorDOUBLE_PRECISIONrendersDOUBLEinstead ofREAL.CREATE TABLErendersSTRUCT<name:type, ...>forAthenaStructat every depth of a column type, including inside MAP and ARRAY.INTEGER, which Athena's DDL accepts.CASToutput, and an emptyAthenaStruct()are unchanged.Release notes for v3.36.1:
CASTtoDouble/DOUBLE_PRECISIONnow yieldsDOUBLEinstead ofREAL.CREATE TABLEstrings containSTRUCT<...>instead ofROW(...)for struct columns.WHY
These fixes are merged on master (4.x development) and apply to 3.x users without behavior changes beyond the corrected output. #855 promised the STRUCT fix for
3.x.The
3.xworkflows still ran every suite on every Python version for every pull request event, including drafts, and had no release test gate.TEST
Tested on the pre-reorder head 7c1db8f with Python 3.13.1 and SQLAlchemy 2.0.46. The reorder moved the CI commit to the front without changing any code or test files.
just lint: passed (ruff check, ruff format --check, mypy).to_sql.test_floating_point_typesfails forDoubleandDOUBLE_PRECISION(2 failed).actionlint1.7.12: no findings fortest.yaml,test-suite.yaml, andrelease.yaml.versions,test,test-sqla, andtest-sqla-asyncwere skipped.s3tables:GetNamespaceforpyathenain the test table bucket returns NotFound, so the fixed namespace is gone. The SQLAlchemy S3 Tables tests (test_base.py:2041,:2082,:2117) are all gated onAWS_ATHENA_S3_TABLES_NAMESPACE.markdownlint-cli2 docs/sqlalchemy.md: 0 errors.🤖 Generated with Claude Code