Reflect the field and value types of top-level STRUCT and MAP columns

[laughingman7743]

WHAT

Column reflection now parses the types of top-level MAP and STRUCT/ROW columns.

• A top-level map<...> / map(...) column reflects as AthenaMap(key_type, value_type) instead of String.
• A top-level struct<...> / row(...) column reflects as AthenaStruct with its field names and types, instead of an AthenaStruct without fields.
• Both spellings are handled: Hive from the metadata API (map<int,int>, struct<a:int,b:int>) and Trino from the information_schema fallback (map(integer, integer), row(a integer, b integer)). The parser is the one already used for MAP and STRUCT/ROW types nested in an ARRAY. _get_column_type now re-enters it for a top-level type, so the change is a single branch.
• A top-level MAP or STRUCT/ROW type whose arguments cannot be parsed (for example struct<a>) warns "Did not recognize type" and reflects as NullType, as a top-level ARRAY already does. A nested type whose arguments cannot be parsed makes its enclosing ARRAY, MAP, or STRUCT NullType, as before.
• An unrecognized element, key, value, or field type name becomes NullType in place with a warning, as it already did inside an ARRAY (for example struct<a:foo> reflects as an AthenaStruct whose field a is NullType).
• Bare map, struct, and row without arguments keep their ischema_names types (String, AthenaStruct()); Athena reports complex types with arguments.
• AthenaTypeCompiler no longer overrides visit_null, so NullType at any depth in DDL (CREATE TABLE, TypeEngine.compile()) raises SQLAlchemy's CompileError ("Can't generate DDL for NullType(); ...") instead of emitting NULL, which Athena DDL rejects. The override was added in 19efdaa for an untyped foreign-key column in the compliance suite's BizarroCharacterTest.test_fk_ref, which the dialect's requirements now skip (foreign_key_constraint_reflection is unsupported), and it later served CAST, which uses AthenaDMLTypeCompiler since Split SQLAlchemy type rendering into Hive DDL and Trino DML compilers #961. CAST(x AS NULL) is unchanged.
• docs/sqlalchemy.md states the reflected types in the STRUCT and MAP sections, that an unrecognized key, value, or field type is reflected as NullType with a warning, that selecting such a column returns the cursor's value without conversion to the reflected types, and that NullType raises CompileError in DDL.

Behavior changes for the 4.0.0 release notes:

• Reflected top-level MAP columns are AthenaMap instead of String. python_type is dict instead of str, and String-specific behavior no longer applies: column + 'x' renders + instead of ||, and literal_binds cannot render a value for the column. like() and contains() render the same.
• Reflected top-level STRUCT/ROW columns carry their fields.
• CREATE TABLE compiled from a reflected table renders these columns as MAP<...> and STRUCT<...>. Before, a MAP column rendered as STRING, and a STRUCT column raised CompileError after Split SQLAlchemy type rendering into Hive DDL and Trino DML compilers #961 (it rendered ROW(), which Athena rejects, before that).
• A top-level MAP/STRUCT type string whose arguments cannot be parsed reflects as NullType with a warning, instead of String or an AthenaStruct without fields.
• NullType in DDL raises CompileError instead of rendering NULL. This covers a column declared without a type, ARRAY<NULL> from a reflected array of an unrecognized type, and an unrecognized reflected MAP key or value or STRUCT field. Compiling such a table's CREATE TABLE now fails at compile time instead of in Athena.

Unchanged, compared on 6f258501 with the old and new types: compiled SELECT, INSERT, and WHERE SQL, bind and result processing, and the values returned for these columns. Neither type has bind or result processors, and no JSON projection is added for top-level MAP/STRUCT columns.

WHY

Closes #886.
#961 made AthenaTypeCompiler render Hive DDL types and raise CompileError for an AthenaStruct without fields.
Reflection still produced that type for every top-level STRUCT column, and String for every top-level MAP column, so a reflected table could not be re-created with CREATE TABLE.
Reflection already parsed these types when they were nested in an ARRAY (#774); top-level columns were outside that change's ARRAY scope.

Measured on Athena engine version 3 for the one_row_complex test table:

Path	col_map	col_struct
GetTableMetadata	map<int,int>	struct<a:int,b:int>
information_schema.columns	map(integer, integer)	row(a integer, b integer)

Selecting both columns through the reflected table returned ({'1': '2', '3': '4'}, {'a': '1', 'b': '2'}) before the change, and test_reflect_select asserts the same values with the new types.

TEST

Tested commit: db6be39.

• just lint, just docs lint: passed.
• New offline tests in TestAthenaDialect:
  • test_top_level_map_and_struct_reflect_their_types covers both spellings, quoted field names, and a nested ARRAY of STRUCT, and checks that CREATE TABLE renders MAP<INT, STRING> and STRUCT<a:INT, `b c`:ARRAY<STRUCT<x:INT>>>.
  • test_unrecognized_top_level_map_or_struct_reflects_null_type covers five malformed types.
  • test_unrecognized_field_type_reflects_null_type_and_blocks_ddl reflects struct<a:int,b:timestamp(3) with time zone> and asserts the column-qualified CompileError.
  • test_columns_from_information_schema now asserts the STRUCT fields.
• New offline tests in TestAthenaTypeCompiler: test_null_type_is_rejected_in_ddl (top level and inside ARRAY/MAP/STRUCT), and test_null_type_cast_is_unchanged.
• With master's pyathena/ (6f25850), the reflection tests fail (8, checked on 4217bbe). The DDL NullType tests fail with the visit_null override restored (per the independent review; the 58-type comparison shows NULL before the change and CompileError after).
• Updated AWS tests: test_reflect_select asserts the reflected MAP/STRUCT types, the unchanged row values, and the CREATE TABLE rendering of the reflected one_row_complex table; test_get_column_type asserts the parsed types.
• uv run --env-file .env pytest -n 8 tests/pyathena/sqlalchemy/ tests/pyathena/aio/sqlalchemy/ on db6be39: 683 passed.
• uv run --env-file .env just test sqla on db6be39: 589 passed, 759 skipped (unchanged; the compliance test the override was added for is skipped).
• CI run 37112891716 passed test, test-sqla, and test-sqla-async on e06653d, before the visit_null change.
• Not run locally: just test sqla-async and the non-SQLAlchemy suites on db6be39; CI runs them once the PR is Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior, public contracts, simplicity, regression coverage)

Base 6f258501a56124f6e29c6d530ce429b702ed2ab3, head 4217bbe65b6203f42522c63646bb2b5b06c02a7b. Result: CLEAN.

Covered: every file in git diff 6f258501a56124f6e29c6d530ce429b702ed2ab3..4217bbe65b6203f42522c63646bb2b5b06c02a7b.

• Re-entry: a top-level map/row/struct with arguments calls _get_column_type(type_, _nested=True), which takes the existing nested MAP or STRUCT/ROW branch. Only the top-level call catches TypeError/ValueError.
• Nested errors keep their old reporting. A bad type inside an ARRAY still turns the whole ARRAY into NullType; a bad type inside a top-level STRUCT turns the whole STRUCT into NullType. An unknown scalar nested in a STRUCT still warns and becomes a NullType field, without raising, as before.
• Bare map/struct/row (no arguments) keep the ischema_names fallback.
• Both reflection paths: _columns_from_metadata and _columns_from_information_schema (whose varchar to string substitution applies only to the whole type) both reach _column → _get_column_type. The live type strings for both paths are in the PR body.
• Runtime effects, compared by a script for String() vs AthenaMap(Integer, Integer) and AthenaStruct() vs AthenaStruct(a, b): SELECT/INSERT/WHERE SQL, bind and result processors, and returned values are unchanged; python_type, literal rendering, +, and DDL differ, as listed in the PR body.
• Simplicity: one guarded branch, no code moved. An earlier draft extracted a helper for the parser body and was reverted, because the re-entry gives the same behavior with a smaller diff.
• Tests: the 8 new or extended offline cases fail with master's pyathena/, and the AWS assertions cover the live table and its CREATE TABLE rendering.

[laughingman7743]

Self-review round two (claims, callers, operations, evidence)

Base 6f258501a56124f6e29c6d530ce429b702ed2ab3, head 4217bbe65b6203f42522c63646bb2b5b06c02a7b. Result: FINDINGS (1, a PR body correction; no code change).

• Finding: the PR body said "String comparator operators no longer apply". Measured: column + 'x' renders m || ... for String but m + ... for AthenaMap, while contains() and like() render the same. The body now states exactly that.
• "Values unchanged": the live probe showed ({'1': '2', '3': '4'}, {'a': '1', 'b': '2'}) before the change, and test_reflect_select asserts the same row values with the new types (677 passed on AWS). The body now cites both.
• Docs: "SQLAlchemy does not convert it to the reflected key and value types" holds; AthenaMap and AthenaStruct have no result processor. Links to the duplicated "Data format support" headings were avoided, because Sphinx renders the second one as #id6.
• Callers: _get_column_type is called only from _column and from itself; the signature is unchanged.
• AWS operations: unchanged (parsing only; no additional requests).
• Evidence: just test sqla/sqla-async were not run locally; CI runs them on Ready.

[laughingman7743]

Independent review (relayed Codex result)

• Reviewer: Codex CLI 0.160.0, model gpt-6-astra, session 01a10100-05d3-7690-844b-e82cb56d13db.
• Invocation: codex exec --sandbox read-only on a detached snapshot of 4217bbe65b6203f42522c63646bb2b5b06c02a7b (base 6f258501a56124f6e29c6d530ce429b702ed2ab3). The prompt gave the diff range and measured type-string facts, and omitted the PR number, description, commits, and self-review findings.
• Snapshot: tracked files are unchanged. An ignored .serena/ directory appeared; the read-only sandbox cannot write, and the same artifact appeared in the Split SQLAlchemy type rendering into Hive DDL and Trino DML compilers #961 snapshots, so it comes from local tooling rather than the reviewer.
• Coverage, as reported: parsing (both spellings, quoting, nesting, whitespace, bare and malformed types, exception handling); the metadata/information_schema paths, _column, get_columns, has_table, and async; types, comparators, binds/results, DDL/CAST, and cache keys; tests and docs.
• Result: FINDINGS (4). Static review.

Dispositions (each reproduced with a script on 4217bbe65b6203f42522c63646bb2b5b06c02a7b):

1. P2, map<int,> → AthenaMap(INTEGER, NullType) and struct<a:map<>> → struct with a NullType field: accepted as a documentation finding, no code change. This is the existing nested-parser contract: an unrecognized leaf becomes NullType in place with a warning. ARRAY behaves the same (array<foo> → AthenaArray(NullType), array<struct<a:foo>> → struct with a NullType field). Keeping recognized fields is deliberate. The docstring (543d681f4c89376840f00ea349fe4798818b0917) and the PR body overstated whole-column fallback and now describe both cases.
2. P2, bare map/struct/row: pre-existing, not changed. These are not parse failures, and Athena reports complex types with arguments. The PR body now states that bare types keep their ischema_names types.
3. P3, mismatched outer delimiters (map<int,int)): pre-existing, not changed. _pattern_column_type already accepted array<int) with no warning. Athena does not emit such strings, and tightening the shared regex is outside this PR.
4. P3, RecursionError at about 1,100 nesting levels: not changed. Recursive ARRAY parsing already had this limit, and Athena/Hive types do not nest that deep. Catching RecursionError only for top-level MAP/STRUCT would diverge from ARRAY.

Repair 543d681f4c89376840f00ea349fe4798818b0917 changes only the _get_column_type docstring; lint passed.

[laughingman7743]

Independent follow-ups on the docstring repair (relayed Codex results; Codex CLI 0.160.0, gpt-6-astra, codex exec --sandbox read-only, snapshot verified unchanged after each run)

1. Session 01a10108-5f42-76d2-82f5-6c2ef3c74763, diff 4217bbe6..543d681f: FINDINGS. The Raises section omitted scalar argument errors: a top-level varchar(nope) raises ValueError, and decimal(1,2,3,4,5) raises TypeError. Reproduced; this is pre-existing behavior. Documented in 035a3b9e.
2. Session 01a1010c-6636-74e3-b3ed-9a4372ae9194, diff 543d681f..035a3b9e: FINDINGS. The "whole top-level type becomes NullType" wording was wrong when a nested ARRAY handles the error first: map<int,array<varchar(nope)>> → AthenaMap(INTEGER, NullType), and array<array<map<int>>> → AthenaArray(NullType). Reproduced with 11 probe cases. e06653de rewrites Returns/Raises around the single rule: the innermost enclosing ARRAY handles a parse error, or else a top-level MAP or STRUCT/ROW, and otherwise it is raised.
3. Session 01a1010f-f5c6-7641-a0d2-a42ef07923cf, diff 035a3b9e..e06653de plus the complete docstring: CLEAN.

No code behavior changed in 543d681f, 035a3b9e, or e06653de (docstring only); lint passed on each.

Correction to the review prompts: the prompts for runs 1 and 2 called keeping the in-place NullType leaf behavior a "maintainer-side decision". It was the author's decision, recorded in the disposition above, and is open to the maintainer's review.

[laughingman7743]

Self-review of the docstring repairs (4217bbe6..e06653de): behavior CLEAN (no code statements changed; git diff 4217bbe6..e06653de touches only the _get_column_type docstring). Claims CLEAN: every Returns/Raises example was checked against a probe run on e06653de (varchar(x) raises; array<varchar(x)>, struct<a:varchar(x)>, and struct<a:map<int>> are NullType; map<int,array<varchar(x)>> keeps the MAP; struct<a:foo> keeps the field as NullType; a nested map<int> raises). The PR body states that the later commits change only the docstring.

[laughingman7743]

Design consultation (relayed Claude Fable 5.1 opinion, requested by the maintainer)

• claude -p, model claude-fable-5-1, effort high, profile max (first-party Max), session 17845510-8bb0-4015-a88b-a0d3617e0de5. Run on a clean detached snapshot of e06653de, read-only (MCP and hooks disabled; uv/python/pip denied). The snapshot was verified unchanged afterwards.
• The question was whether (1) an unrecognized MAP key/value or STRUCT field type should stay NullType in place, and (2) AthenaTypeCompiler.visit_null should be removed so that DDL raises CompileError.

Opinion:

1. Keep in-place NullType. It matches ARRAY elements and top-level scalars, and it keeps the known fields. A String fallback would let CREATE TABLE succeed with a wrong schema. The PostgreSQL dialect makes a whole array NULLTYPE, but it has no field names to keep.
2. Remove visit_null. It was added in 19efdaa for BizarroCharacterTest.test_fk_ref (an untyped FK column), which requirements.py now skips through the unsupported foreign_key_constraint_reflection. No code in pyathena/ or tests/ relies on DDL NULL; pyathena/pandas/util.py to_sql uses its own type mapping; and no built-in SQLAlchemy dialect overrides visit_null.
3. Alternatives (an Athena-specific message, or a NullType subclass keeping the raw type string) were rejected as not worth the complexity.

Open points it raised, with their status:

• Release note: done in the PR body.
• Docs: the reflected NullType leaves and the DDL CompileError are now documented.
• Alembic autogenerate with a reflected NullType: not verified, because Alembic is not a dependency.
• ischema_names["map"] now applies only to a bare map without arguments: unchanged, as behavior for bare types is out of scope.
• DML CAST(x AS NULL): unchanged, out of scope.

Author verification before implementing: the 19efdaa commit message (the other.ref NullType error) and requirements.py (foreign_key_ddl and foreign_key_constraint_reflection unsupported) were checked. The implementation is db6be39f06803b1fd0e851f20598eefd2512ed61.

[laughingman7743]

Self-review of e06653de..db6be39f (removing DDL visit_null, plus docs and tests)

Round-one perspective: CLEAN.

• AthenaTypeCompiler is reached with a NullType only through DDL (get_column_specification, compiler.py:1474) and TypeEngine.compile(). The statement compiler renders CAST through AthenaDMLTypeCompiler, whose visit_null remains. test_null_type_cast_is_unchanged asserts CAST(x AS NULL).
• The 58-type comparison differs from the previous head only in the two DDL NullType lines, which now raise.
• New tests: test_null_type_is_rejected_in_ddl (top level and in ARRAY/MAP/STRUCT) and test_unrecognized_field_type_reflects_null_type_and_blocks_ddl, which reflects struct<a:int,b:timestamp(3) with time zone> and asserts the column-qualified CompileError.

Round-two perspective: CLEAN.

• The docs sentences and the class docstring match the tests.
• The PR body lists the DDL NullType behavior change, including columns declared without a type and ARRAY<NULL>.
• The claim that the compliance suite skips test_fk_ref is confirmed by just test sqla on db6be39f: 589 passed, 759 skipped, the same as before.
• AWS tests/pyathena/sqlalchemy/ + aio on db6be39f: 683 passed.

[laughingman7743]

Independent review of e06653de..db6be39f (relayed Codex result): Codex CLI 0.160.0, gpt-6-astra, session 01a10131-ae3b-7243-b086-2e47be732d74. Run with codex exec --sandbox read-only on a detached snapshot of db6be39f, with the whole branch diff as context. Because this change widens the DDL contract, the review covered its full scope. Static review; the snapshot was verified unchanged.

Result: CLEAN. Covered:

• every dialect (sync, aio, pandas, Arrow, Polars, S3FS) sharing type_compiler_cls
• DDL, TypeEngine.compile(), variants and decorators, nested ARRAY/MAP/STRUCT, and both reflection paths
• SELECT/DML, binds, CAST, projections, indexing, slicing, and partial updates, including _ArrayAssignmentType
• the installed SQLAlchemy 2.0.46 compliance suite: test_fk_ref is skipped by requirements.py:76, the CTE fixture supplies Integer, and the normalized-name and cross-schema FK fixtures are gated off
• the tests (they fail with the override restored) and the docs/docstring