From 9ff1b4aac7f6e9fcbd1e8501bf2387e70fcf7805 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Fri, 9 May 2025 11:20:12 +0000 Subject: [PATCH 1/3] fix: updates _get_transitive_schema_fields and tests --- sqlalchemy_bigquery/_types.py | 2 +- tests/unit/test__types.py | 132 ++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test__types.py diff --git a/sqlalchemy_bigquery/_types.py b/sqlalchemy_bigquery/_types.py index 8399e978..65540dbd 100644 --- a/sqlalchemy_bigquery/_types.py +++ b/sqlalchemy_bigquery/_types.py @@ -83,7 +83,7 @@ def _get_transitive_schema_fields(fields): results = [] for field in fields: results += [field] - if field.field_type in STRUCT_FIELD_TYPES: + if field.field_type in STRUCT_FIELD_TYPES and field.mode != "REPEATED": sub_fields = [ SchemaField.from_api_repr( dict(f.to_api_repr(), name=f"{field.name}.{f.name}") diff --git a/tests/unit/test__types.py b/tests/unit/test__types.py new file mode 100644 index 00000000..984746bd --- /dev/null +++ b/tests/unit/test__types.py @@ -0,0 +1,132 @@ +import pytest +from google.cloud.bigquery.schema import SchemaField + +from sqlalchemy_bigquery._types import _get_transitive_schema_fields, STRUCT_FIELD_TYPES + +def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): + """ + Helper function to create a SchemaField object for testing. + `sub_fields` should be a list of already created SchemaField objects. + """ + api_repr = { + "name": name, + "type": field_type, + "mode": mode, + "fields": [sf.to_api_repr() for sf in sub_fields] if sub_fields else [], + } + return SchemaField.from_api_repr(api_repr) + + +test_cases = [ + ( + "STRUCT field, not REPEATED, with sub-fields, should recurse", + [ + create_fut("s1", "STRUCT", "NULLABLE", sub_fields=[ + create_fut("child1", "STRING", "NULLABLE") + ]) + ], + ["s1", "s1.child1"], + ), + ( + "RECORD field (alias for STRUCT), not REPEATED, with sub-fields, should recurse", + [ + create_fut("r1", "RECORD", "NULLABLE", sub_fields=[ + create_fut("child_r1", "INTEGER", "NULLABLE") + ]) + ], + ["r1", "r1.child_r1"], + ), + ( + "STRUCT field, REPEATED, with sub-fields, should NOT recurse", + [ + create_fut("s2", "STRUCT", "REPEATED", sub_fields=[ + create_fut("child2", "STRING", "NULLABLE") + ]) + ], + ["s2"], + ), + ( + "Non-STRUCT field (STRING), not REPEATED, should NOT recurse", + [ + create_fut("f1", "STRING", "NULLABLE") + ], + ["f1"], + ), + ( + "Non-STRUCT field (INTEGER), REPEATED, should NOT recurse", + [ + create_fut("f2", "INTEGER", "REPEATED") + ], + ["f2"], + ), + ( + "Deeply nested STRUCT, not REPEATED, should recurse fully", + [ + create_fut("s_outer", "STRUCT", "NULLABLE", sub_fields=[ + create_fut("s_inner1", "STRUCT", "NULLABLE", sub_fields=[ + create_fut("s_leaf1", "STRING", "NULLABLE") + ]), + create_fut("s_sibling", "INTEGER", "NULLABLE"), + create_fut("s_inner2_repeated_struct", "STRUCT", "REPEATED", sub_fields=[ + create_fut("s_leaf2_ignored", "BOOLEAN", "NULLABLE") # This sub-field should be ignored + ]), + ]) + ], + ["s_outer", "s_outer.s_inner1", "s_outer.s_inner1.s_leaf1", "s_outer.s_sibling", "s_outer.s_inner2_repeated_struct"], + ), + ( + "STRUCT field, not REPEATED, but no sub-fields, should not error and not recurse further", + [ + create_fut("s3", "STRUCT", "NULLABLE", sub_fields=[]) + ], + ["s3"], + ), + ( + "Multiple top-level fields with mixed conditions", + [ + create_fut("id", "INTEGER", "REQUIRED"), + create_fut("user_profile", "STRUCT", "NULLABLE", sub_fields=[ + create_fut("name", "STRING", "NULLABLE"), + create_fut("addresses", "RECORD", "REPEATED", sub_fields=[ # addresses is REPEATED STRUCT + create_fut("street", "STRING", "NULLABLE"), # This sub-field should be ignored + create_fut("city", "STRING", "NULLABLE") # This sub-field should be ignored + ]) + ]), + create_fut("tags", "STRING", "REPEATED"), + ], + ["id", "user_profile", "user_profile.name", "user_profile.addresses", "tags"], + ), + ( + "Empty input list of fields", + [], + [], + ), + ( + "Field type not in STRUCT_FIELD_TYPES and mode is REPEATED", + [ + create_fut("f_arr", "FLOAT", "REPEATED") + ], + ["f_arr"] + ), + ( + "Field type not in STRUCT_FIELD_TYPES and mode is not REPEATED", + [ + create_fut("f_single", "DATE", "NULLABLE") + ], + ["f_single"] + ) +] + + +@pytest.mark.parametrize( + "description, input_fields_list, expected_field_names", + test_cases +) +def test_get_transitive_schema_fields_conditions(description, input_fields_list, expected_field_names): + """ + Tests the _get_transitive_schema_fields function, focusing on the conditional logic + `if field.field_type in STRUCT_FIELD_TYPES and field.mode != "REPEATED":`. + """ + result_fields = _get_transitive_schema_fields(input_fields_list) + result_names = [field.name for field in result_fields] + assert result_names == expected_field_names, description From 2355811f5376bf2f5abee2c55145164e32325e9f Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 9 May 2025 11:23:16 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/test__types.py | 130 ++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 47 deletions(-) diff --git a/tests/unit/test__types.py b/tests/unit/test__types.py index 984746bd..5036d196 100644 --- a/tests/unit/test__types.py +++ b/tests/unit/test__types.py @@ -3,6 +3,7 @@ from sqlalchemy_bigquery._types import _get_transitive_schema_fields, STRUCT_FIELD_TYPES + def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): """ Helper function to create a SchemaField object for testing. @@ -21,77 +22,115 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ( "STRUCT field, not REPEATED, with sub-fields, should recurse", [ - create_fut("s1", "STRUCT", "NULLABLE", sub_fields=[ - create_fut("child1", "STRING", "NULLABLE") - ]) + create_fut( + "s1", + "STRUCT", + "NULLABLE", + sub_fields=[create_fut("child1", "STRING", "NULLABLE")], + ) ], ["s1", "s1.child1"], ), ( "RECORD field (alias for STRUCT), not REPEATED, with sub-fields, should recurse", [ - create_fut("r1", "RECORD", "NULLABLE", sub_fields=[ - create_fut("child_r1", "INTEGER", "NULLABLE") - ]) + create_fut( + "r1", + "RECORD", + "NULLABLE", + sub_fields=[create_fut("child_r1", "INTEGER", "NULLABLE")], + ) ], ["r1", "r1.child_r1"], ), ( "STRUCT field, REPEATED, with sub-fields, should NOT recurse", [ - create_fut("s2", "STRUCT", "REPEATED", sub_fields=[ - create_fut("child2", "STRING", "NULLABLE") - ]) + create_fut( + "s2", + "STRUCT", + "REPEATED", + sub_fields=[create_fut("child2", "STRING", "NULLABLE")], + ) ], ["s2"], ), ( "Non-STRUCT field (STRING), not REPEATED, should NOT recurse", - [ - create_fut("f1", "STRING", "NULLABLE") - ], + [create_fut("f1", "STRING", "NULLABLE")], ["f1"], ), ( "Non-STRUCT field (INTEGER), REPEATED, should NOT recurse", - [ - create_fut("f2", "INTEGER", "REPEATED") - ], + [create_fut("f2", "INTEGER", "REPEATED")], ["f2"], ), ( "Deeply nested STRUCT, not REPEATED, should recurse fully", [ - create_fut("s_outer", "STRUCT", "NULLABLE", sub_fields=[ - create_fut("s_inner1", "STRUCT", "NULLABLE", sub_fields=[ - create_fut("s_leaf1", "STRING", "NULLABLE") - ]), - create_fut("s_sibling", "INTEGER", "NULLABLE"), - create_fut("s_inner2_repeated_struct", "STRUCT", "REPEATED", sub_fields=[ - create_fut("s_leaf2_ignored", "BOOLEAN", "NULLABLE") # This sub-field should be ignored - ]), - ]) + create_fut( + "s_outer", + "STRUCT", + "NULLABLE", + sub_fields=[ + create_fut( + "s_inner1", + "STRUCT", + "NULLABLE", + sub_fields=[create_fut("s_leaf1", "STRING", "NULLABLE")], + ), + create_fut("s_sibling", "INTEGER", "NULLABLE"), + create_fut( + "s_inner2_repeated_struct", + "STRUCT", + "REPEATED", + sub_fields=[ + create_fut( + "s_leaf2_ignored", "BOOLEAN", "NULLABLE" + ) # This sub-field should be ignored + ], + ), + ], + ) + ], + [ + "s_outer", + "s_outer.s_inner1", + "s_outer.s_inner1.s_leaf1", + "s_outer.s_sibling", + "s_outer.s_inner2_repeated_struct", ], - ["s_outer", "s_outer.s_inner1", "s_outer.s_inner1.s_leaf1", "s_outer.s_sibling", "s_outer.s_inner2_repeated_struct"], ), ( "STRUCT field, not REPEATED, but no sub-fields, should not error and not recurse further", - [ - create_fut("s3", "STRUCT", "NULLABLE", sub_fields=[]) - ], + [create_fut("s3", "STRUCT", "NULLABLE", sub_fields=[])], ["s3"], ), ( "Multiple top-level fields with mixed conditions", [ create_fut("id", "INTEGER", "REQUIRED"), - create_fut("user_profile", "STRUCT", "NULLABLE", sub_fields=[ - create_fut("name", "STRING", "NULLABLE"), - create_fut("addresses", "RECORD", "REPEATED", sub_fields=[ # addresses is REPEATED STRUCT - create_fut("street", "STRING", "NULLABLE"), # This sub-field should be ignored - create_fut("city", "STRING", "NULLABLE") # This sub-field should be ignored - ]) - ]), + create_fut( + "user_profile", + "STRUCT", + "NULLABLE", + sub_fields=[ + create_fut("name", "STRING", "NULLABLE"), + create_fut( + "addresses", + "RECORD", + "REPEATED", + sub_fields=[ # addresses is REPEATED STRUCT + create_fut( + "street", "STRING", "NULLABLE" + ), # This sub-field should be ignored + create_fut( + "city", "STRING", "NULLABLE" + ), # This sub-field should be ignored + ], + ), + ], + ), create_fut("tags", "STRING", "REPEATED"), ], ["id", "user_profile", "user_profile.name", "user_profile.addresses", "tags"], @@ -103,26 +142,23 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ), ( "Field type not in STRUCT_FIELD_TYPES and mode is REPEATED", - [ - create_fut("f_arr", "FLOAT", "REPEATED") - ], - ["f_arr"] + [create_fut("f_arr", "FLOAT", "REPEATED")], + ["f_arr"], ), ( "Field type not in STRUCT_FIELD_TYPES and mode is not REPEATED", - [ - create_fut("f_single", "DATE", "NULLABLE") - ], - ["f_single"] - ) + [create_fut("f_single", "DATE", "NULLABLE")], + ["f_single"], + ), ] @pytest.mark.parametrize( - "description, input_fields_list, expected_field_names", - test_cases + "description, input_fields_list, expected_field_names", test_cases ) -def test_get_transitive_schema_fields_conditions(description, input_fields_list, expected_field_names): +def test_get_transitive_schema_fields_conditions( + description, input_fields_list, expected_field_names +): """ Tests the _get_transitive_schema_fields function, focusing on the conditional logic `if field.field_type in STRUCT_FIELD_TYPES and field.mode != "REPEATED":`. From c9839481321af9dbd64d8e3c123a20e55306b40a Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Fri, 9 May 2025 13:28:57 +0000 Subject: [PATCH 3/3] refactor of the test_cases --- tests/unit/test__types.py | 212 +++++++++++++++++++++++++------------- 1 file changed, 138 insertions(+), 74 deletions(-) diff --git a/tests/unit/test__types.py b/tests/unit/test__types.py index 5036d196..ce99c069 100644 --- a/tests/unit/test__types.py +++ b/tests/unit/test__types.py @@ -1,19 +1,35 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import pytest -from google.cloud.bigquery.schema import SchemaField -from sqlalchemy_bigquery._types import _get_transitive_schema_fields, STRUCT_FIELD_TYPES +from sqlalchemy_bigquery._types import _get_transitive_schema_fields +from google.cloud.bigquery.schema import SchemaField -def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): +def create_schema_field_from_dict(schema_dict): """ - Helper function to create a SchemaField object for testing. - `sub_fields` should be a list of already created SchemaField objects. + Helper function to create a SchemaField object from a dictionary representation. """ api_repr = { - "name": name, - "type": field_type, - "mode": mode, - "fields": [sf.to_api_repr() for sf in sub_fields] if sub_fields else [], + "name": schema_dict["name"], + "type": schema_dict["type"], + "mode": schema_dict.get("mode", "NULLABLE"), + "fields": [ + create_schema_field_from_dict(sf_dict).to_api_repr() + for sf_dict in schema_dict.get("fields", []) + ], } return SchemaField.from_api_repr(api_repr) @@ -22,11 +38,15 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ( "STRUCT field, not REPEATED, with sub-fields, should recurse", [ - create_fut( - "s1", - "STRUCT", - "NULLABLE", - sub_fields=[create_fut("child1", "STRING", "NULLABLE")], + create_schema_field_from_dict( + { + "name": "s1", + "type": "STRUCT", + "mode": "NULLABLE", + "fields": [ + {"name": "child1", "type": "STRING", "mode": "NULLABLE"} + ], + } ) ], ["s1", "s1.child1"], @@ -34,11 +54,15 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ( "RECORD field (alias for STRUCT), not REPEATED, with sub-fields, should recurse", [ - create_fut( - "r1", - "RECORD", - "NULLABLE", - sub_fields=[create_fut("child_r1", "INTEGER", "NULLABLE")], + create_schema_field_from_dict( + { + "name": "r1", + "type": "RECORD", + "mode": "NULLABLE", + "fields": [ + {"name": "child_r1", "type": "INTEGER", "mode": "NULLABLE"} + ], + } ) ], ["r1", "r1.child_r1"], @@ -46,51 +70,73 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ( "STRUCT field, REPEATED, with sub-fields, should NOT recurse", [ - create_fut( - "s2", - "STRUCT", - "REPEATED", - sub_fields=[create_fut("child2", "STRING", "NULLABLE")], + create_schema_field_from_dict( + { + "name": "s2", + "type": "STRUCT", + "mode": "REPEATED", + "fields": [ + {"name": "child2", "type": "STRING", "mode": "NULLABLE"} + ], + } ) ], ["s2"], ), ( "Non-STRUCT field (STRING), not REPEATED, should NOT recurse", - [create_fut("f1", "STRING", "NULLABLE")], + [ + create_schema_field_from_dict( + {"name": "f1", "type": "STRING", "mode": "NULLABLE"} + ) + ], ["f1"], ), ( "Non-STRUCT field (INTEGER), REPEATED, should NOT recurse", - [create_fut("f2", "INTEGER", "REPEATED")], + [ + create_schema_field_from_dict( + {"name": "f2", "type": "INTEGER", "mode": "REPEATED"} + ) + ], ["f2"], ), ( "Deeply nested STRUCT, not REPEATED, should recurse fully", [ - create_fut( - "s_outer", - "STRUCT", - "NULLABLE", - sub_fields=[ - create_fut( - "s_inner1", - "STRUCT", - "NULLABLE", - sub_fields=[create_fut("s_leaf1", "STRING", "NULLABLE")], - ), - create_fut("s_sibling", "INTEGER", "NULLABLE"), - create_fut( - "s_inner2_repeated_struct", - "STRUCT", - "REPEATED", - sub_fields=[ - create_fut( - "s_leaf2_ignored", "BOOLEAN", "NULLABLE" - ) # This sub-field should be ignored - ], - ), - ], + create_schema_field_from_dict( + { + "name": "s_outer", + "type": "STRUCT", + "mode": "NULLABLE", + "fields": [ + { + "name": "s_inner1", + "type": "STRUCT", + "mode": "NULLABLE", + "fields": [ + { + "name": "s_leaf1", + "type": "STRING", + "mode": "NULLABLE", + } + ], + }, + {"name": "s_sibling", "type": "INTEGER", "mode": "NULLABLE"}, + { + "name": "s_inner2_repeated_struct", + "type": "STRUCT", + "mode": "REPEATED", + "fields": [ + { + "name": "s_leaf2_ignored", + "type": "BOOLEAN", + "mode": "NULLABLE", + } + ], + }, + ], + } ) ], [ @@ -103,35 +149,45 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ), ( "STRUCT field, not REPEATED, but no sub-fields, should not error and not recurse further", - [create_fut("s3", "STRUCT", "NULLABLE", sub_fields=[])], + [ + create_schema_field_from_dict( + {"name": "s3", "type": "STRUCT", "mode": "NULLABLE", "fields": []} + ) + ], ["s3"], ), ( "Multiple top-level fields with mixed conditions", [ - create_fut("id", "INTEGER", "REQUIRED"), - create_fut( - "user_profile", - "STRUCT", - "NULLABLE", - sub_fields=[ - create_fut("name", "STRING", "NULLABLE"), - create_fut( - "addresses", - "RECORD", - "REPEATED", - sub_fields=[ # addresses is REPEATED STRUCT - create_fut( - "street", "STRING", "NULLABLE" - ), # This sub-field should be ignored - create_fut( - "city", "STRING", "NULLABLE" - ), # This sub-field should be ignored - ], - ), - ], + create_schema_field_from_dict( + {"name": "id", "type": "INTEGER", "mode": "REQUIRED"} + ), + create_schema_field_from_dict( + { + "name": "user_profile", + "type": "STRUCT", + "mode": "NULLABLE", + "fields": [ + {"name": "name", "type": "STRING", "mode": "NULLABLE"}, + { + "name": "addresses", + "type": "RECORD", + "mode": "REPEATED", + "fields": [ + { + "name": "street", + "type": "STRING", + "mode": "NULLABLE", + }, + {"name": "city", "type": "STRING", "mode": "NULLABLE"}, + ], + }, + ], + } + ), + create_schema_field_from_dict( + {"name": "tags", "type": "STRING", "mode": "REPEATED"} ), - create_fut("tags", "STRING", "REPEATED"), ], ["id", "user_profile", "user_profile.name", "user_profile.addresses", "tags"], ), @@ -142,12 +198,20 @@ def create_fut(name, field_type, mode="NULLABLE", sub_fields=None): ), ( "Field type not in STRUCT_FIELD_TYPES and mode is REPEATED", - [create_fut("f_arr", "FLOAT", "REPEATED")], + [ + create_schema_field_from_dict( + {"name": "f_arr", "type": "FLOAT", "mode": "REPEATED"} + ) + ], ["f_arr"], ), ( "Field type not in STRUCT_FIELD_TYPES and mode is not REPEATED", - [create_fut("f_single", "DATE", "NULLABLE")], + [ + create_schema_field_from_dict( + {"name": "f_single", "type": "DATE", "mode": "NULLABLE"} + ) + ], ["f_single"], ), ]