Skip to content

Commit 44be1ac

Browse files
varun-edachali-dbxvarun-edachali-dbx
committed
stop throwing errors from type conversion
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
1 parent 1332a43 commit 44be1ac

File tree

4 files changed

+60
-94
lines changed

4 files changed

+60
-94
lines changed

‎src/databricks/sql/backend/sea/result_set.py‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,19 @@ def _convert_json_types(self, row: List[str]) -> List[Any]:
9292
converted_row = []
9393

9494
for i, value in enumerate(row):
95+
column_name = self.description[i][0]
9596
column_type = self.description[i][1]
9697
precision = self.description[i][4]
9798
scale = self.description[i][5]
9899

99-
try:
100-
converted_value = SqlTypeConverter.convert_value(
101-
value, column_type, precision=precision, scale=scale
102-
)
103-
converted_row.append(converted_value)
104-
except Exception as e:
105-
logger.warning(
106-
f"Error converting value '{value}' to {column_type}: {e}"
107-
)
108-
converted_row.append(value)
100+
converted_value = SqlTypeConverter.convert_value(
101+
value,
102+
column_type,
103+
column_name=column_name,
104+
precision=precision,
105+
scale=scale,
106+
)
107+
converted_row.append(converted_value)
109108

110109
return converted_row
111110

‎src/databricks/sql/backend/sea/utils/conversion.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ class SqlTypeConverter:
135135
def convert_value(
136136
value: str,
137137
sql_type: str,
138+
column_name: Optional[str],
138139
**kwargs,
139140
) -> object:
140141
"""
@@ -143,6 +144,7 @@ def convert_value(
143144
Args:
144145
value: The string value to convert
145146
sql_type: The SQL type (e.g., 'tinyint', 'decimal')
147+
column_name: The name of the column being converted
146148
**kwargs: Additional keyword arguments for the conversion function
147149
148150
Returns:
@@ -162,6 +164,10 @@ def convert_value(
162164
return converter_func(value, precision, scale)
163165
else:
164166
return converter_func(value)
165-
except (ValueError, TypeError, decimal.InvalidOperation) as e:
166-
logger.warning(f"Error converting value '{value}' to {sql_type}: {e}")
167+
except Exception as e:
168+
warning_message = f"Error converting value '{value}' to {sql_type}"
169+
if column_name:
170+
warning_message += f" in column {column_name}"
171+
warning_message += f": {e}"
172+
logger.warning(warning_message)
167173
return value

‎tests/unit/test_sea_conversion.py‎

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,61 +18,62 @@ class TestSqlTypeConverter:
1818
def test_convert_numeric_types(self):
1919
"""Test converting numeric types."""
2020
# Test integer types
21-
assert SqlTypeConverter.convert_value("123", SqlType.TINYINT) == 123
22-
assert SqlTypeConverter.convert_value("456", SqlType.SMALLINT) == 456
23-
assert SqlTypeConverter.convert_value("789", SqlType.INT) == 789
21+
assert SqlTypeConverter.convert_value("123", SqlType.TINYINT, None) == 123
22+
assert SqlTypeConverter.convert_value("456", SqlType.SMALLINT, None) == 456
23+
assert SqlTypeConverter.convert_value("789", SqlType.INT, None) == 789
2424
assert (
25-
SqlTypeConverter.convert_value("1234567890", SqlType.BIGINT) == 1234567890
25+
SqlTypeConverter.convert_value("1234567890", SqlType.BIGINT, None)
26+
== 1234567890
2627
)
2728

2829
# Test floating point types
29-
assert SqlTypeConverter.convert_value("123.45", SqlType.FLOAT) == 123.45
30-
assert SqlTypeConverter.convert_value("678.90", SqlType.DOUBLE) == 678.90
30+
assert SqlTypeConverter.convert_value("123.45", SqlType.FLOAT, None) == 123.45
31+
assert SqlTypeConverter.convert_value("678.90", SqlType.DOUBLE, None) == 678.90
3132

3233
# Test decimal type
33-
decimal_value = SqlTypeConverter.convert_value("123.45", SqlType.DECIMAL)
34+
decimal_value = SqlTypeConverter.convert_value("123.45", SqlType.DECIMAL, None)
3435
assert isinstance(decimal_value, decimal.Decimal)
3536
assert decimal_value == decimal.Decimal("123.45")
3637

3738
# Test decimal with precision and scale
3839
decimal_value = SqlTypeConverter.convert_value(
39-
"123.45", SqlType.DECIMAL, precision=5, scale=2
40+
"123.45", SqlType.DECIMAL, None, precision=5, scale=2
4041
)
4142
assert isinstance(decimal_value, decimal.Decimal)
4243
assert decimal_value == decimal.Decimal("123.45")
4344

4445
# Test invalid numeric input
45-
result = SqlTypeConverter.convert_value("not_a_number", SqlType.INT)
46+
result = SqlTypeConverter.convert_value("not_a_number", SqlType.INT, None)
4647
assert result == "not_a_number" # Returns original value on error
4748

4849
def test_convert_boolean_type(self):
4950
"""Test converting boolean types."""
5051
# True values
51-
assert SqlTypeConverter.convert_value("true", SqlType.BOOLEAN) is True
52-
assert SqlTypeConverter.convert_value("True", SqlType.BOOLEAN) is True
53-
assert SqlTypeConverter.convert_value("t", SqlType.BOOLEAN) is True
54-
assert SqlTypeConverter.convert_value("1", SqlType.BOOLEAN) is True
55-
assert SqlTypeConverter.convert_value("yes", SqlType.BOOLEAN) is True
56-
assert SqlTypeConverter.convert_value("y", SqlType.BOOLEAN) is True
52+
assert SqlTypeConverter.convert_value("true", SqlType.BOOLEAN, None) is True
53+
assert SqlTypeConverter.convert_value("True", SqlType.BOOLEAN, None) is True
54+
assert SqlTypeConverter.convert_value("t", SqlType.BOOLEAN, None) is True
55+
assert SqlTypeConverter.convert_value("1", SqlType.BOOLEAN, None) is True
56+
assert SqlTypeConverter.convert_value("yes", SqlType.BOOLEAN, None) is True
57+
assert SqlTypeConverter.convert_value("y", SqlType.BOOLEAN, None) is True
5758

5859
# False values
59-
assert SqlTypeConverter.convert_value("false", SqlType.BOOLEAN) is False
60-
assert SqlTypeConverter.convert_value("False", SqlType.BOOLEAN) is False
61-
assert SqlTypeConverter.convert_value("f", SqlType.BOOLEAN) is False
62-
assert SqlTypeConverter.convert_value("0", SqlType.BOOLEAN) is False
63-
assert SqlTypeConverter.convert_value("no", SqlType.BOOLEAN) is False
64-
assert SqlTypeConverter.convert_value("n", SqlType.BOOLEAN) is False
60+
assert SqlTypeConverter.convert_value("false", SqlType.BOOLEAN, None) is False
61+
assert SqlTypeConverter.convert_value("False", SqlType.BOOLEAN, None) is False
62+
assert SqlTypeConverter.convert_value("f", SqlType.BOOLEAN, None) is False
63+
assert SqlTypeConverter.convert_value("0", SqlType.BOOLEAN, None) is False
64+
assert SqlTypeConverter.convert_value("no", SqlType.BOOLEAN, None) is False
65+
assert SqlTypeConverter.convert_value("n", SqlType.BOOLEAN, None) is False
6566

6667
def test_convert_datetime_types(self):
6768
"""Test converting datetime types."""
6869
# Test date type
69-
date_value = SqlTypeConverter.convert_value("2023-01-15", SqlType.DATE)
70+
date_value = SqlTypeConverter.convert_value("2023-01-15", SqlType.DATE, None)
7071
assert isinstance(date_value, datetime.date)
7172
assert date_value == datetime.date(2023, 1, 15)
7273

7374
# Test timestamp type
7475
timestamp_value = SqlTypeConverter.convert_value(
75-
"2023-01-15T12:30:45", SqlType.TIMESTAMP
76+
"2023-01-15T12:30:45", SqlType.TIMESTAMP, None
7677
)
7778
assert isinstance(timestamp_value, datetime.datetime)
7879
assert timestamp_value.year == 2023
@@ -84,58 +85,65 @@ def test_convert_datetime_types(self):
8485

8586
# Test interval types (currently return as string)
8687
interval_ym_value = SqlTypeConverter.convert_value(
87-
"1-6", SqlType.INTERVAL_YEAR_MONTH
88+
"1-6", SqlType.INTERVAL_YEAR_MONTH, None
8889
)
8990
assert interval_ym_value == "1-6"
9091

9192
interval_dt_value = SqlTypeConverter.convert_value(
92-
"1 day 2 hours", SqlType.INTERVAL_DAY_TIME
93+
"1 day 2 hours", SqlType.INTERVAL_DAY_TIME, None
9394
)
9495
assert interval_dt_value == "1 day 2 hours"
9596

9697
# Test invalid date input
97-
result = SqlTypeConverter.convert_value("not_a_date", SqlType.DATE)
98+
result = SqlTypeConverter.convert_value("not_a_date", SqlType.DATE, None)
9899
assert result == "not_a_date" # Returns original value on error
99100

100101
def test_convert_string_types(self):
101102
"""Test converting string types."""
102103
# String types don't need conversion, they should be returned as-is
103104
assert (
104-
SqlTypeConverter.convert_value("test string", SqlType.STRING)
105+
SqlTypeConverter.convert_value("test string", SqlType.STRING, None)
105106
== "test string"
106107
)
107-
assert SqlTypeConverter.convert_value("test char", SqlType.CHAR) == "test char"
108108
assert (
109-
SqlTypeConverter.convert_value("test varchar", SqlType.VARCHAR)
109+
SqlTypeConverter.convert_value("test char", SqlType.CHAR, None)
110+
== "test char"
111+
)
112+
assert (
113+
SqlTypeConverter.convert_value("test varchar", SqlType.VARCHAR, None)
110114
== "test varchar"
111115
)
112116

113117
def test_convert_binary_type(self):
114118
"""Test converting binary type."""
115119
# Test valid hex string
116-
binary_value = SqlTypeConverter.convert_value("48656C6C6F", SqlType.BINARY)
120+
binary_value = SqlTypeConverter.convert_value(
121+
"48656C6C6F", SqlType.BINARY, None
122+
)
117123
assert isinstance(binary_value, bytes)
118124
assert binary_value == b"Hello"
119125

120126
# Test invalid binary input
121-
result = SqlTypeConverter.convert_value("not_hex", SqlType.BINARY)
127+
result = SqlTypeConverter.convert_value("not_hex", SqlType.BINARY, None)
122128
assert result == "not_hex" # Returns original value on error
123129

124130
def test_convert_unsupported_type(self):
125131
"""Test converting an unsupported type."""
126132
# Should return the original value
127-
assert SqlTypeConverter.convert_value("test", "unsupported_type") == "test"
133+
assert (
134+
SqlTypeConverter.convert_value("test", "unsupported_type", None) == "test"
135+
)
128136

129137
# Complex types should return as-is (not yet implemented in TYPE_MAPPING)
130138
assert (
131-
SqlTypeConverter.convert_value("complex_value", SqlType.ARRAY)
139+
SqlTypeConverter.convert_value("complex_value", SqlType.ARRAY, None)
132140
== "complex_value"
133141
)
134142
assert (
135-
SqlTypeConverter.convert_value("complex_value", SqlType.MAP)
143+
SqlTypeConverter.convert_value("complex_value", SqlType.MAP, None)
136144
== "complex_value"
137145
)
138146
assert (
139-
SqlTypeConverter.convert_value("complex_value", SqlType.STRUCT)
147+
SqlTypeConverter.convert_value("complex_value", SqlType.STRUCT, None)
140148
== "complex_value"
141149
)

‎tests/unit/test_sea_result_set.py‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -565,50 +565,3 @@ def test_fetchall_empty_arrow_queue(self, result_set_with_arrow_queue):
565565

566566
# Verify _convert_arrow_table was called
567567
result_set_with_arrow_queue._convert_arrow_table.assert_called_once()
568-
569-
@patch("databricks.sql.backend.sea.utils.conversion.SqlTypeConverter.convert_value")
570-
def test_convert_json_types_with_errors(
571-
self, mock_convert_value, result_set_with_data
572-
):
573-
"""Test error handling in _convert_json_types."""
574-
# Mock the conversion to fail for the second and third values
575-
mock_convert_value.side_effect = [
576-
"value1", # First value converts normally
577-
Exception("Invalid int"), # Second value fails
578-
Exception("Invalid boolean"), # Third value fails
579-
]
580-
581-
# Data with invalid values
582-
data_row = ["value1", "not_an_int", "not_a_boolean"]
583-
584-
# Should not raise an exception but log warnings
585-
result = result_set_with_data._convert_json_types(data_row)
586-
587-
# The first value should be converted normally
588-
assert result[0] == "value1"
589-
590-
# The invalid values should remain as strings
591-
assert result[1] == "not_an_int"
592-
assert result[2] == "not_a_boolean"
593-
594-
@patch("databricks.sql.backend.sea.result_set.logger")
595-
@patch("databricks.sql.backend.sea.utils.conversion.SqlTypeConverter.convert_value")
596-
def test_convert_json_types_with_logging(
597-
self, mock_convert_value, mock_logger, result_set_with_data
598-
):
599-
"""Test that errors in _convert_json_types are logged."""
600-
# Mock the conversion to fail for the second and third values
601-
mock_convert_value.side_effect = [
602-
"value1", # First value converts normally
603-
Exception("Invalid int"), # Second value fails
604-
Exception("Invalid boolean"), # Third value fails
605-
]
606-
607-
# Data with invalid values
608-
data_row = ["value1", "not_an_int", "not_a_boolean"]
609-
610-
# Call the method
611-
result_set_with_data._convert_json_types(data_row)
612-
613-
# Verify warnings were logged
614-
assert mock_logger.warning.call_count == 2

0 commit comments

Comments
 (0)