From a3efe4a369a68b1c97f288c33f2cac9206b8a2cd Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 15:05:16 -0700 Subject: [PATCH 01/13] fix: add warning when encountering unknown field types The types returned for currently unsupported field types may change in the future, when support is added. Warn users that the types they are using are not yet supported. --- google/cloud/bigquery/_helpers.py | 6 +++++- tests/unit/test__helpers.py | 20 +++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index 5ee5e1850..d3dc94b62 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -21,6 +21,7 @@ import math import re import os +import warnings from typing import Optional, Union from dateutil import relativedelta @@ -382,7 +383,10 @@ def _field_to_index_mapping(schema): def _field_from_json(resource, field): - converter = _CELLDATA_FROM_JSON.get(field.field_type, lambda value, _: value) + converter = _CELLDATA_FROM_JSON.get(field.field_type) + if not converter: + warnings.warn("Unknown field type '{}'.".format(field.field_type), FutureWarning) + converter = lambda value, _: value if field.mode == "REPEATED": return [converter(item["v"], field) for item in resource] else: diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 1bf21479f..4cfbe1563 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -17,6 +17,7 @@ import decimal import json import os +import warnings import pytest import packaging import unittest @@ -632,7 +633,8 @@ def _call_fut(self, row, schema): from google.cloud.bigquery._helpers import _row_tuple_from_json with _field_isinstance_patcher(): - return _row_tuple_from_json(row, schema) + ret = _row_tuple_from_json(row, schema) + return ret def test_w_single_scalar_column(self): # SELECT 1 AS col @@ -640,6 +642,22 @@ def test_w_single_scalar_column(self): row = {"f": [{"v": "1"}]} self.assertEqual(self._call_fut(row, schema=[col]), (1,)) + def test_w_unknown_type(self): + # SELECT 1 AS col + col = _Field("REQUIRED", "col", "UNKNOWN") + row = {"f": [{"v": "1"}]} + with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: + self.assertEqual(self._call_fut(row, schema=[col]), ('1',)) + self.assertEqual(len(record), 1) + + def test_w_unknown_type_repeated(self): + # SELECT 1 AS col + col = _Field("REPEATED", "col", "UNKNOWN") + row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} + with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: + self.assertEqual(self._call_fut(row, schema=[col]), (['1', '2', '3'],)) + self.assertEqual(len(record), 1) + def test_w_single_scalar_geography_column(self): # SELECT 1 AS col col = _Field("REQUIRED", "geo", "GEOGRAPHY") From fa8329f54f1e32130bec60ff6bf4a7dc1e9686f9 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 18:43:31 -0700 Subject: [PATCH 02/13] fix: add warning for unknown subfield types as well --- google/cloud/bigquery/_helpers.py | 7 +------ tests/unit/test__helpers.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index d3dc94b62..10bde9e54 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -298,12 +298,7 @@ def _record_from_json(value, field): record = {} record_iter = zip(field.fields, value["f"]) for subfield, cell in record_iter: - converter = _CELLDATA_FROM_JSON[subfield.field_type] - if subfield.mode == "REPEATED": - value = [converter(item["v"], subfield) for item in cell["v"]] - else: - value = converter(cell["v"], subfield) - record[subfield.name] = value + record[subfield.name] = _field_from_json(cell["v"], subfield) return record diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 4cfbe1563..e4db79994 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -702,6 +702,32 @@ def test_w_struct_w_nested_array_column(self): ({"first": [1, 2], "second": 3, "third": [4, 5]},), ) + def test_w_unknown_type_subfield(self): + # SELECT [(1, 2, 3), (4, 5, 6)] as col + first = _Field("REPEATED", "first", "UNKNOWN1") + second = _Field("REQUIRED", "second", "UNKNOWN2") + third = _Field("REPEATED", "third", "INTEGER") + col = _Field("REQUIRED", "col", "RECORD", fields=[first, second, third]) + row = { + "f": [ + { + "v": { + "f": [ + {"v": [{"v": "1"}, {"v": "2"}]}, + {"v": "3"}, + {"v": [{"v": "4"}, {"v": "5"}]}, + ] + } + } + ] + } + with pytest.warns(FutureWarning, match="Unknown field type") as record: + self.assertEqual( + self._call_fut(row, schema=[col]), + ({"first": ['1', '2'], "second": '3', "third": [4, 5]},), + ) + self.assertEqual(len(record), 2) + def test_w_array_of_struct(self): # SELECT [(1, 2, 3), (4, 5, 6)] as col first = _Field("REQUIRED", "first", "INTEGER") From f3821697924bfce51fce69f7ea784d09a0d392cc Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 18:45:09 -0700 Subject: [PATCH 03/13] fix: remove unused warnings --- tests/unit/test__helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index e4db79994..156bc0ad7 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -17,7 +17,6 @@ import decimal import json import os -import warnings import pytest import packaging import unittest From f4e006f3676b1e54605a0a47ae43925e4dc00296 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 18:46:08 -0700 Subject: [PATCH 04/13] fix: remove leftover debugging code --- tests/unit/test__helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 156bc0ad7..844758cb1 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -632,8 +632,7 @@ def _call_fut(self, row, schema): from google.cloud.bigquery._helpers import _row_tuple_from_json with _field_isinstance_patcher(): - ret = _row_tuple_from_json(row, schema) - return ret + return _row_tuple_from_json(row, schema) def test_w_single_scalar_column(self): # SELECT 1 AS col From 83ee4aa7d8e851db70295eb33131cfc867a88b18 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 18:48:27 -0700 Subject: [PATCH 05/13] move test case closer to related test --- tests/unit/test__helpers.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 844758cb1..149eb0d55 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -648,14 +648,6 @@ def test_w_unknown_type(self): self.assertEqual(self._call_fut(row, schema=[col]), ('1',)) self.assertEqual(len(record), 1) - def test_w_unknown_type_repeated(self): - # SELECT 1 AS col - col = _Field("REPEATED", "col", "UNKNOWN") - row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} - with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: - self.assertEqual(self._call_fut(row, schema=[col]), (['1', '2', '3'],)) - self.assertEqual(len(record), 1) - def test_w_single_scalar_geography_column(self): # SELECT 1 AS col col = _Field("REQUIRED", "geo", "GEOGRAPHY") @@ -676,6 +668,14 @@ def test_w_single_array_column(self): row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} self.assertEqual(self._call_fut(row, schema=[col]), ([1, 2, 3],)) + def test_w_unknown_type_repeated(self): + # SELECT 1 AS col + col = _Field("REPEATED", "col", "UNKNOWN") + row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} + with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: + self.assertEqual(self._call_fut(row, schema=[col]), (['1', '2', '3'],)) + self.assertEqual(len(record), 1) + def test_w_struct_w_nested_array_column(self): # SELECT ([1, 2], 3, [4, 5]) as col first = _Field("REPEATED", "first", "INTEGER") From d1d1eb44c251382fc6a9081401c9d84788f8404e Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 2 Aug 2024 18:52:35 -0700 Subject: [PATCH 06/13] add comments --- tests/unit/test__helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 149eb0d55..bc721290a 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -674,7 +674,7 @@ def test_w_unknown_type_repeated(self): row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: self.assertEqual(self._call_fut(row, schema=[col]), (['1', '2', '3'],)) - self.assertEqual(len(record), 1) + self.assertEqual(len(record), 1) # only 1 warning for repeated field. def test_w_struct_w_nested_array_column(self): # SELECT ([1, 2], 3, [4, 5]) as col @@ -724,7 +724,7 @@ def test_w_unknown_type_subfield(self): self._call_fut(row, schema=[col]), ({"first": ['1', '2'], "second": '3', "third": [4, 5]},), ) - self.assertEqual(len(record), 2) + self.assertEqual(len(record), 2) # 1 warning per unknown field. def test_w_array_of_struct(self): # SELECT [(1, 2, 3), (4, 5, 6)] as col From a379e7b84c67452a61c046469f1d5909b6aff89f Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Mon, 5 Aug 2024 10:42:21 -0700 Subject: [PATCH 07/13] fix formatting --- google/cloud/bigquery/_helpers.py | 9 +++++---- tests/unit/test__helpers.py | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index 10bde9e54..8493035d0 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -378,10 +378,11 @@ def _field_to_index_mapping(schema): def _field_from_json(resource, field): - converter = _CELLDATA_FROM_JSON.get(field.field_type) - if not converter: - warnings.warn("Unknown field type '{}'.".format(field.field_type), FutureWarning) - converter = lambda value, _: value + if field.field_type not in _CELLDATA_FROM_JSON: + warnings.warn( + "Unknown field type '{}'.".format(field.field_type), FutureWarning + ) + converter = _CELLDATA_FROM_JSON.get(field.field_type, lambda value, _: value) if field.mode == "REPEATED": return [converter(item["v"], field) for item in resource] else: diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index bc721290a..3a7980ab6 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -644,8 +644,10 @@ def test_w_unknown_type(self): # SELECT 1 AS col col = _Field("REQUIRED", "col", "UNKNOWN") row = {"f": [{"v": "1"}]} - with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: - self.assertEqual(self._call_fut(row, schema=[col]), ('1',)) + with pytest.warns( + FutureWarning, match="Unknown field type 'UNKNOWN'." + ) as record: + self.assertEqual(self._call_fut(row, schema=[col]), ("1",)) self.assertEqual(len(record), 1) def test_w_single_scalar_geography_column(self): @@ -672,9 +674,11 @@ def test_w_unknown_type_repeated(self): # SELECT 1 AS col col = _Field("REPEATED", "col", "UNKNOWN") row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} - with pytest.warns(FutureWarning, match="Unknown field type 'UNKNOWN'.") as record: - self.assertEqual(self._call_fut(row, schema=[col]), (['1', '2', '3'],)) - self.assertEqual(len(record), 1) # only 1 warning for repeated field. + with pytest.warns( + FutureWarning, match="Unknown field type 'UNKNOWN'." + ) as record: + self.assertEqual(self._call_fut(row, schema=[col]), (["1", "2", "3"],)) + self.assertEqual(len(record), 1) # only 1 warning for repeated field. def test_w_struct_w_nested_array_column(self): # SELECT ([1, 2], 3, [4, 5]) as col @@ -701,7 +705,7 @@ def test_w_struct_w_nested_array_column(self): ) def test_w_unknown_type_subfield(self): - # SELECT [(1, 2, 3), (4, 5, 6)] as col + # SELECT [(1, 2, 3), (4, 5, 6)] as col first = _Field("REPEATED", "first", "UNKNOWN1") second = _Field("REQUIRED", "second", "UNKNOWN2") third = _Field("REPEATED", "third", "INTEGER") @@ -719,12 +723,12 @@ def test_w_unknown_type_subfield(self): } ] } - with pytest.warns(FutureWarning, match="Unknown field type") as record: + with pytest.warns(FutureWarning, match="Unknown field type") as record: self.assertEqual( self._call_fut(row, schema=[col]), - ({"first": ['1', '2'], "second": '3', "third": [4, 5]},), + ({"first": ["1", "2"], "second": "3", "third": [4, 5]},), ) - self.assertEqual(len(record), 2) # 1 warning per unknown field. + self.assertEqual(len(record), 2) # 1 warning per unknown field. def test_w_array_of_struct(self): # SELECT [(1, 2, 3), (4, 5, 6)] as col From 93dda1660fea4f26bbad2a480f49b03e0d91aa78 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Mon, 5 Aug 2024 14:11:14 -0700 Subject: [PATCH 08/13] fix test_table and use warnings.warn instead of pytest.warn --- google/cloud/bigquery/_helpers.py | 9 ++++--- google/cloud/bigquery/_pandas_helpers.py | 4 +++- tests/unit/test__helpers.py | 30 ++++++++++++++++-------- tests/unit/test_table.py | 6 ++--- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index 8493035d0..cd83579de 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -378,11 +378,14 @@ def _field_to_index_mapping(schema): def _field_from_json(resource, field): - if field.field_type not in _CELLDATA_FROM_JSON: + def default_converter(value, field): warnings.warn( - "Unknown field type '{}'.".format(field.field_type), FutureWarning + "Unknown type '{}' for field '{}'.".format(field.field_type, field.name), + FutureWarning, ) - converter = _CELLDATA_FROM_JSON.get(field.field_type, lambda value, _: value) + return value + + converter = _CELLDATA_FROM_JSON.get(field.field_type, default_converter) if field.mode == "REPEATED": return [converter(item["v"], field) for item in resource] else: diff --git a/google/cloud/bigquery/_pandas_helpers.py b/google/cloud/bigquery/_pandas_helpers.py index 8395478fb..c21a02569 100644 --- a/google/cloud/bigquery/_pandas_helpers.py +++ b/google/cloud/bigquery/_pandas_helpers.py @@ -204,7 +204,9 @@ def bq_to_arrow_field(bq_field, array_type=None): metadata=metadata, ) - warnings.warn("Unable to determine type for field '{}'.".format(bq_field.name)) + warnings.warn( + "Unable to determine Arrow type for field '{}'.".format(bq_field.name) + ) return None diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 3a7980ab6..031ae7784 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -17,6 +17,7 @@ import decimal import json import os +import warnings import pytest import packaging import unittest @@ -644,11 +645,12 @@ def test_w_unknown_type(self): # SELECT 1 AS col col = _Field("REQUIRED", "col", "UNKNOWN") row = {"f": [{"v": "1"}]} - with pytest.warns( - FutureWarning, match="Unknown field type 'UNKNOWN'." - ) as record: + with warnings.catch_warnings(record=True) as warned: self.assertEqual(self._call_fut(row, schema=[col]), ("1",)) - self.assertEqual(len(record), 1) + self.assertEqual(len(warned), 1) + warning = warned[0] + self.assertTrue("UNKNOWN" in str(warning)) + self.assertTrue("col" in str(warning)) def test_w_single_scalar_geography_column(self): # SELECT 1 AS col @@ -674,11 +676,12 @@ def test_w_unknown_type_repeated(self): # SELECT 1 AS col col = _Field("REPEATED", "col", "UNKNOWN") row = {"f": [{"v": [{"v": "1"}, {"v": "2"}, {"v": "3"}]}]} - with pytest.warns( - FutureWarning, match="Unknown field type 'UNKNOWN'." - ) as record: + with warnings.catch_warnings(record=True) as warned: self.assertEqual(self._call_fut(row, schema=[col]), (["1", "2", "3"],)) - self.assertEqual(len(record), 1) # only 1 warning for repeated field. + self.assertEqual(len(warned), 1) + warning = warned[0] + self.assertTrue("UNKNOWN" in str(warning)) + self.assertTrue("col" in str(warning)) def test_w_struct_w_nested_array_column(self): # SELECT ([1, 2], 3, [4, 5]) as col @@ -723,12 +726,19 @@ def test_w_unknown_type_subfield(self): } ] } - with pytest.warns(FutureWarning, match="Unknown field type") as record: + with warnings.catch_warnings(record=True) as warned: self.assertEqual( self._call_fut(row, schema=[col]), ({"first": ["1", "2"], "second": "3", "third": [4, 5]},), ) - self.assertEqual(len(record), 2) # 1 warning per unknown field. + self.assertEqual(len(warned), 2) # 1 warning per unknown field. + warned = [str(warning) for warning in warned] + self.assertTrue( + any("first" in warning and "UNKNOWN1" in warning for warning in warned) + ) + self.assertTrue( + any("second" in warning and "UNKNOWN2" in warning for warning in warned) + ) def test_w_array_of_struct(self): # SELECT [(1, 2, 3), (4, 5, 6)] as col diff --git a/tests/unit/test_table.py b/tests/unit/test_table.py index 7a97c7b78..d6febcfb1 100644 --- a/tests/unit/test_table.py +++ b/tests/unit/test_table.py @@ -2751,9 +2751,9 @@ def test_to_arrow_w_unknown_type(self): self.assertEqual(ages, [33, 29]) self.assertEqual(sports, ["volleyball", "basketball"]) - self.assertEqual(len(warned), 1) - warning = warned[0] - self.assertTrue("sport" in str(warning)) + # Expect warning from both the arrow conversion, and the json deserialization. + self.assertEqual(len(warned), 2) + self.assertTrue(all("sport" in str(warning) for warning in warned)) def test_to_arrow_w_empty_table(self): pyarrow = pytest.importorskip( From 2be28305a32a9c9645edec707c9a914b74c7f6f8 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Mon, 5 Aug 2024 16:35:56 -0700 Subject: [PATCH 09/13] add explicit warning about behavior subject to change in the future add warning for write and warn about future behavior changes --- google/cloud/bigquery/_helpers.py | 18 ++++++++++++++++-- tests/unit/test__helpers.py | 6 +++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index cd83579de..fbe5e2571 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -380,7 +380,9 @@ def _field_to_index_mapping(schema): def _field_from_json(resource, field): def default_converter(value, field): 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 @@ -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. @@ -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. @@ -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) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 031ae7784..910b1396f 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -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") From 094e7dcd5ed13a4a0d29c98cb8a238c0d919952f Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 7 Aug 2024 08:24:21 -0700 Subject: [PATCH 10/13] add default converter for _SCALAR_VALUE_TO_JSON_PARAM --- google/cloud/bigquery/query.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/google/cloud/bigquery/query.py b/google/cloud/bigquery/query.py index 9c59056fd..6695c4b52 100644 --- a/google/cloud/bigquery/query.py +++ b/google/cloud/bigquery/query.py @@ -591,9 +591,8 @@ def to_api_repr(self) -> dict: Dict: JSON mapping """ value = self.value - converter = _SCALAR_VALUE_TO_JSON_PARAM.get(self.type_) - if converter is not None: - value = converter(value) # type: ignore + converter = _SCALAR_VALUE_TO_JSON_PARAM.get(self.type_, lambda value: value) + value = converter(value) # type: ignore resource: Dict[str, Any] = { "parameterType": {"type": self.type_}, "parameterValue": {"value": value}, @@ -748,9 +747,8 @@ def to_api_repr(self) -> dict: else: a_type = self.array_type.to_api_repr() - converter = _SCALAR_VALUE_TO_JSON_PARAM.get(a_type["type"]) - if converter is not None: - values = [converter(value) for value in values] # type: ignore + converter = _SCALAR_VALUE_TO_JSON_PARAM.get(a_type["type"], lambda value: value) + values = [converter(value) for value in values] # type: ignore a_values = [{"value": value} for value in values] resource = { @@ -897,10 +895,8 @@ def to_api_repr(self) -> dict: values[name] = repr_["parameterValue"] else: s_types[name] = {"name": name, "type": {"type": type_}} - converter = _SCALAR_VALUE_TO_JSON_PARAM.get(type_) - if converter is not None: - value = converter(value) # type: ignore - values[name] = {"value": value} + converter = _SCALAR_VALUE_TO_JSON_PARAM.get(type_, lambda value: value) + values[name] = {"value": converter(value)} resource = { "parameterType": { From f29bb055abe4c1f8e2f66b2db2ac87e92037b9eb Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 7 Aug 2024 11:42:41 -0700 Subject: [PATCH 11/13] factor out shared warning --- google/cloud/bigquery/_helpers.py | 31 ++++++++++++++++--------------- google/cloud/bigquery/query.py | 4 +++- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/google/cloud/bigquery/_helpers.py b/google/cloud/bigquery/_helpers.py index fbe5e2571..1eda80712 100644 --- a/google/cloud/bigquery/_helpers.py +++ b/google/cloud/bigquery/_helpers.py @@ -379,12 +379,7 @@ def _field_to_index_mapping(schema): def _field_from_json(resource, field): def default_converter(value, field): - warnings.warn( - "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, - ) + _warn_unknown_field_type(field) return value converter = _CELLDATA_FROM_JSON.get(field.field_type, default_converter) @@ -620,6 +615,15 @@ def _range_field_to_json(range_element_type, value): _SCALAR_VALUE_TO_JSON_PARAM["TIMESTAMP"] = _timestamp_to_json_parameter +def _warn_unknown_field_type(field): + warnings.warn( + "Unknown type '{}' for field '{}'. Behavior reading and writing this type is not officially supported and may change in the future.".format( + field.field_type, field.name + ), + FutureWarning, + ) + + def _scalar_field_to_json(field, row_value): """Maps a field and value to a JSON-safe value. @@ -632,15 +636,12 @@ def _scalar_field_to_json(field, row_value): Returns: Any: A JSON-serializable object. """ - converter = _SCALAR_VALUE_TO_JSON_ROW.get(field.field_type) - 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 + + def default_converter(value): + _warn_unknown_field_type(field) + return value + + converter = _SCALAR_VALUE_TO_JSON_ROW.get(field.field_type, default_converter) return converter(row_value) diff --git a/google/cloud/bigquery/query.py b/google/cloud/bigquery/query.py index 6695c4b52..1625c2d15 100644 --- a/google/cloud/bigquery/query.py +++ b/google/cloud/bigquery/query.py @@ -747,7 +747,9 @@ def to_api_repr(self) -> dict: else: a_type = self.array_type.to_api_repr() - converter = _SCALAR_VALUE_TO_JSON_PARAM.get(a_type["type"], lambda value: value) + converter = _SCALAR_VALUE_TO_JSON_PARAM.get( + a_type["type"], lambda value: value + ) values = [converter(value) for value in values] # type: ignore a_values = [{"value": value} for value in values] From c0b6272a200424f445d96b6fa0c878b7715bd3c6 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 7 Aug 2024 13:48:55 -0700 Subject: [PATCH 12/13] fix test case and make coverage happy --- google/cloud/bigquery/query.py | 2 +- tests/unit/test__helpers.py | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/google/cloud/bigquery/query.py b/google/cloud/bigquery/query.py index 1625c2d15..f1090a7dc 100644 --- a/google/cloud/bigquery/query.py +++ b/google/cloud/bigquery/query.py @@ -792,7 +792,7 @@ def __repr__(self): class StructQueryParameter(_AbstractQueryParameter): - """Named / positional query parameters for struct values. + """Name / positional query parameters for struct values. Args: name (Optional[str]): diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 910b1396f..7071858ce 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -734,10 +734,10 @@ def test_w_unknown_type_subfield(self): self.assertEqual(len(warned), 2) # 1 warning per unknown field. warned = [str(warning) for warning in warned] self.assertTrue( - any("first" in warning and "UNKNOWN1" in warning for warning in warned) + any(["first" in warning and "UNKNOWN1" in warning for warning in warned]) ) self.assertTrue( - any("second" in warning and "UNKNOWN2" in warning for warning in warned) + any(["second" in warning and "UNKNOWN2" in warning for warning in warned]) ) def test_w_array_of_struct(self): @@ -1637,6 +1637,18 @@ def test_decimal_as_float_api_repr(): } +def test_unknown_type_as_api_repr(): + """Make sure decimals get converted to float.""" + import google.cloud.bigquery.query + + param = google.cloud.bigquery.query.ScalarQueryParameter("x", "UNKNOWN_TYPE", 54) + assert param.to_api_repr() == { + "parameterType": {"type": "UNKNOWN_TYPE"}, + "parameterValue": {"value": 54}, + "name": "x", + } + + class Test__get_bigquery_host(unittest.TestCase): @staticmethod def _call_fut(): From be54ee56d363685122500a7781f12d172a74d178 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 7 Aug 2024 14:36:50 -0700 Subject: [PATCH 13/13] add unit test to StructQueryParameter class --- tests/unit/test__helpers.py | 12 ------------ tests/unit/test_query.py | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 7071858ce..0a307498f 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -1637,18 +1637,6 @@ def test_decimal_as_float_api_repr(): } -def test_unknown_type_as_api_repr(): - """Make sure decimals get converted to float.""" - import google.cloud.bigquery.query - - param = google.cloud.bigquery.query.ScalarQueryParameter("x", "UNKNOWN_TYPE", 54) - assert param.to_api_repr() == { - "parameterType": {"type": "UNKNOWN_TYPE"}, - "parameterValue": {"value": 54}, - "name": "x", - } - - class Test__get_bigquery_host(unittest.TestCase): @staticmethod def _call_fut(): diff --git a/tests/unit/test_query.py b/tests/unit/test_query.py index 7c36eb75b..40ef080f7 100644 --- a/tests/unit/test_query.py +++ b/tests/unit/test_query.py @@ -1780,6 +1780,25 @@ def test_to_api_repr_w_nested_struct(self): param = self._make_one("foo", scalar_1, sub) self.assertEqual(param.to_api_repr(), EXPECTED) + def test_to_api_repr_w_unknown_type(self): + EXPECTED = { + "name": "foo", + "parameterType": { + "type": "STRUCT", + "structTypes": [ + {"name": "bar", "type": {"type": "INT64"}}, + {"name": "baz", "type": {"type": "UNKNOWN_TYPE"}}, + ], + }, + "parameterValue": { + "structValues": {"bar": {"value": "123"}, "baz": {"value": "abc"}} + }, + } + sub_1 = _make_subparam("bar", "INT64", 123) + sub_2 = _make_subparam("baz", "UNKNOWN_TYPE", "abc") + param = self._make_one("foo", sub_1, sub_2) + self.assertEqual(param.to_api_repr(), EXPECTED) + def test___eq___wrong_type(self): field = self._make_one("test", _make_subparam("bar", "STRING", "abc")) other = object()