From e27cd9095503cfe9fa7e0a806ba25d42920c68c5 Mon Sep 17 00:00:00 2001
From: "Yair Halevi (Spock)" <118175475+spock-abadai@users.noreply.github.com>
Date: Sun, 14 Jul 2024 22:11:04 +0300
Subject: [PATCH] Allow empty `names` in mapped field of Name Mapping (#927)

* 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.

* Remove irrelevant test case

* Fixing pydantic model

No longer requiring minimum length of names list to be 1.

* Added test case for empty names in name mapping

* Fixed formatting error
---
 pyiceberg/table/name_mapping.py  | 14 +-------------
 tests/table/test_name_mapping.py | 22 +++++++++++++++++-----
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/pyiceberg/table/name_mapping.py b/pyiceberg/table/name_mapping.py
index 5a4e76900..cb9f72bf9 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")
@@ -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."""
diff --git a/tests/table/test_name_mapping.py b/tests/table/test_name_mapping.py
index d4a2bf6c4..3c50a24e5 100644
--- a/tests/table/test_name_mapping.py
+++ b/tests/table/test_name_mapping.py
@@ -91,6 +91,23 @@ 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 = """
 [
@@ -247,11 +264,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