Skip to content

Commit 3bd24b1

Browse files
author
Somasundaram Sekar
committed
fix: Allow update_column to change required for list elements and map values
Closes #2786 The `update_schema().update_column()` method was not updating the `required` property for list elements or map values. The `_ApplyChanges` visitor's `list()` and `map()` methods always used the original `element_required` and `value_required` values without checking for updates in `self._updates`. This fix modifies both methods to check `self._updates` for the element/value field ID and use the updated `required` value if present.
1 parent 65ba595 commit 3bd24b1

File tree

2 files changed

+58
-2
lines changed

2 files changed

+58
-2
lines changed

pyiceberg/table/update/schema.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,11 @@ def list(self, list_type: ListType, element_result: IcebergType | None) -> Icebe
805805
if element_type is None:
806806
raise ValueError(f"Cannot delete element type from list: {element_result}")
807807

808-
return ListType(element_id=list_type.element_id, element=element_type, element_required=list_type.element_required)
808+
element_required = list_type.element_required
809+
if update := self._updates.get(list_type.element_id):
810+
element_required = update.required
811+
812+
return ListType(element_id=list_type.element_id, element=element_type, element_required=element_required)
809813

810814
def map(self, map_type: MapType, key_result: IcebergType | None, value_result: IcebergType | None) -> IcebergType | None:
811815
key_id: int = map_type.key_field.field_id
@@ -827,12 +831,16 @@ def map(self, map_type: MapType, key_result: IcebergType | None, value_result: I
827831
if value_type is None:
828832
raise ValueError(f"Cannot delete value type from map: {value_field}")
829833

834+
value_required = map_type.value_required
835+
if update := self._updates.get(map_type.value_id):
836+
value_required = update.required
837+
830838
return MapType(
831839
key_id=map_type.key_id,
832840
key_type=map_type.key_type,
833841
value_id=map_type.value_id,
834842
value_type=value_type,
835-
value_required=map_type.value_required,
843+
value_required=value_required,
836844
)
837845

838846
def primitive(self, primitive: PrimitiveType) -> IcebergType | None:

tests/table/test_init.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,54 @@ def test_add_nested_list_type_column(table_v2: Table) -> None:
627627
assert new_schema.highest_field_id == 7
628628

629629

630+
def test_update_list_element_required(table_v2: Table) -> None:
631+
"""Test that update_column can change list element's required property."""
632+
# Add a list column with optional elements
633+
update = UpdateSchema(transaction=table_v2.transaction())
634+
list_type = ListType(element_id=1, element_type=StringType(), element_required=False)
635+
update.add_column(path="tags", field_type=list_type)
636+
schema_with_list = update._apply()
637+
638+
# Verify initial state
639+
field = schema_with_list.find_field("tags")
640+
assert isinstance(field.field_type, ListType)
641+
assert field.field_type.element_required is False
642+
643+
# Update element to required
644+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_list)
645+
update2._allow_incompatible_changes = True # Allow optional -> required
646+
new_schema = update2.update_column(("tags", "element"), required=True)._apply()
647+
648+
# Verify the update
649+
field = new_schema.find_field("tags")
650+
assert isinstance(field.field_type, ListType)
651+
assert field.field_type.element_required is True
652+
653+
654+
def test_update_map_value_required(table_v2: Table) -> None:
655+
"""Test that update_column can change map value's required property."""
656+
# Add a map column with optional values
657+
update = UpdateSchema(transaction=table_v2.transaction())
658+
map_type = MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False)
659+
update.add_column(path="metadata", field_type=map_type)
660+
schema_with_map = update._apply()
661+
662+
# Verify initial state
663+
field = schema_with_map.find_field("metadata")
664+
assert isinstance(field.field_type, MapType)
665+
assert field.field_type.value_required is False
666+
667+
# Update value to required
668+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_map)
669+
update2._allow_incompatible_changes = True # Allow optional -> required
670+
new_schema = update2.update_column(("metadata", "value"), required=True)._apply()
671+
672+
# Verify the update
673+
field = new_schema.find_field("metadata")
674+
assert isinstance(field.field_type, MapType)
675+
assert field.field_type.value_required is True
676+
677+
630678
def test_apply_set_properties_update(table_v2: Table) -> None:
631679
base_metadata = table_v2.metadata
632680

0 commit comments

Comments
 (0)