Skip to content

fix: add warning when encountering unknown field types #1989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Aug 13, 2024
Merged
Prev Previous commit
Next Next commit
add explicit warning about behavior subject to change in the future
add warning for write and warn about future behavior changes
  • Loading branch information
suzmue committed Aug 5, 2024
commit 2be28305a32a9c9645edec707c9a914b74c7f6f8
18 changes: 16 additions & 2 deletions google/cloud/bigquery/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,9 @@ def _field_to_index_mapping(schema):
def _field_from_json(resource, field):
def default_converter(value, field):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can move this function to outside, so we can reuse it for _scalar_field_to_json() as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up just moving the warning itself to a new function, since the converter in _scalar_field_to_json has a different function signature.

warnings.warn(
"Unknown type '{}' for field '{}'.".format(field.field_type, field.name),
"Unknown type '{}' for field '{}'. Behavior reading this type is not officially supported and may change in the future.".format(
field.field_type, field.name
),
FutureWarning,
)
return value
Expand Down Expand Up @@ -487,6 +489,11 @@ def _json_to_json(value):
return json.dumps(value)


def _string_to_json(value):
"""NOOP string -> string coercion"""
return value


def _timestamp_to_json_parameter(value):
"""Coerce 'value' to an JSON-compatible representation.

Expand Down Expand Up @@ -599,6 +606,7 @@ def _range_field_to_json(range_element_type, value):
"DATE": _date_to_json,
"TIME": _time_to_json,
"JSON": _json_to_json,
"STRING": _string_to_json,
# Make sure DECIMAL and BIGDECIMAL are handled, even though
# requests for them should be converted to NUMERIC. Better safe
# than sorry.
Expand All @@ -625,7 +633,13 @@ def _scalar_field_to_json(field, row_value):
Any: A JSON-serializable object.
"""
converter = _SCALAR_VALUE_TO_JSON_ROW.get(field.field_type)
if converter is None: # STRING doesn't need converting
if converter is None:
warnings.warn(
"Unknown type '{}' for field '{}'. Behavior writing this type is not officially supported and may change in the future.".format(
field.field_type, field.name
),
FutureWarning,
)
return row_value
return converter(row_value)

Expand Down
6 changes: 5 additions & 1 deletion tests/unit/test__helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1132,8 +1132,12 @@ def _call_fut(self, field, value):
def test_w_unknown_field_type(self):
field = _make_field("UNKNOWN")
original = object()
converted = self._call_fut(field, original)
with warnings.catch_warnings(record=True) as warned:
converted = self._call_fut(field, original)
self.assertIs(converted, original)
self.assertEqual(len(warned), 1)
warning = warned[0]
self.assertTrue("UNKNOWN" in str(warning))

def test_w_known_field_type(self):
field = _make_field("INT64")
Expand Down