Skip to content

Cast to a TypeDecorator as its implementation type - #836

Merged
laughingman7743 merged 7 commits into
masterfrom
fix/829-typedecorator-cast
Sep 26, 2026
Merged

laughingman7743 merged 7 commits into
masterfrom
fix/829-typedecorator-cast

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

WHAT

  • Before choosing the Athena DML type name, visit_cast now resolves the type the dialect actually uses.
    It repeatedly takes the awsathena variant from with_variant(), or unwraps a TypeDecorator, until neither applies.
    This covers nested decorators, variants on a decorator or on its implementation, and variants on plain types.
    The resolution lives in a new _dialect_type helper, shared with the ARRAY/MAP/ROW element path (_complex_dml_type) and _timestamp_dml_type.
    Element types with an awsathena variant therefore cast by the variant, as the table DDL already does:

    • ARRAY(String().with_variant(Integer(), "awsathena")): ARRAY(VARCHAR) → ARRAY(INTEGER) (the DDL is ARRAY<INT>).
    • ARRAY(DateTime().with_variant(AthenaTimestamp(3), "awsathena")): ARRAY(TIMESTAMP(6)) → ARRAY(TIMESTAMP(3)).
      The unwrapping goes through _ArrayTypeInspector.decorator_impl, which _timestamp_dml_type and _complex_dml_type already use.
      A decorated type now renders the same CAST as its implementation type.
    Decorated impl Before After
    String(50), Text() CAST(x AS STRING) CAST(x AS VARCHAR)
    LargeBinary(), VARBINARY() CAST(x AS BINARY) CAST(x AS VARBINARY)
    Float(), REAL() CAST(x AS FLOAT) CAST(x AS REAL)
    ARRAY(String) CAST(x AS ARRAY<STRING>) CAST(x AS ARRAY(VARCHAR))
    AthenaMap(String, Integer) CAST(x AS MAP<STRING, INTEGER>) CAST(x AS MAP(VARCHAR, INTEGER))
    CHAR(3) CAST(x AS CHAR(3)) CAST(x AS VARCHAR), the same as a plain CHAR(3)

    Integer, Numeric, Boolean, Date, DateTime, AthenaTimestamp, Double, and STRUCT types were already unaffected.

  • Variants on plain types (pre-existing, same failure class):

    Target Before After
    Integer().with_variant(String(50), "awsathena") CAST(x AS STRING) CAST(x AS VARCHAR)
    Integer().with_variant(Float(), "awsathena") CAST(x AS FLOAT) CAST(x AS REAL)
    String().with_variant(Integer(), "awsathena") CAST(x AS VARCHAR) CAST(x AS INTEGER)

    A variant for another dialect is still ignored.

  • ARRAY element values (bind, literal, and result processing in pyathena/sqlalchemy/array.py) now resolve an awsathena variant on a plain element type.
    Previously only a variant on a TypeDecorator element was honored.
    For example, AthenaArray(String().with_variant(Integer(), "awsathena")) returned ['1', '2'] instead of [1, 2], although the column and casts use INTEGER.
    A variant that changes the element's kind (such as String → AthenaMap) raised TypeError on bind.
    The variant is now resolved first, so the decorator branches no longer need their not in _variant_mapping conditions.

  • The typed ARRAY JSON projection used in SELECT (_array_json) resolves element variants with the same _dialect_type helper.
    An element whose awsathena variant is a MAP, ROW, ARRAY, or binary type is no longer projected as a scalar VARCHAR.

  • Variant lookups go through one helper, _ArrayTypeInspector.variant, which tolerates SQLAlchemy 1.x types without _variant_mapping.

  • visit_cast gains a Google-style docstring.

  • A test helper, tests.pyathena.util.decorated(impl), builds a TypeDecorator around impl.

WHY

Closes #829.

Athena rejects the Hive DDL names STRING, BINARY, FLOAT, and MAP<...> in DML with TYPE_MISMATCH: Unknown type, so these casts failed at execution time.

Behavior note for reviewers: a decorated CHAR(n) previously produced CHAR(n), which Athena accepts.
It now follows the plain CHAR(n) mapping (VARCHAR), which comes from the existing isinstance(type_, types.String) branch.
The plain mapping is unchanged here.

SQLAlchemy 1.x: types there have no _variant_mapping, so the variant lookup is skipped.
Checked in an isolated SQLAlchemy 1.4.54 environment:

  • Plain casts and AthenaArray(Integer) bind, literal, and result processing behave as on master.
  • A decorated String cast, which raised AttributeError on master, now renders CAST(x AS VARCHAR).
  • Raising the declared SQLAlchemy floor to 2.0 is proposed separately.

Correction to the issue: it said decorated ARRAY binds skipped the explicit-precision check.
They do not.
The ARRAY bind path casts to the unwrapped AthenaArray, and bindparam(..., type_=Decorated(AthenaArray(Numeric()))) already raised the precision CompileError before this change.

TEST

Tested commit: 3a04a32 (all results below, including Athena), which includes a merge of master at c01c56f.

  • just lint: passed.
  • uv run pytest tests/pyathena/sqlalchemy/test_compiler.py -k resolves_variants: 21 passed.
    • Each case checks the plain, decorated, and double-decorated type.
    • On master's compiler, the decorator cases fail except Double and Numeric.
    • With decorator unwrapping but no variant resolution (b7425b9), the three top-level awsathena variant cases fail.
    • With top-level variants only (b1cee3b), the three element-variant cases fail.
  • uv run pytest over test_compiler.py, test_array.py, test_temporal.py, test_types.py, test_map.py, and test_struct.py in tests/pyathena/sqlalchemy/, with -k "not Integration": 384 passed.
  • test_array.py::...::test_element_variant_bind_literal_and_result_paths: 2 passed. Each case covers bind, literal, result, and the SELECT projection.
    • Without the array.py change, both fail.
    • Without the _array_json change, the MAP case fails.
  • uv run --env-file .env pytest -n 1 tests/pyathena/sqlalchemy/test_base.py -k test_cast_as_decorated_types against Athena: passed.
    • It casts to String(50), LargeBinary, Float, and AthenaMap in one query, plain and decorated, and asserts that both return the same values.
    • Without the compiler change, it fails with TYPE_MISMATCH: line 1:8: Unknown type: STRING.
  • test_cast_as_varchar and test_cast_as_binary against Athena: passed.
    All three Athena tests were rerun at 3a04a32: 3 passed.
  • test_array_element_variant_round_trip against Athena: passed.
    • It binds [1, 2] as a parameter and as a literal to AthenaArray(String().with_variant(Integer(), "awsathena")).
    • Without the array.py change, it fails with ['1', '2'] != [1, 2].

Not run locally: the full just test pyathena, just test sqla, and just test sqla-async suites (left to CI).

🤖 Generated with Claude Code

laughingman7743 and others added 2 commits September 26, 2026 14:01
visit_cast checked the decorator itself, so decorated String, binary,
Float, ARRAY, and MAP types fell through to the DDL type names STRING,
BINARY, FLOAT, ARRAY<...>, and MAP<...>. Athena rejects STRING, BINARY,
FLOAT, and MAP<...> in DML. Resolve decorators before choosing the
type clause, as the timestamp and ARRAY paths already do. (fix #829)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/sqlalchemy/compiler.py Outdated
"""
type_ = cast.type
while isinstance(type_, types.TypeDecorator):
type_ = self._array_type_inspector.decorator_impl(type_)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round one — implementation behavior (Claude Code)

Scope: git diff 37999c3c6f52365a25297782d544d8412bab4801..b7425b9a72766f73ad68f2c09081cd59f81ec004. Covers compiler.py, tests/pyathena/util.py, test_compiler.py, and test_base.py.

Result: CLEAN, with one wording fix to the PR body.

  • Unwrapping:
    • decorator_impl returns the decorator's awsathena variant or load_dialect_impl(). The default load_dialect_impl returns the impl instance, never the decorator itself, so the loop terminates.
    • Nested decorators resolve; the unit test covers two levels.
  • Branches: every isinstance check now uses the unwrapped type. _timestamp_dml_type keeps its own loop because _complex_dml_type also calls it with possibly decorated element types; for visit_cast the loop is now a no-op.
  • Fallback: cast.typeclause still dispatches on the decorator, and visit_type_decorator renders the impl. Probed plain, decorated, and double-decorated Integer, Numeric, Boolean, Date, DateTime, AthenaTimestamp(3), Double, and STRUCT: all identical.
  • Result processing is unchanged: Cast.type is still the decorator, so process_result_value runs as before. Only the SQL type name changes.
  • ARRAY binds: bindparam(..., type_=Decorated(AthenaArray(Numeric()))) raised the precision CompileError before and after this change (probed both). The claim in SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects #829 that the check was skipped was wrong, and the PR body corrects it.
  • Tests:
    • The unit test fails 10 of 12 cases without the change; the Double and Numeric cases were already correct.
    • The Athena test fails without the change with Unknown type: STRING.
    • The Athena test compares plain against decorated in one query, instead of pinning the existing MAP value decoding ({'a': '1'}).
  • Wording fix: the PR body said "dialect variants" generally. Variants on a decorator are resolved, but a variant on a plain type (for example String().with_variant(Float(), "awsathena")) still renders by its base type (VARCHAR), unchanged from master. The body now says so.

Out of scope, pre-existing: visit_cast ignores with_variant on a non-decorator type.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round one — repair follow-up for b1cee3b (Claude Code)

Scope: git range-diff 37999c3..b7425b9 37999c3..b1cee3b. The merge-base is unchanged, and b1cee3b adds one commit touching visit_cast and test_compiler.py.

Result: CLEAN.

  • Termination: a variant value cannot itself carry variants (SQLAlchemy raises ArgumentError: can't pass a type that already has variants), and decorators unwrap to their implementation, so the loop ends.
  • Ordering:
    • A variant on a decorator is taken before unwrapping, matching decorator_impl.
    • A variant on the implementation is taken on the next pass (the Codex finding).
    • A variant for another dialect is ignored.
  • SQLAlchemy 1.x: getattr(type_, "_variant_mapping", {}). In an isolated SQLAlchemy 1.4.54 environment, plain String/Float/LargeBinary/DateTime casts render as on master, and decorated/Variant casts raise the same AttributeError as on master (from decorator_impl, unchanged).
  • Validation:
    • just lint passed.
    • No-AWS SQLAlchemy tests: 378 passed.
    • test_cast_resolves_variants_and_decorators: 17 passed, and 3 fail at b7425b9.
    • Athena cast tests: 3 passed at b1cee3b (-n 1).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round one — repair follow-up for b03b588 (Claude Code)

Scope: b1cee3b..b03b58838452dba65ea5e079fbba4c831fd22070 (one commit, compiler.py and test_compiler.py), on the same merge-base 37999c3.

Result: CLEAN.

  • _dialect_type is the loop from b1cee3b, moved into a method. For decorators it behaves as before; it additionally takes awsathena variants.
  • _complex_dml_type:
    • It previously recursed once per decorator; the loop now resolves fully before the branch checks. For decorated elements this is equivalent.
    • NullType and Numeric precision checks run on the resolved type, as they did after the old decorator recursion.
    • Callers are the ARRAY bind and literal casts, the typed ARRAY JSON transport, and visit_cast. All now see element variants.
  • _timestamp_dml_type: the resolution is idempotent, so calling it after _complex_dml_type or visit_cast has already resolved the type is harmless.
  • SQLAlchemy 1.x: the helper keeps the getattr guard. _complex_dml_type on a plain type no longer reads _variant_mapping directly.
  • Validation:
    • just lint passed.
    • No-AWS SQLAlchemy tests: 382 passed.
    • test_cast_resolves_variants_and_decorators: 21 passed, and 3 fail at b1cee3b.
    • Athena cast tests at b03b588: 3 passed (-n 1).
    • The array and typed-transport integration paths are left to CI (test, test-sqla, test-sqla-async).

assert actual[1] == b"varchar"
assert actual[2] == b"a string"

def test_cast_as_decorated_types(self, engine):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round two — claims, compatibility, evidence (Claude Code)

Scope: 37999c3c6f52365a25297782d544d8412bab4801..b7425b9a72766f73ad68f2c09081cd59f81ec004, plus the PR body and commit messages.

Result: CLEAN, after the round-one wording fix.

Claims checked:

  • Athena rejects STRING, BINARY, FLOAT, and MAP<STRING, INTEGER> in CAST: measured live today (TYPE_MISMATCH: line 1:8: Unknown type: ...). ARRAY<INT> is accepted, which is why the table lists the ARRAY(String) case.
  • The "Before" column: produced by compiling at merge-base 37999c3.
  • The CHAR note: a decorated CHAR(3) gave CHAR(3) on master and now gives VARCHAR, like plain CHAR(3). The elif isinstance(type_, types.CHAR) branch remains unreachable for all String subclasses, as on master.

Caller compatibility:

  • Decorated casts that previously failed on Athena now execute.
  • Decorated casts that already worked may render differently. ARRAY<INT> becomes ARRAY(INTEGER), both valid. A decorated ARRAY of DateTime now gets TIMESTAMP(6) elements like the plain type (previously the Hive ARRAY<TIMESTAMP>). A decorated CHAR(n) becomes VARCHAR.
  • No public API or signature changes.

Evidence limits:

  • The Athena run covered test_cast_as_decorated_types, test_cast_as_varchar, and test_cast_as_binary on the default rest engine, with -n 1.
  • The full pyathena, sqla, and sqla-async suites are left to CI.
  • The new integration test adds one Athena query per run.
  • Docs: docs/sqlalchemy.md:1096 (outer TypeDecorator result processor) is unaffected, and no documentation describes decorated CAST output.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round two — claim check for b1cee3b (Claude Code)

Result: CLEAN.

  • The PR body now describes variant resolution, with a before/after table for plain variants. Each row was compiled at master ("Before") and at b1cee3b ("After").
  • The SQLAlchemy 1.x statement is limited to what the isolated 1.4.54 run showed: plain casts are unchanged, and decorated/Variant casts keep the existing AttributeError.
  • The TEST section now names b1cee3b as the tested commit for all results, including the Athena rerun.
  • Compatibility:
    • A plain type with an awsathena variant now casts as the variant, as its DDL already did (type_compiler.process gives INTEGER for String().with_variant(Integer(), "awsathena") on master).
    • Variants for other dialects are unaffected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round two — claim check for b03b588 (Claude Code)

Result: CLEAN.

  • The PR body's element-variant examples were compiled at master and at b03b588: ARRAY(VARCHAR) → ARRAY(INTEGER), and ARRAY(TIMESTAMP(6)) → ARRAY(TIMESTAMP(3)).
  • The DDL comparison (ARRAY<INT> from type_compiler.process) was checked earlier.
  • The TEST section names b03b588 for all results, and its counts match the runs: 21 in the parametrized test, 382 no-AWS, 3 on Athena.
  • The SQLAlchemy 1.x statement in the body is unchanged and still accurate: plain casts render as on master, and decorated and Variant casts raise the pre-existing AttributeError.
  • Compatibility: element types with an awsathena variant now cast by the variant in bind, literal, and transport casts. Variants for other dialects and elements without variants render as before.

Unwrapping a TypeDecorator dropped a with_variant() on its
implementation, so a decorated String().with_variant(Integer(),
"awsathena") was cast as VARCHAR instead of INTEGER. A variant on a
plain type was also ignored and fell through to the DDL names STRING
and FLOAT. Resolve variants and decorators in one loop, and skip the
variant lookup on SQLAlchemy 1.x, whose types have no variant mapping.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/sqlalchemy/compiler.py Outdated
"""
type_ = cast.type
while True:
# SQLAlchemy 1.x types have no _variant_mapping.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review (relayed)

This is a static review of a detached snapshot; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0dc18-f181-7b20-9b96-44779811f944.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: 37999c3c6f52365a25297782d544d8412bab4801..b7425b9a72766f73ad68f2c09081cd59f81ec004.
  • The prompt contained the literal diff and constraints only.

Covered surfaces:

  • Decorator unwrapping, nested decorators, variants, and load_dialect_impl.
  • Every visit_cast branch and the typeclause fallback.
  • Complex and timestamp rendering, ARRAY bind annotations, and internal Cast callers.
  • Result processing, the SQLAlchemy version range, and the new tests.

Result: FINDINGS (2).

P2, introduced — compiler.py:654: the loop unwraps a decorator but does not resolve a variant on the returned implementation. cast(column("x"), decorated(String().with_variant(Integer(), "awsathena"))) emits CAST(x AS VARCHAR); before this change the typeclause path selected the variant and emitted CAST(x AS INTEGER).

P2, pre-existing — compiler.py:652: a bare String().with_variant(Integer(), "awsathena") is also unresolved, and both revisions emit VARCHAR.

Author disposition: both verified and fixed in b1cee3b.

  • Probing at master and at b7425b9 confirmed both findings.
  • The pre-existing gap was wider than reported: Integer().with_variant(String(), "awsathena") rendered CAST(x AS STRING), and a Float variant rendered FLOAT. Both are invalid in Athena, the same failure class as SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects #829.
  • visit_cast now loops over both steps: take the awsathena variant, else unwrap a TypeDecorator.
  • Five cases were added to test_cast_resolves_variants_and_decorators, including a variant for another dialect and a variant on a decorator. Three of them fail at b7425b9.

The snapshot and PR worktree were unchanged by the review.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review follow-up 1 (relayed)

This is a static review of a detached snapshot at b1cee3b; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0dc24-7b9e-7da0-844e-5c5f32583186.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: b7425b9..b1cee3bf47e9aca382ba6605e561f8e6018c3f2d, on the same merge-base 37999c3.

Result: FINDINGS (2, both pre-existing). The reviewer confirmed the loop's termination and variant/decorator ordering for top-level types, and that the added cases fail on the prior code.

P2, pre-existing — array.py:217: decorator_impl reads _variant_mapping unconditionally, so on SQLAlchemy 1.x a decorated or Variant cast raises AttributeError. The getattr in visit_cast does not resolve that.

P2, pre-existing — compiler.py:746: _complex_dml_type resolves element types without their Athena variants. cast(column("col"), ARRAY(String().with_variant(Integer(), "awsathena"))) renders ARRAY(VARCHAR), and an AthenaTimestamp(3) variant on a nested DateTime renders TIMESTAMP(6).

Author disposition:

  • SQLAlchemy 1.x: deferred, not changed.
    • Verified in an isolated SQLAlchemy 1.4.54 environment: master already raises the same AttributeError for decorated and Variant casts, and this PR keeps plain casts working as on master.
    • decorator_impl and the ARRAY value processors in array.py access _variant_mapping directly in several places, and the test tree already requires SQLAlchemy 2.0 at import.
    • Making them 1.x-compatible is a broader support question than SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects #829.
  • Element variants: verified and fixed in b03b588.
    • The loop moved to a _dialect_type helper, now used by visit_cast, _complex_dml_type (replacing its decorator-only branch), and _timestamp_dml_type (replacing its decorator-only loop).
    • master renders ARRAY(VARCHAR) and ARRAY(TIMESTAMP(6)); this commit renders ARRAY(INTEGER) and ARRAY(TIMESTAMP(3)), matching the table DDL (ARRAY<INT>).
    • Three element-variant cases were added, and all three fail at b1cee3b.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review follow-up 2 (relayed)

This is a static review of a detached snapshot at b03b588; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0dc33-89e5-7790-bb4a-fb6aa96a0065.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: b1cee3b..b03b58838452dba65ea5e079fbba4c831fd22070, on the same merge-base 37999c3.

Covered surfaces:

  • Every caller of _complex_dml_type and _timestamp_dml_type.
  • Decorator and variant resolution, and ARRAY bind and literal processing.
  • The NullType and Numeric precision checks.
  • Termination, SQLAlchemy 1.x relative to the previous head, and the added cases.

Result: FINDINGS (1, pre-existing). The reviewer confirmed the following:

  • The extraction preserves decorator resolution and termination.
  • The precision and NullType checks still apply after resolution.
  • SQLAlchemy 1.x follows the previous head's decorator path.
  • The three nested variant cases fail without this commit.

P2, pre-existing — array.py:310: ARRAY value processing does not resolve a variant on a non-decorator element before checking value shape. For AthenaArray(String().with_variant(AthenaMap(String, Integer), "awsathena")), the compiler renders ARRAY(MAP(VARCHAR, INTEGER)), but binding a dict element raises TypeError.

Author disposition: deferred, not changed.

  • Verified: the bind and literal processors both raise TypeError: ARRAY element shape does not match its declared type.
  • pyathena/sqlalchemy/array.py is unchanged by this PR, so the behavior is the same on master.
  • SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects #829 covers CAST type names. ARRAY value conversion for a variant that changes the element's kind is a separate surface, and a separate issue can be filed if wanted.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review follow-up 3 (relayed)

This is a static review of a detached snapshot at e1b0ae8; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0dca1-c37f-7f43-af8e-75bd3b503aab.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: 87c831e..e1b0ae8a6ef4a8c77a20eb96119a6f1473ed15d7, on merge-base c01c56f.

Covered surfaces:

  • TypeDecorator bind, literal, and result hooks.
  • Plain scalar and complex variants, as_tuple, and termination.
  • ARRAY type inspection, SQL compilation, and the added tests.

Result: FINDINGS (3).

P1 — array.py:300, 330, 364: each processor now reads _variant_mapping on every element type. With SQLAlchemy 1.x, an ordinary AthenaArray(Integer) element raises AttributeError.

P2, pre-existing — compiler.py:779: the ARRAY JSON projection does not resolve a plain element's variant. AthenaArray(String().with_variant(AthenaMap(String, Integer), "awsathena")) is projected as CAST(... AS VARCHAR) instead of through map_entries(...).

P2, pre-existing — compiler.py:239: ARRAY DDL does not resolve element variants, so String().with_variant(Integer(), "awsathena") renders ARRAY<STRING>.

Author disposition:

  • P1: verified and fixed in 3a04a32.
    • A single _ArrayTypeInspector.variant() uses getattr(type_, "_variant_mapping", {}), and the processors, decorator_impl, and _dialect_type all use it.
    • In isolated SQLAlchemy 1.4.54, AthenaArray(Integer) bind, literal, and result behave as on master.
    • A decorated String cast, which raised AttributeError on master, now renders VARCHAR.
  • Projection: verified and fixed in 3a04a32.
    • _array_json resolves types with _dialect_type, replacing its decorator-only branch.
    • The element-variant test now also asserts the projection, and the MAP case fails without the change.
  • DDL: rejected. AthenaDialect().type_compiler_instance.process(...) renders ARRAY<INT> for both AthenaArray(String().with_variant(Integer(), "awsathena")) and types.ARRAY(...). SQLAlchemy's type compiler resolves the variant before visit_array processes the element.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review follow-up 4 (relayed)

This is a static review of a detached snapshot at 3a04a32; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0dcab-761f-7c83-ada7-5029804656a5.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: e1b0ae8..3a04a3281f1133e39aeca6b4ddf07a1107e4375b.

Covered surfaces:

  • The three value processors, and decorator and variant resolution on SQLAlchemy 1.4 and 2.x.
  • The recursive JSON projection for decorated, nested, binary, JSON/NullType, and ROW elements.
  • The remaining direct type inspections, and the added tests.

The reviewer found both previously reported defects resolved.

Result: FINDINGS (3, all pre-existing).

P2 — array.py:113: has_unknown_element() ignores variants, so AthenaArray(NullType().with_variant(String(), "awsathena")) skips the JSON projection and returns the raw string.

P2 — compiler.py:239: nested ARRAY DDL renders ARRAY<STRING> for an element declared as String().with_variant(AthenaMap(String, Integer), "awsathena").

P3 — array.py:599: partial-update index validation rejects a bound index declared as String().with_variant(Integer(), "awsathena").

Author disposition:

  • DDL: rejected. type_compiler_instance.process(AthenaArray(String().with_variant(AthenaMap(String, Integer), "awsathena"))) renders ARRAY<MAP<STRING, INT>>, because SQLAlchemy's type compiler resolves element variants.
  • NullType element with a variant: verified, deferred.
    • The projection is skipped, as the reviewer described.
    • has_unknown_element runs in column_expression without a dialect, so resolving variants there is a larger change.
    • The case requires declaring an unknown element type and then giving it a variant.
  • Write-index variant: deferred. It requires declaring an ARRAY write index as a non-integer type with an integer variant.
  • Neither deferred item is changed by this PR. Both are outside SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects #829's CAST scope and the ARRAY value fix requested for this PR.

Element types in DML casts only unwrapped decorators, so an element
with an awsathena variant was cast by its base type, while the table
DDL already used the variant. Resolve variants and decorators in one
helper for the top-level cast, element types, and timestamp precision.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@laughingman7743
laughingman7743 marked this pull request as ready for review September 26, 2026 06:17
…or-cast

# Conflicts:
#	tests/pyathena/util.py
@laughingman7743
laughingman7743 marked this pull request as draft September 26, 2026 07:29
ARRAY bind, literal, and result processing honored a variant only on a
TypeDecorator element. A plain element type with an awsathena variant
was converted as its base type, so String().with_variant(Integer(),
"awsathena") elements came back as strings although the column and
casts use INTEGER. Resolve the variant first for every element type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pyathena/sqlalchemy/array.py Outdated
return None

def _bind(self, value: Any, type_: TypeEngine[Any]) -> Any:
if self.dialect.name in type_._variant_mapping:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review, both rounds — ARRAY element value variants (87c831e..e1b0ae8) (Claude Code)

Scope:

  • 87c831e merges master at c01c56f. Its only conflict was tests/pyathena/util.py, where both sides appended helpers, and both were kept.
  • e1b0ae8 changes array.py, test_array.py, and test_base.py.
  • New merge-base: c01c56f73c7dbf973fe73b52b6093f0a32952321.

Round one — behavior: CLEAN.

  • _bind, _literal, and _decode now return early through the awsathena variant for any type, before the TypeDecorator branch.
  • For a decorator carrying a variant, this is the old path: its own processors were skipped, and it recursed into decorator_impl, which returned the variant. The not in _variant_mapping conditions in the decorator branch were therefore always true after the early return, and were removed.
  • Plain element types with a variant now convert as the variant.
  • Termination: a variant cannot carry variants (SQLAlchemy ArgumentError).
  • _decode passes as_tuple through the variant recursion.
  • Tests:
    • No-AWS SQLAlchemy tests: 384 passed.
    • test_element_variant_bind_literal_and_result_paths: 2 passed, and 2 fail without the array.py change.
    • Athena test_array_element_variant_round_trip: passed, and fails without the change (['1', '2'] != [1, 2]).
    • The three cast tests passed again at e1b0ae8.

Round two — claims and compatibility: CLEAN.

  • The PR body's examples were probed before and after the change:
    • Decode gave ['1'] → [1].
    • Binding a dict element for a String → AthenaMap variant raised TypeError → now produces ARRAY[MAP(ARRAY['a'], ARRAY[1])].
  • Compatibility:
    • A plain element type with an awsathena variant now binds, renders, and decodes as that variant, matching its DDL and the casts from the earlier commits.
    • Elements without variants, and variants for other dialects, are unchanged. The existing decorator and variant tests (test_decorator_bind_literal_and_result_paths, test_array_variant_keeps_transport_and_result_types) still pass.
  • SQLAlchemy 1.x: array.py already read _variant_mapping directly on this path, so nothing changes there. Raising the declared floor to 2.0 will be proposed in a separate issue.
  • CI: under the new trigger policy (Run AWS test suites only on ready pull requests and related changes #837), the PR was returned to Draft before pushing, and the AWS suites will run when it is marked Ready after review.

The typed ARRAY projection unwrapped decorators but not variants, so an
element whose awsathena variant is a MAP, ROW, ARRAY, or binary type was
projected as a scalar VARCHAR. Resolve it with the same helper as DML
casts.

Look up variants through one _ArrayTypeInspector.variant helper that
tolerates SQLAlchemy 1.x types, which have no variant mapping, so the
value processors keep working there for plain element types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Returns:
The variant type, or None when the type has no variant for this dialect.
"""
# SQLAlchemy 1.x types have no _variant_mapping.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review, both rounds — 3a04a32 (Claude Code)

Scope: e1b0ae8..3a04a3281f1133e39aeca6b4ddf07a1107e4375b, covering array.py, compiler.py, and test_array.py.

Round one — behavior: CLEAN.

  • variant() returns the same value as the old _variant_mapping lookups on SQLAlchemy 2.x. The only other change is that 1.x types without the attribute yield None.
  • decorator_impl keeps its semantics.
  • _array_json now resolves the whole chain up front instead of recursing once per decorator, which is equivalent for decorators and additionally takes variants. Its scalar fallback (CAST(CAST(... AS VARCHAR) AS JSON)) is unchanged for types without a variant.
  • Validation:
    • just lint passed.
    • No-AWS SQLAlchemy tests: 384 passed.
    • Athena test_cast_as_decorated_types, test_cast_as_varchar, test_cast_as_binary, and test_array_element_variant_round_trip at 3a04a32: 4 passed.

Round two — claims: CLEAN.

  • The PR body now states the projection change and the SQLAlchemy 1.x results as measured in the isolated 1.4.54 run (master vs 3a04a32, same script).
  • The TEST section names 3a04a32, with counts matching the runs.
  • The earlier round-two note that decorated and Variant casts "still raise" on 1.x is superseded: decorated casts now work there.

@laughingman7743
laughingman7743 marked this pull request as ready for review September 26, 2026 07:48
@laughingman7743
laughingman7743 merged commit d6891e9 into master Sep 26, 2026
24 checks passed
@laughingman7743
laughingman7743 deleted the fix/829-typedecorator-cast branch September 26, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQLAlchemy: CAST to a TypeDecorator renders Hive DDL type names that Athena rejects

1 participant