Skip to content

Commit b8b8c53

Browse files
kevinjqliuCopilot
andauthored
Add tests for update_column changing list/map optionality (#2950)
<!-- Thanks for opening a pull request! --> <!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. --> <!-- Closes #${GITHUB_ISSUE_ID} --> # Rationale for this change Follow up to #2798 More test coverages ## Are these changes tested? ## Are there any user-facing changes? <!-- In the case of user-facing changes, please add the changelog label. --> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 79a0557 commit b8b8c53

File tree

1 file changed

+169
-8
lines changed

1 file changed

+169
-8
lines changed

tests/table/test_init.py

Lines changed: 169 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -513,8 +513,7 @@ def test_update_list_element_required(table_v2: Table) -> None:
513513
"""Test that update_column can change list element's required property."""
514514
# Add a list column with optional elements
515515
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)
516+
update.add_column(path="tags", field_type=ListType(element_id=1, element_type=StringType(), element_required=False))
518517
schema_with_list = update._apply()
519518

520519
# Verify initial state
@@ -523,8 +522,7 @@ def test_update_list_element_required(table_v2: Table) -> None:
523522
assert field.field_type.element_required is False
524523

525524
# Update element to required
526-
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_list)
527-
update2._allow_incompatible_changes = True # Allow optional -> required
525+
update2 = UpdateSchema(transaction=table_v2.transaction(), allow_incompatible_changes=True, schema=schema_with_list)
528526
new_schema = update2.update_column(("tags", "element"), required=True)._apply()
529527

530528
# Verify the update
@@ -537,8 +535,10 @@ def test_update_map_value_required(table_v2: Table) -> None:
537535
"""Test that update_column can change map value's required property."""
538536
# Add a map column with optional values
539537
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)
538+
update.add_column(
539+
path="metadata",
540+
field_type=MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False),
541+
)
542542
schema_with_map = update._apply()
543543

544544
# Verify initial state
@@ -547,8 +547,7 @@ def test_update_map_value_required(table_v2: Table) -> None:
547547
assert field.field_type.value_required is False
548548

549549
# Update value to required
550-
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_map)
551-
update2._allow_incompatible_changes = True # Allow optional -> required
550+
update2 = UpdateSchema(transaction=table_v2.transaction(), allow_incompatible_changes=True, schema=schema_with_map)
552551
new_schema = update2.update_column(("metadata", "value"), required=True)._apply()
553552

554553
# Verify the update
@@ -557,6 +556,168 @@ def test_update_map_value_required(table_v2: Table) -> None:
557556
assert field.field_type.value_required is True
558557

559558

559+
def test_update_list_element_required_to_optional(table_v2: Table) -> None:
560+
"""Test that update_column can change list element from required to optional (safe direction)."""
561+
# Add a list column with required elements
562+
update = UpdateSchema(transaction=table_v2.transaction())
563+
update.add_column(path="tags", field_type=ListType(element_id=1, element_type=StringType(), element_required=True))
564+
schema_with_list = update._apply()
565+
566+
# Verify initial state
567+
field = schema_with_list.find_field("tags")
568+
assert isinstance(field.field_type, ListType)
569+
assert field.field_type.element_required is True
570+
571+
# Update element to optional - should work without allow_incompatible_changes
572+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_list)
573+
new_schema = update2.update_column(("tags", "element"), required=False)._apply()
574+
575+
# Verify the update
576+
field = new_schema.find_field("tags")
577+
assert isinstance(field.field_type, ListType)
578+
assert field.field_type.element_required is False
579+
580+
581+
def test_update_list_element_required_fails_without_allow_incompatible(table_v2: Table) -> None:
582+
"""Test that optional -> required fails without allow_incompatible_changes."""
583+
# Add a list column with optional elements
584+
update = UpdateSchema(transaction=table_v2.transaction())
585+
update.add_column(path="tags", field_type=ListType(element_id=1, element_type=StringType(), element_required=False))
586+
schema_with_list = update._apply()
587+
588+
# Try to update element to required without allow_incompatible_changes - should fail
589+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_list)
590+
with pytest.raises(ValueError, match="Cannot change column nullability"):
591+
update2.update_column(("tags", "element"), required=True)._apply()
592+
593+
594+
def test_update_map_value_required_fails_without_allow_incompatible(table_v2: Table) -> None:
595+
"""Test that optional -> required for map value fails without allow_incompatible_changes."""
596+
# Add a map column with optional values
597+
update = UpdateSchema(transaction=table_v2.transaction())
598+
update.add_column(
599+
path="metadata",
600+
field_type=MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False),
601+
)
602+
schema_with_map = update._apply()
603+
604+
# Try to update value to required without allow_incompatible_changes - should fail
605+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_map)
606+
with pytest.raises(ValueError, match="Cannot change column nullability"):
607+
update2.update_column(("metadata", "value"), required=True)._apply()
608+
609+
610+
def test_update_map_key_fails(table_v2: Table) -> None:
611+
"""Test that updating map keys is not allowed."""
612+
# Add a map column
613+
update = UpdateSchema(transaction=table_v2.transaction())
614+
update.add_column(
615+
path="metadata",
616+
field_type=MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False),
617+
)
618+
schema_with_map = update._apply()
619+
620+
# Try to update the key - should fail even with allow_incompatible_changes
621+
update2 = UpdateSchema(transaction=table_v2.transaction(), allow_incompatible_changes=True, schema=schema_with_map)
622+
with pytest.raises(ValueError, match="Cannot update map keys"):
623+
update2.update_column(("metadata", "key"), required=False)._apply()
624+
625+
626+
def test_update_map_value_required_to_optional(table_v2: Table) -> None:
627+
"""Test that update_column can change map value from required to optional (safe direction)."""
628+
# Add a map column with required values
629+
update = UpdateSchema(transaction=table_v2.transaction())
630+
update.add_column(
631+
path="metadata",
632+
field_type=MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=True),
633+
)
634+
schema_with_map = update._apply()
635+
636+
# Verify initial state
637+
field = schema_with_map.find_field("metadata")
638+
assert isinstance(field.field_type, MapType)
639+
assert field.field_type.value_required is True
640+
641+
# Update value to optional - should work without allow_incompatible_changes
642+
update2 = UpdateSchema(transaction=table_v2.transaction(), schema=schema_with_map)
643+
new_schema = update2.update_column(("metadata", "value"), required=False)._apply()
644+
645+
# Verify the update
646+
field = new_schema.find_field("metadata")
647+
assert isinstance(field.field_type, MapType)
648+
assert field.field_type.value_required is False
649+
650+
651+
def test_update_list_and_map_in_single_schema_change(table_v2: Table) -> None:
652+
"""Test updating both list element and map value required properties in a single schema change."""
653+
# Add both a list and a map column with optional elements/values
654+
update = UpdateSchema(transaction=table_v2.transaction())
655+
update.add_column(path="tags", field_type=ListType(element_id=1, element_type=StringType(), element_required=False))
656+
update.add_column(
657+
path="metadata",
658+
field_type=MapType(key_id=1, key_type=StringType(), value_id=2, value_type=IntegerType(), value_required=False),
659+
)
660+
schema_with_both = update._apply()
661+
662+
# Verify initial state
663+
list_field = schema_with_both.find_field("tags")
664+
assert isinstance(list_field.field_type, ListType)
665+
assert list_field.field_type.element_required is False
666+
667+
map_field = schema_with_both.find_field("metadata")
668+
assert isinstance(map_field.field_type, MapType)
669+
assert map_field.field_type.value_required is False
670+
671+
# Update both in a single schema change
672+
update2 = UpdateSchema(transaction=table_v2.transaction(), allow_incompatible_changes=True, schema=schema_with_both)
673+
update2.update_column(("tags", "element"), required=True)
674+
update2.update_column(("metadata", "value"), required=True)
675+
new_schema = update2._apply()
676+
677+
# Verify both updates
678+
list_field = new_schema.find_field("tags")
679+
assert isinstance(list_field.field_type, ListType)
680+
assert list_field.field_type.element_required is True
681+
682+
map_field = new_schema.find_field("metadata")
683+
assert isinstance(map_field.field_type, MapType)
684+
assert map_field.field_type.value_required is True
685+
686+
687+
def test_update_nested_list_in_struct_required(table_v2: Table) -> None:
688+
"""Test updating nested list element required property inside a struct."""
689+
690+
# Add a struct column containing a list
691+
update = UpdateSchema(transaction=table_v2.transaction())
692+
struct_type = StructType(
693+
NestedField(
694+
field_id=1,
695+
name="coordinates",
696+
field_type=ListType(element_id=2, element_type=DoubleType(), element_required=False),
697+
required=False,
698+
)
699+
)
700+
update.add_column(path="location", field_type=struct_type)
701+
schema_with_nested = update._apply()
702+
703+
# Verify initial state
704+
field = schema_with_nested.find_field("location")
705+
assert isinstance(field.field_type, StructType)
706+
nested_list = field.field_type.fields[0].field_type
707+
assert isinstance(nested_list, ListType)
708+
assert nested_list.element_required is False
709+
710+
# Update nested list element to required
711+
update2 = UpdateSchema(transaction=table_v2.transaction(), allow_incompatible_changes=True, schema=schema_with_nested)
712+
new_schema = update2.update_column(("location", "coordinates", "element"), required=True)._apply()
713+
714+
# Verify the update
715+
field = new_schema.find_field("location")
716+
nested_list = field.field_type.fields[0].field_type
717+
assert isinstance(nested_list, ListType)
718+
assert nested_list.element_required is True
719+
720+
560721
def test_apply_set_properties_update(table_v2: Table) -> None:
561722
base_metadata = table_v2.metadata
562723

0 commit comments

Comments
 (0)