From c9bfa271bc8b51e70f7efabdeb517ebbab6ac993 Mon Sep 17 00:00:00 2001 From: "Yair Halevi (Spock)" Date: Sun, 14 Jul 2024 00:00:08 +0300 Subject: [PATCH 1/5] Remove check_at_least_one field validator Iceberg spec permits an emtpy list of names in the default name mapping. check_at_least_one is therefore unnecessary. --- pyiceberg/table/name_mapping.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pyiceberg/table/name_mapping.py b/pyiceberg/table/name_mapping.py index 5a4e769003..1a23ea39b7 100644 --- a/pyiceberg/table/name_mapping.py +++ b/pyiceberg/table/name_mapping.py @@ -45,18 +45,6 @@ class MappedField(IcebergBaseModel): def convert_null_to_empty_List(cls, v: Any) -> Any: return v or [] - @field_validator("names", mode="after") - @classmethod - def check_at_least_one(cls, v: List[str]) -> Any: - """ - Conlist constraint does not seem to be validating the class on instantiation. - - Adding a custom validator to enforce min_length=1 constraint. - """ - if len(v) < 1: - raise ValueError("At least one mapped name must be provided for the field") - return v - @model_serializer def ser_model(self) -> Dict[str, Any]: """Set custom serializer to leave out the field when it is empty.""" From 15b05c756f3265f08a7c6b4b543a06b6158cbbb0 Mon Sep 17 00:00:00 2001 From: "Yair Halevi (Spock)" Date: Sun, 14 Jul 2024 00:36:07 +0300 Subject: [PATCH 2/5] Remove irrelevant test case --- tests/table/test_name_mapping.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/table/test_name_mapping.py b/tests/table/test_name_mapping.py index d4a2bf6c41..ae8527c48c 100644 --- a/tests/table/test_name_mapping.py +++ b/tests/table/test_name_mapping.py @@ -247,11 +247,6 @@ def test_mapping_lookup_by_name(table_name_mapping_nested: NameMapping) -> None: table_name_mapping_nested.find("boom") -def test_invalid_mapped_field() -> None: - with pytest.raises(ValueError): - MappedField(field_id=1, names=[]) - - def test_update_mapping_no_updates_or_adds(table_name_mapping_nested: NameMapping) -> None: assert update_mapping(table_name_mapping_nested, {}, {}) == table_name_mapping_nested From 12c35913b489d458d3d920fff33846a89b9a67c6 Mon Sep 17 00:00:00 2001 From: "Yair Halevi (Spock)" Date: Sun, 14 Jul 2024 08:51:50 +0300 Subject: [PATCH 3/5] Fixing pydantic model No longer requiring minimum length of names list to be 1. --- pyiceberg/table/name_mapping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyiceberg/table/name_mapping.py b/pyiceberg/table/name_mapping.py index 1a23ea39b7..cb9f72bf97 100644 --- a/pyiceberg/table/name_mapping.py +++ b/pyiceberg/table/name_mapping.py @@ -37,7 +37,7 @@ class MappedField(IcebergBaseModel): field_id: int = Field(alias="field-id") - names: List[str] = conlist(str, min_length=1) + names: List[str] = conlist(str) fields: List[MappedField] = Field(default_factory=list) @field_validator("fields", mode="before") From f7ef81c627d0dcc31f39f4929ad6c8c9d722bee0 Mon Sep 17 00:00:00 2001 From: "Yair Halevi (Spock)" Date: Sun, 14 Jul 2024 09:05:36 +0300 Subject: [PATCH 4/5] Added test case for empty names in name mapping --- tests/table/test_name_mapping.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/table/test_name_mapping.py b/tests/table/test_name_mapping.py index ae8527c48c..6fb39a5ec1 100644 --- a/tests/table/test_name_mapping.py +++ b/tests/table/test_name_mapping.py @@ -90,6 +90,22 @@ def test_json_mapped_field_deserialization() -> None: """ assert MappedField(field_id=1, names=["id", "record_id"]) == MappedField.model_validate_json(mapped_field_with_null_fields) +def test_json_mapped_field_no_names_deserialization() -> None: + mapped_field = """{ + "field-id": 1, + "names": [] + } + """ + assert MappedField(field_id=1, names=[]) == MappedField.model_validate_json(mapped_field) + + mapped_field_with_null_fields = """{ + "field-id": 1, + "names": [], + "fields": null + } + """ + assert MappedField(field_id=1, names=[]) == MappedField.model_validate_json(mapped_field_with_null_fields) + def test_json_name_mapping_deserialization() -> None: name_mapping = """ From 9bea754af6d2d4456160721cfc0724bc89783970 Mon Sep 17 00:00:00 2001 From: "Yair Halevi (Spock)" Date: Sun, 14 Jul 2024 12:02:06 +0300 Subject: [PATCH 5/5] Fixed formatting error --- tests/table/test_name_mapping.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/table/test_name_mapping.py b/tests/table/test_name_mapping.py index 6fb39a5ec1..3c50a24e5e 100644 --- a/tests/table/test_name_mapping.py +++ b/tests/table/test_name_mapping.py @@ -90,6 +90,7 @@ def test_json_mapped_field_deserialization() -> None: """ assert MappedField(field_id=1, names=["id", "record_id"]) == MappedField.model_validate_json(mapped_field_with_null_fields) + def test_json_mapped_field_no_names_deserialization() -> None: mapped_field = """{ "field-id": 1,