Skip to content

Commit af17de8

Browse files
authored
fix: Allow update_column to change required for list elements and map values (#2798)
## Summary - Fixes #2786: `update_schema().update_column()` was not updating the `required` property for list elements or map values - Modified `_ApplyChanges.list()` to check `self._updates` for element's required property - Modified `_ApplyChanges.map()` to check `self._updates` for value's required property - Added 2 unit tests for list element and map value required updates ## Test plan - [x] All 3032 unit tests pass - [x] Lint passes - [x] New tests cover: updating list element required, updating map value required Closes #2786 Co-authored-by: Somasundaram Sekar <somasundaramsekar.1986@gmail.com>
1 parent bad9cda commit af17de8

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
@@ -509,6 +509,54 @@ def test_add_nested_list_type_column(table_v2: Table) -> None:
509509
assert new_schema.highest_field_id == 7
510510

511511

512+
def test_update_list_element_required(table_v2: Table) -> None:
513+
"""Test that update_column can change list element's required property."""
514+
# Add a list column with optional elements
515+
update = UpdateSchema(transaction=table_v2.transaction())
516+
list_type = ListType(element_id=1, element_type=StringType(), element_required=False)
517+
update.add_column(path="tags", field_type=list_type)
518+
schema_with_list = update._apply()
519+
520+
# Verify initial state
521+
field = schema_with_list.find_field("tags")
522+
assert isinstance(field.field_type, ListType)
523+
assert field.field_type.element_required is False
524+
525+
# Update element to required
526+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_list)
527+
update2._allow_incompatible_changes = True # Allow optional -> required
528+
new_schema = update2.update_column(("tags", "element"), required=True)._apply()
529+
530+
# Verify the update
531+
field = new_schema.find_field("tags")
532+
assert isinstance(field.field_type, ListType)
533+
assert field.field_type.element_required is True
534+
535+
536+
def test_update_map_value_required(table_v2: Table) -> None:
537+
"""Test that update_column can change map value's required property."""
538+
# Add a map column with optional values
539+
update = UpdateSchema(transaction=table_v2.transaction())
540+
map_type = MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False)
541+
update.add_column(path="metadata", field_type=map_type)
542+
schema_with_map = update._apply()
543+
544+
# Verify initial state
545+
field = schema_with_map.find_field("metadata")
546+
assert isinstance(field.field_type, MapType)
547+
assert field.field_type.value_required is False
548+
549+
# Update value to required
550+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_map)
551+
update2._allow_incompatible_changes = True # Allow optional -> required
552+
new_schema = update2.update_column(("metadata", "value"), required=True)._apply()
553+
554+
# Verify the update
555+
field = new_schema.find_field("metadata")
556+
assert isinstance(field.field_type, MapType)
557+
assert field.field_type.value_required is True
558+
559+
512560
def test_apply_set_properties_update(table_v2: Table) -> None:
513561
base_metadata = table_v2.metadata
514562

0 commit comments

Comments
 (0)