Fall back to the original string for complex MAP/STRUCT/ARRAY values instead of returning None

[seunggabi]

Closes #764

Problem

When a complex-type column contains values that are too complex for the native-format parsers, _to_array / _to_map / _to_struct return None. To the caller this is indistinguishable from a NULL cell, so the data appears silently lost.

Real-world example: a MAP<VARCHAR, VARCHAR> column where one value holds a serialized JSON array:

{items=[{"product_id":285,"option_id":6049}], url=/webview/checkout, brand_id=75}

_to_map('{push=Y}')                      # {'push': 'Y'}  -> shown
_to_map('{items=[{...}], brand_id=75}')  # None           -> shown as NULL

In BI tools built on PyAthena (e.g. Superset), simple maps display fine while complex ones display as NULL — even though Athena returned the value. Users reasonably conclude the data pipeline dropped their data. (We spent a full debugging session tracing Kafka → Flink → Iceberg before finding the converter.)

Notably, the existing comment in _to_map already states the intent:

# But for complex structures, return None to keep as string

…but returning None does not keep the string.

Change

On the give-up paths (nested-structure skip and parse exceptions), return the original varchar value instead of None:

• _to_array: nested-array skip / parse exception → original string
• _to_map: complex-structure skip / parse exception → original string
• _to_struct: parse exception → original string
• parser.py: widened struct_parser type annotation accordingly

Behavior is unchanged for:

• None input → None
• values that don't look like the type at all → None
• all simple/parsable cases → parsed dict / list as before

CAST(col AS JSON) remains the recommended way to get reliably structured values; this change only ensures the fallback is lossless.

Tests

• Added tests for the raw-string fallback (map with nested array values incl. the real-world case above, native nested arrays)
• Added regression tests that simple values still parse ({push=Y})
• Updated 2 existing expectations in test_to_array_complex_nested_cases from None to the original string
• pytest tests/pyathena/test_parser.py tests/pyathena/test_converter.py: 130 passed
• ruff check / ruff format --check / mypy: clean

[laughingman7743]

Thanks for the fix — the direction matches the comment's original intent. One class of silent None still slips through here, though: values that are nested structs/maps (braces only, no []()) pass the "()[]" check above, and _parse_map_native then skips every pair whose value contains {}=" and returns None when nothing is left:

_to_map('{a={b=1}}')       # None — e.g. MAP<VARCHAR, ROW(...)> or MAP<VARCHAR, MAP(...)>
_to_map('{a={b=1, c=2}}')  # None

Since these are exactly the kind of complex values this PR is about, could you extend the fallback to cover the helper's give-up path as well? For example:

result = _parse_map_native(inner)
return result if result is not None else varchar_value

The same pattern applies to _to_array and _to_struct — see the other comments.

[seunggabi]

Good catch — fixed in f04125b. _to_map now falls back to the original string when _parse_map_native returns None, and added tests for {a={b=1}} / {a={b=1, c=2}}.

[laughingman7743]

Same remaining gap here: _parse_array_native returns None when every item is skipped by its safety check (items containing []="), and that None still propagates to the caller:

_to_array('[a=1, b=2]')  # None — items containing '=' are skipped

Suggest the same treatment as in _to_map:

result = _parse_array_native(inner)
return result if result is not None else varchar_value

[seunggabi]

Fixed in f04125b — same None → original-string fallback applied to _parse_array_native's result, with tests for [a=1, b=2].

[laughingman7743]

_parse_named_struct (the branch just above) can also return None when all pairs are skipped, which still propagates as a silent None:

_to_struct('{"a"=1}')  # None — the key containing a quote is skipped

Worth applying the same None → original-string fallback to its result as well (_parse_unnamed_struct always returns a dict, so only the named branch needs it).

[seunggabi]

Fixed in f04125b — _parse_named_struct's result now gets the same fallback (left _parse_unnamed_struct as-is since it always returns a dict), with a test for {"a"=1}.

[laughingman7743]

One more case in the same class of silent data loss (pre-existing, but I'd like it folded into this PR since it directly undermines the "never silently lose data" goal): _parse_map_native silently drops individual pairs whose key/value contains {}=", so a partially parseable map returns a partial dict — which is arguably worse than None was, because the loss is invisible:

_to_map('{a="x", b=1}')  # {'b': '1'} — the a="x" pair is silently dropped

_parse_array_native has the same issue for skipped items (_to_array('[a, b=1]') → ['a']).

A clean way to handle this together with the other comments: make the _parse_*_native helpers return None whenever they had to skip anything, and let the outer functions fall back to the original string. That way the result is always either fully parsed or the intact raw string, never a partial value.

[seunggabi]

Agreed that a partial dict/list is worse than None was — implemented exactly as you suggested in f04125b: _parse_map_native / _parse_array_native / _parse_named_struct now return None whenever any pair/item has to be skipped, and the outer functions fall back to the original string. The result is always either fully parsed or the intact raw string, never partial. Added tests for {a="x", b=1}, {a, b=1}, and [a, b=1] (previously {'b': '1'} / ['a']). All 137 unit tests pass, ruff/mypy clean.

[laughingman7743]

Thank you for the report and the fix, and for the quick follow-up addressing all the review comments — much appreciated!

Since fork PRs can't run the AWS-backed CI here, I ran the full test suites locally against this branch. Two integration tests in tests/pyathena/test_cursor.py (special_chars_struct / special_map) still asserted the old "dict or None" contract, so I pushed cd3bd74 to update them to accept the raw-string fallback — the new behavior is actually an improvement there (the old code partially parsed {msg=Hello, world, ...} and dropped data).

Final results on this branch:

• just test pyathena: 1293 passed, 11 skipped
• just test sqla: 234 passed, 152 skipped
• just test sqla-async: 234 passed, 152 skipped

Merging. Thanks again for the contribution!