From 8a75aeaf86241a0b0606bc5c5da3817098321433 Mon Sep 17 00:00:00 2001 From: seunggabi Date: Tue, 1 Sep 2026 17:56:08 +0900 Subject: [PATCH 1/3] (#764) Fall back to the original string for complex MAP/STRUCT/ARRAY values instead of returning None The native-format parsers for complex types give up when a value contains nested structures (e.g. MAP whose value holds a JSON array), returning None. To callers this is indistinguishable from a NULL cell, so the data appears silently lost - e.g. Superset over PyAthena shows NULL for such columns even though Athena returned the value. The existing comment in _to_map already stated the intent ('return None to keep as string'), but returning None does not keep the string. This change makes the give-up paths of _to_array/_to_map/_to_struct return the original varchar value, preserving the data while keeping simple cases parsed as before. CAST(col AS JSON) remains the recommended way to get structured values. Fixes #764 --- pyathena/converter.py | 35 ++++++++++++++++---------- pyathena/parser.py | 2 +- tests/pyathena/test_converter.py | 42 +++++++++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/pyathena/converter.py b/pyathena/converter.py index 7432d5b9..36677407 100644 --- a/pyathena/converter.py +++ b/pyathena/converter.py @@ -89,7 +89,7 @@ def _to_json(varchar_value: str | None) -> Any | None: return json.loads(varchar_value) -def _to_array(varchar_value: str | None) -> list[Any] | None: +def _to_array(varchar_value: str | None) -> list[Any] | str | None: """Convert array data to Python list. Supports two formats: @@ -102,7 +102,9 @@ def _to_array(varchar_value: str | None) -> list[Any] | None: varchar_value: String representation of array data Returns: - List representation of array, or None if parsing fails + List representation of array, the original string if the value is + too complex to parse (so the data is preserved), or None if the + input is None or does not look like an array """ if varchar_value is None: return None @@ -127,15 +129,16 @@ def _to_array(varchar_value: str | None) -> list[Any] | None: try: # For nested arrays, too complex for basic parsing if "[" in inner: - # Contains nested arrays - too complex for basic parsing - return None + # Contains nested arrays - keep the original string + # instead of silently dropping the value + return varchar_value # Try native parsing (including struct arrays) return _parse_array_native(inner) except Exception: - return None + return varchar_value -def _to_map(varchar_value: str | None) -> dict[str, Any] | None: +def _to_map(varchar_value: str | None) -> dict[str, Any] | str | None: """Convert map data to Python dictionary. Supports two formats: @@ -148,7 +151,9 @@ def _to_map(varchar_value: str | None) -> dict[str, Any] | None: varchar_value: String representation of map data Returns: - Dictionary representation of map, or None if parsing fails + Dictionary representation of map, the original string if the value + is too complex to parse (so the data is preserved), or None if the + input is None or does not look like a map """ if varchar_value is None: return None @@ -177,16 +182,17 @@ def _to_map(varchar_value: str | None) -> dict[str, Any] | None: try: # MAP format is always key=value pairs - # But for complex structures, return None to keep as string + # But for complex structures, keep the original string if any(char in inner for char in "()[]"): # Contains complex structures (arrays, structs), skip parsing - return None + # and keep the original string instead of silently dropping it + return varchar_value return _parse_map_native(inner) except Exception: - return None + return varchar_value -def _to_struct(varchar_value: str | None) -> dict[str, Any] | None: +def _to_struct(varchar_value: str | None) -> dict[str, Any] | str | None: """Convert struct data to Python dictionary. Supports two formats: @@ -199,7 +205,9 @@ def _to_struct(varchar_value: str | None) -> dict[str, Any] | None: varchar_value: String representation of struct data Returns: - Dictionary representation of struct, or None if parsing fails + Dictionary representation of struct, the original string if the + value is too complex to parse (so the data is preserved), or None + if the input is None or does not look like a struct """ if varchar_value is None: return None @@ -233,7 +241,8 @@ def _to_struct(varchar_value: str | None) -> dict[str, Any] | None: # Unnamed struct: {Alice, 25} return _parse_unnamed_struct(inner) except Exception: - return None + # Keep the original string instead of silently dropping the value + return varchar_value def _parse_array_native(inner: str) -> list[Any] | None: diff --git a/pyathena/parser.py b/pyathena/parser.py index e263ed53..5d3635cf 100644 --- a/pyathena/parser.py +++ b/pyathena/parser.py @@ -229,7 +229,7 @@ def __init__( self, converters: dict[str, Callable[[str | None], Any | None]], default_converter: Callable[[str | None], Any | None], - struct_parser: Callable[[str | None], dict[str, Any] | None], + struct_parser: Callable[[str | None], dict[str, Any] | str | None], ) -> None: self._converters = converters self._default_converter = default_converter diff --git a/tests/pyathena/test_converter.py b/tests/pyathena/test_converter.py index 1efeb8da..ea2a35c2 100644 --- a/tests/pyathena/test_converter.py +++ b/tests/pyathena/test_converter.py @@ -96,7 +96,9 @@ def test_to_struct_athena_nested_formats(input_value, expected): ) def test_to_struct_athena_complex_cases(input_value): result = _to_struct(input_value) - assert result is None or isinstance(result, dict) + # Values too complex to parse fall back to the original string + # instead of being silently dropped + assert result == input_value or isinstance(result, dict) @pytest.mark.parametrize( @@ -115,6 +117,38 @@ def test_to_map_athena_numeric_keys(): assert _to_map("{1=2, 3=4}") == {"1": "2", "3": "4"} +@pytest.mark.parametrize( + "input_value", + [ + # MAP whose values contain nested structures: + # too complex to parse reliably, so the original string is kept + # instead of returning None (which looked like silent data loss) + "{items=[{product_id=285, option_id=6049, amount=1, price=12000}], brand_id=75}", + '{items=[{"product_id":285,"option_id":6049}], brand_id=75}', + "{callback=fn(x), retries=3}", + ], +) +def test_to_map_complex_values_keep_original_string(input_value): + assert _to_map(input_value) == input_value + + +def test_to_map_simple_values_still_parse(): + assert _to_map("{push=Y}") == {"push": "Y"} + assert _to_map("{url=/webview/checkout, brand_id=75}") == { + "url": "/webview/checkout", + "brand_id": "75", + } + + +def test_to_array_nested_values_keep_original_string(): + # Nested arrays in native format are too complex to parse reliably, + # so the original string is kept instead of returning None + value = "[[1, 2], [3, 4]]" + assert _to_array(value) == [[1, 2], [3, 4]] # valid JSON parses first + native_value = "[{a=[1, 2]}, {b=[3]}]" + assert _to_array(native_value) == native_value + + @pytest.mark.parametrize( ("input_value", "expected"), [ @@ -184,9 +218,11 @@ def test_to_array_athena_nested_struct_elements(input_value, expected): @pytest.mark.parametrize( ("input_value", "expected"), [ - ("[ARRAY[1, 2], ARRAY[3, 4]]", None), + # Too complex for native parsing: the original string is kept + # instead of returning None (which looked like silent data loss) + ("[ARRAY[1, 2], ARRAY[3, 4]]", "[ARRAY[1, 2], ARRAY[3, 4]]"), ("[[1, 2], [3, 4]]", [[1, 2], [3, 4]]), - ("[MAP(ARRAY['key'], ARRAY['value'])]", None), + ("[MAP(ARRAY['key'], ARRAY['value'])]", "[MAP(ARRAY['key'], ARRAY['value'])]"), ], ) def test_to_array_complex_nested_cases(input_value, expected): From f04125b9ba75cb1a5073ce5496891922aec053d9 Mon Sep 17 00:00:00 2001 From: seunggabi Date: Wed, 2 Sep 2026 09:27:24 +0900 Subject: [PATCH 2/3] (#764) Address review: extend raw-string fallback to helper give-up paths and forbid partial results Per review feedback on #763: - _to_map/_to_array/_to_struct now fall back to the original string when the _parse_*_native helpers return None (e.g. nested braces-only values like '{a={b=1}}' that pass the '()[]' pre-check, or '[a=1, b=2]' where every item is skipped, or '{"a"=1}' where the quoted key is skipped). - _parse_map_native/_parse_array_native/_parse_named_struct now return None whenever any pair/item has to be skipped, instead of silently dropping it and returning a partial dict/list. Combined with the outer fallback, the result is always either fully parsed or the intact raw string - never a partial value. --- pyathena/converter.py | 68 +++++++++++++++++++++++--------- tests/pyathena/test_converter.py | 42 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/pyathena/converter.py b/pyathena/converter.py index 36677407..8ae5cfdf 100644 --- a/pyathena/converter.py +++ b/pyathena/converter.py @@ -132,8 +132,10 @@ def _to_array(varchar_value: str | None) -> list[Any] | str | None: # Contains nested arrays - keep the original string # instead of silently dropping the value return varchar_value - # Try native parsing (including struct arrays) - return _parse_array_native(inner) + # Try native parsing (including struct arrays); if the helper had to + # give up (or skip any item), keep the original string + result = _parse_array_native(inner) + return result if result is not None else varchar_value except Exception: return varchar_value @@ -187,7 +189,10 @@ def _to_map(varchar_value: str | None) -> dict[str, Any] | str | None: # Contains complex structures (arrays, structs), skip parsing # and keep the original string instead of silently dropping it return varchar_value - return _parse_map_native(inner) + # If the helper had to give up (or skip any pair, e.g. nested + # braces-only values like '{a={b=1}}'), keep the original string + result = _parse_map_native(inner) + return result if result is not None else varchar_value except Exception: return varchar_value @@ -236,8 +241,10 @@ def _to_struct(varchar_value: str | None) -> dict[str, Any] | str | None: try: if "=" in inner: - # Named struct: {a=1, b=2} - return _parse_named_struct(inner) + # Named struct: {a=1, b=2}; if the helper had to give up + # (or skip any pair), keep the original string + result = _parse_named_struct(inner) + return result if result is not None else varchar_value # Unnamed struct: {Alice, 25} return _parse_unnamed_struct(inner) except Exception: @@ -252,7 +259,10 @@ def _parse_array_native(inner: str) -> list[Any] | None: inner: Interior content of array without brackets. Returns: - List with parsed values, or None if no valid values found. + List with parsed values, or None if no valid values were found or + any item had to be skipped. Returning None on a skipped item lets + callers fall back to the original string, so the result is always + either fully parsed or the intact raw value - never a partial list. """ result = [] @@ -267,13 +277,15 @@ def _parse_array_native(inner: str) -> list[Any] | None: if item.strip().startswith("{") and item.strip().endswith("}"): # This is a struct value - parse it as a struct struct_value = _to_struct(item.strip()) - if struct_value is not None: - result.append(struct_value) + if struct_value is None: + return None + result.append(struct_value) continue - # Skip items with nested arrays or complex quoting (safety check) + # Items with nested arrays or complex quoting (safety check): + # give up entirely instead of silently dropping the item if any(char in item for char in '[]="'): - continue + return None # Convert item to appropriate type converted_item = _convert_value(item) @@ -289,7 +301,11 @@ def _parse_map_native(inner: str) -> dict[str, Any] | None: inner: Interior content of map without braces. Returns: - Dictionary with parsed key-value pairs, or None if no valid pairs found. + Dictionary with parsed key-value pairs, or None if no valid pairs + were found or any pair had to be skipped. Returning None on a + skipped pair lets callers fall back to the original string, so the + result is always either fully parsed or the intact raw value - + never a partial dict. """ result = {} @@ -297,16 +313,22 @@ def _parse_map_native(inner: str) -> dict[str, Any] | None: pairs = [pair.strip() for pair in inner.split(",")] for pair in pairs: - if "=" not in pair: + if not pair: continue + # A chunk without '=' is not a parseable pair: give up entirely + # instead of silently dropping it + if "=" not in pair: + return None + key, value = pair.split("=", 1) key = key.strip() value = value.strip() - # Skip pairs with special characters (safety check) + # Pairs with special characters (safety check): give up entirely + # instead of silently dropping the pair if any(char in key for char in '{}="') or any(char in value for char in '{}="'): - continue + return None # Convert both key and value to appropriate types converted_key = _convert_value(key) @@ -326,7 +348,11 @@ def _parse_named_struct(inner: str) -> dict[str, Any] | None: inner: Interior content of struct without braces. Returns: - Dictionary with parsed key-value pairs, or None if no valid pairs found. + Dictionary with parsed key-value pairs, or None if no valid pairs + were found or any pair had to be skipped. Returning None on a + skipped pair lets callers fall back to the original string, so the + result is always either fully parsed or the intact raw value - + never a partial dict. """ result = {} @@ -334,16 +360,22 @@ def _parse_named_struct(inner: str) -> dict[str, Any] | None: pairs = _split_array_items(inner) for pair in pairs: - if "=" not in pair: + if not pair: continue + # A chunk without '=' is not a parseable pair: give up entirely + # instead of silently dropping it + if "=" not in pair: + return None + key, value = pair.split("=", 1) key = key.strip() value = value.strip() - # Skip if key contains special characters (safety check) + # Keys with special characters (safety check): give up entirely + # instead of silently dropping the pair if any(char in key for char in '{}="'): - continue + return None # Handle nested struct values if value.startswith("{") and value.endswith("}"): diff --git a/tests/pyathena/test_converter.py b/tests/pyathena/test_converter.py index ea2a35c2..667d9509 100644 --- a/tests/pyathena/test_converter.py +++ b/tests/pyathena/test_converter.py @@ -140,6 +140,48 @@ def test_to_map_simple_values_still_parse(): } +@pytest.mark.parametrize( + "input_value", + [ + # Nested braces-only values (e.g. MAP) pass the + # "()[]" pre-check but every pair is skipped by _parse_map_native + "{a={b=1}}", + "{a={b=1, c=2}}", + ], +) +def test_to_map_nested_brace_values_keep_original_string(input_value): + assert _to_map(input_value) == input_value + + +@pytest.mark.parametrize( + "input_value", + [ + # Partially parseable values must not return a partial result: + # either fully parsed or the intact original string + '{a="x", b=1}', + "{a, b=1}", + ], +) +def test_to_map_never_returns_partial_dict(input_value): + assert _to_map(input_value) == input_value + + +@pytest.mark.parametrize( + "input_value", + [ + "[a=1, b=2]", # every item skipped by the '=' safety check + "[a, b=1]", # partially parseable: must not return ['a'] + ], +) +def test_to_array_never_returns_partial_list(input_value): + assert _to_array(input_value) == input_value + + +def test_to_struct_skipped_pairs_keep_original_string(): + # The quoted key is skipped by _parse_named_struct's safety check + assert _to_struct('{"a"=1}') == '{"a"=1}' + + def test_to_array_nested_values_keep_original_string(): # Nested arrays in native format are too complex to parse reliably, # so the original string is kept instead of returning None From cd3bd7441d5d5e814c560ec3698c0f872caad8e6 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Wed, 2 Sep 2026 21:58:40 +0900 Subject: [PATCH 3/3] (#764) Update complex-type cursor tests for the raw-string fallback contract The integration tests in TestComplexDataTypes asserted the old contract (conversion returns a dict or None). With the raw-string fallback, values too complex to parse losslessly now return the original string instead of None, so accept that as a valid conversion result. Co-Authored-By: Claude Fable 5 --- tests/pyathena/test_cursor.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/pyathena/test_cursor.py b/tests/pyathena/test_cursor.py index 2e3c2be3..5e49f6af 100644 --- a/tests/pyathena/test_cursor.py +++ b/tests/pyathena/test_cursor.py @@ -1313,10 +1313,11 @@ def test_struct_types(self, cursor, query, description): if isinstance(struct_value, str): converted = _to_struct(struct_value) _logger.info("%s: Converted %r -> %r", description, struct_value, converted) - # For string structs, conversion should succeed or return None for complex cases + # Conversion either fully parses to a dict or falls back to the + # original string for values too complex to parse losslessly if converted is not None: - assert isinstance(converted, dict), ( - f"Converted struct should be dict for {description}" + assert isinstance(converted, dict) or converted == struct_value, ( + f"Converted struct should be dict or the original string for {description}" ) elif isinstance(struct_value, dict): # Already converted by the cursor converter @@ -1409,9 +1410,11 @@ def test_map_types(self, cursor, query, description): # Simple MAP, try conversion converted = _to_map(map_value) _logger.info("%s: Converted %r -> %r", description, map_value, converted) + # Conversion either fully parses to a dict or falls back to the + # original string for values too complex to parse losslessly if converted is not None: - assert isinstance(converted, dict), ( - f"Converted map should be dict for {description}" + assert isinstance(converted, dict) or converted == map_value, ( + f"Converted map should be dict or the original string for {description}" ) elif isinstance(map_value, dict): # Already converted by the cursor converter