Skip to content

Commit a540a2d

Browse files
SNOW-3215485: Fix colon escape in XML custom schema (#4114)
1 parent f51382c commit a540a2d

6 files changed

Lines changed: 176 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Fixed a bug in `Session.client_telemetry` that trace does not have snowflake style trace id.
1212
- Fixed a bug when saving a fdn table into an iceberg table in overwrite mode, error is raised because `StringType` is saved in wrong length.
1313
- Fixed a bug in `ai_complete` where `model_parameters` and `response_format` values containing single quotes would generate malformed SQL.
14+
- Fixed a bug in `DataFrameReader.xml()` where reading XML with a custom schema whose field names contain colons (e.g., `px:name`) raised a `SnowparkColumnException`.
1415

1516
#### Improvements
1617

src/snowflake/snowpark/_internal/type_utils.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,15 +1250,21 @@ def find_top_level_colon(field_def: str) -> int:
12501250
"""
12511251
Returns the index of the first top-level colon in 'field_def',
12521252
or -1 if there is no top-level colon. A colon is considered top-level
1253-
if it is not enclosed in <...> or (...).
1253+
if it is not enclosed in <...>, (...), or "...".
12541254
12551255
Example:
1256-
'a struct<i: integer>' => returns -1 (colon is nested).
1256+
'a struct<i: integer>' => returns -1 (colon is nested).
12571257
'x: struct<i: integer>' => returns index of the colon after 'x'.
1258+
'"px:name": string' => returns index of the colon after the closing '"'.
12581259
"""
12591260
bracket_depth = 0
1261+
in_quotes = False
12601262
for i, ch in enumerate(field_def):
1261-
if ch in ("<", "("):
1263+
if ch == '"':
1264+
in_quotes = not in_quotes
1265+
elif in_quotes:
1266+
continue
1267+
elif ch in ("<", "("):
12621268
bracket_depth += 1
12631269
elif ch in (">", ")"):
12641270
bracket_depth -= 1

src/snowflake/snowpark/_internal/xml_reader.py

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,58 @@ def replace_entity(match: re.Match) -> str:
5757
return match.group(0)
5858

5959

60+
# TODO SNOW-3217320: The escape/restore below works around a bug in
61+
# type_utils.find_top_level_colon which mis-splits colons inside double-quoted
62+
# identifiers. The fix for find_top_level_colon will ship in this same release
63+
# (type_utils.py), but the UDTF resolves snowflake-snowpark-python from
64+
# Snowflake's Anaconda channel at runtime, so the server may still run an older
65+
# version. Once the Anaconda-channel version includes the fix, this
66+
# escape/restore logic becomes a harmless no-op and can be removed.
67+
_COLON_PLACEHOLDER = "\x00COLON\x00"
68+
69+
70+
def _escape_colons_in_quotes(schema_str: str) -> str:
71+
"""Replace colons inside double-quoted identifiers with a placeholder.
72+
73+
The server-side type_utils.find_top_level_colon does not skip colons inside
74+
double-quoted identifiers, so a field like ``"px:name": string`` gets split at
75+
the wrong colon. By replacing those colons with a placeholder, we let the existing
76+
parser find the correct top-level colon that separates the field name from its type.
77+
"""
78+
result = []
79+
in_quotes = False
80+
for ch in schema_str:
81+
if ch == '"':
82+
in_quotes = not in_quotes
83+
result.append(ch)
84+
elif ch == ":" and in_quotes:
85+
result.append(_COLON_PLACEHOLDER)
86+
else:
87+
result.append(ch)
88+
return "".join(result)
89+
90+
91+
def _restore_colons_in_template(template: Optional[dict]) -> Optional[dict]:
92+
"""Recursively restore colon placeholders back to real colons in template keys."""
93+
if template is None:
94+
return None
95+
restored: Dict[str, Any] = {}
96+
for key, value in template.items():
97+
real_key = key.replace(_COLON_PLACEHOLDER, ":")
98+
if isinstance(value, dict):
99+
restored[real_key] = _restore_colons_in_template(value)
100+
else:
101+
restored[real_key] = value
102+
return restored
103+
104+
60105
def schema_string_to_result_dict_and_struct_type(schema_string: str) -> Optional[dict]:
61106
if schema_string == "":
62107
return None
63-
schema = type_string_to_type_object(schema_string)
64-
65-
return struct_type_to_result_template(schema)
108+
safe_string = _escape_colons_in_quotes(schema_string)
109+
schema = type_string_to_type_object(safe_string)
110+
template = struct_type_to_result_template(schema)
111+
return _restore_colons_in_template(template)
66112

67113

68114
def struct_type_to_result_template(dt: DataType) -> Optional[dict]:

tests/integ/test_xml_reader_row_tag.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,28 @@ def test_user_schema_without_rowtag(session):
693693
session.read.schema(user_schema).xml(f"@{tmp_stage_name}/{test_file_books_xml}")
694694

695695

696+
@pytest.mark.parametrize("ignore_namespace", [True, False])
697+
def test_read_xml_custom_schema_with_colon_tags(session, ignore_namespace):
698+
schema = StructType(
699+
[
700+
StructField("px:name", StringType(), True),
701+
StructField("px:value", StringType(), True),
702+
]
703+
)
704+
df = (
705+
session.read.option("rowTag", "px:item")
706+
.schema(schema)
707+
.option("ignoreNamespace", ignore_namespace)
708+
.xml(f"@{tmp_stage_name}/{test_file_xml_undeclared_namespace}")
709+
)
710+
result = df.collect()
711+
assert len(result) == 2
712+
names = {r[0] for r in result}
713+
values = {r[1] for r in result}
714+
assert names == {"Item One", "Item Two"}
715+
assert values == {"100", "200"}
716+
717+
696718
def test_value_tag_custom_schema(session):
697719
test_schema1 = StructType(
698720
[

tests/unit/test_types.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
snow_type_to_dtype_str,
5252
split_top_level_comma_fields,
5353
type_string_to_type_object,
54+
find_top_level_colon,
5455
)
5556
from snowflake.snowpark.types import (
5657
ArrayType,
@@ -2539,3 +2540,12 @@ def process(self, input_a, input_b):
25392540
"", "process", class_name="MyOtherClass", _source=source
25402541
)
25412542
assert arg_names is None
2543+
2544+
2545+
def test_find_top_level_colon():
2546+
assert find_top_level_colon("x: int") == 1
2547+
assert find_top_level_colon("a struct<i: integer>") == -1
2548+
assert find_top_level_colon("x: struct<i: integer>") == 1
2549+
assert find_top_level_colon('"px:name": string') == 9
2550+
assert find_top_level_colon('"a:b:c": string') == 7
2551+
assert find_top_level_colon("plain_field") == -1

tests/unit/test_xml_reader.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
DEFAULT_CHUNK_SIZE,
2626
struct_type_to_result_template,
2727
schema_string_to_result_dict_and_struct_type,
28+
_escape_colons_in_quotes,
29+
_restore_colons_in_template,
30+
_COLON_PLACEHOLDER,
2831
)
2932
from snowflake.snowpark.types import (
3033
StructType,
@@ -991,3 +994,85 @@ def test_user_schema_value_tag():
991994
res = element_to_dict_or_str(element, result_template=result_template)
992995
assert result_template == {"NUM": None, "STR1": None, "STR2": None, "STR3": None}
993996
assert res == {"NUM": "1", "STR1": "NULL", "STR2": None, "STR3": "xxx"}
997+
998+
999+
def test_escape_colons_in_quotes():
1000+
s = '"px:name": string'
1001+
escaped = _escape_colons_in_quotes(s)
1002+
assert escaped.count(":") == 1
1003+
assert _COLON_PLACEHOLDER in escaped
1004+
1005+
s = '"px:name": string, "px:value": string'
1006+
escaped = _escape_colons_in_quotes(s)
1007+
assert escaped.count(":") == 2
1008+
assert escaped.count(_COLON_PLACEHOLDER) == 2
1009+
1010+
s = 'struct<"px:name": string, "detail": struct<"px:sub": string>>'
1011+
escaped = _escape_colons_in_quotes(s)
1012+
assert escaped.count(_COLON_PLACEHOLDER) == 2
1013+
assert escaped.count(":") == 3
1014+
1015+
s = '"Author": string, "Title": string'
1016+
assert _escape_colons_in_quotes(s) == s
1017+
assert _escape_colons_in_quotes("") == ""
1018+
assert _escape_colons_in_quotes("a: int, b: string") == "a: int, b: string"
1019+
1020+
1021+
def test_restore_colons_in_template():
1022+
assert _restore_colons_in_template(None) is None
1023+
assert _restore_colons_in_template({"Author": None}) == {"Author": None}
1024+
1025+
# Flat
1026+
template = {
1027+
f"px{_COLON_PLACEHOLDER}name": None,
1028+
f"px{_COLON_PLACEHOLDER}value": None,
1029+
}
1030+
assert _restore_colons_in_template(template) == {
1031+
"px:name": None,
1032+
"px:value": None,
1033+
}
1034+
1035+
# Nested
1036+
template = {
1037+
f"eq{_COLON_PLACEHOLDER}event": {
1038+
f"eq{_COLON_PLACEHOLDER}sub-id": None,
1039+
},
1040+
"plain": None,
1041+
}
1042+
assert _restore_colons_in_template(template) == {
1043+
"eq:event": {"eq:sub-id": None},
1044+
"plain": None,
1045+
}
1046+
1047+
1048+
def test_schema_string_round_trip_with_colons():
1049+
# Flat
1050+
schema_str = 'struct<"px:name": string, "px:value": string>'
1051+
assert schema_string_to_result_dict_and_struct_type(schema_str) == {
1052+
"px:name": None,
1053+
"px:value": None,
1054+
}
1055+
1056+
# Nested
1057+
schema_str = (
1058+
'struct<"eq:event-id": string,' '"eq:detail": struct<"eq:sub-id": string>>'
1059+
)
1060+
assert schema_string_to_result_dict_and_struct_type(schema_str) == {
1061+
"eq:event-id": None,
1062+
"eq:detail": {"eq:sub-id": None},
1063+
}
1064+
1065+
# Mixed
1066+
schema_str = 'struct<"px:name": string, "Title": string, price: double>'
1067+
assert schema_string_to_result_dict_and_struct_type(schema_str) == {
1068+
"px:name": None,
1069+
"Title": None,
1070+
"PRICE": None,
1071+
}
1072+
1073+
# No colon fields
1074+
schema_str = 'struct<"Author": string, "TITLE": string>'
1075+
assert schema_string_to_result_dict_and_struct_type(schema_str) == {
1076+
"Author": None,
1077+
"TITLE": None,
1078+
}

0 commit comments

Comments
 (0)