From 9dbc85e8c56663f6fd3083c2b92c9b7954675e7f Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 13 Sep 2026 15:56:08 -0700 Subject: [PATCH 1/3] Fix schema equality for empty schemas and identifier fields Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pyiceberg/schema.py | 5 +---- tests/test_schema.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/pyiceberg/schema.py b/pyiceberg/schema.py index a79b3ae95f..99f983074b 100644 --- a/pyiceberg/schema.py +++ b/pyiceberg/schema.py @@ -115,16 +115,13 @@ def __len__(self) -> int: def __eq__(self, other: Any) -> bool: """Return the equality of two instances of the Schema class.""" - if not other: - return False - if not isinstance(other, Schema): return False if len(self.columns) != len(other.columns): return False - identifier_field_ids_is_equal = self.identifier_field_ids == other.identifier_field_ids + identifier_field_ids_is_equal = set(self.identifier_field_ids) == set(other.identifier_field_ids) schema_is_equal = all(lhs == rhs for lhs, rhs in zip(self.columns, other.columns, strict=True)) return identifier_field_ids_is_equal and schema_is_equal diff --git a/tests/test_schema.py b/tests/test_schema.py index 872e95ce36..355540896f 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -119,6 +119,38 @@ def test_schema_repr_two_fields() -> None: assert expected == actual +def test_empty_schema_equality() -> None: + schema = Schema() + other = Schema(schema_id=99) + nonempty = Schema(NestedField(field_id=1, name="foo", field_type=LongType())) + + assert schema == other + assert other == schema + assert schema != nonempty + assert nonempty != schema + + +@pytest.mark.parametrize("other", [None, False, 0, [], {}, StructType()]) +def test_empty_schema_not_equal_to_non_schema(other: object) -> None: + assert Schema() != other + + +@pytest.mark.parametrize( + ("identifier_field_ids", "expected"), + [([2, 1], True), ([1], False), ([2], False), ([], False)], +) +def test_schema_equality_identifier_fields(identifier_field_ids: list[int], expected: bool) -> None: + fields = ( + NestedField(field_id=1, name="foo", field_type=LongType(), required=True), + NestedField(field_id=2, name="bar", field_type=LongType(), required=True), + ) + schema = Schema(*fields, identifier_field_ids=[1, 2]) + other = Schema(*fields, schema_id=99, identifier_field_ids=identifier_field_ids) + + assert (schema == other) is expected + assert (other == schema) is expected + + def test_schema_raise_on_duplicate_names() -> None: """Test schema representation""" with pytest.raises(ValueError) as exc_info: From 34d55b54a6d410a7d59bfe45193b25a28dacd508 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 13 Sep 2026 16:04:04 -0700 Subject: [PATCH 2/3] Use loops for schema equality regression cases Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_schema.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/test_schema.py b/tests/test_schema.py index 355540896f..fbc2fddeba 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -130,25 +130,27 @@ def test_empty_schema_equality() -> None: assert nonempty != schema -@pytest.mark.parametrize("other", [None, False, 0, [], {}, StructType()]) -def test_empty_schema_not_equal_to_non_schema(other: object) -> None: - assert Schema() != other +def test_empty_schema_not_equal_to_non_schema() -> None: + non_schema_values: list[object] = [None, False, 0, [], {}, StructType()] + for other in non_schema_values: + assert Schema() != other -@pytest.mark.parametrize( - ("identifier_field_ids", "expected"), - [([2, 1], True), ([1], False), ([2], False), ([], False)], -) -def test_schema_equality_identifier_fields(identifier_field_ids: list[int], expected: bool) -> None: +def test_schema_equality_identifier_fields() -> None: fields = ( NestedField(field_id=1, name="foo", field_type=LongType(), required=True), NestedField(field_id=2, name="bar", field_type=LongType(), required=True), ) schema = Schema(*fields, identifier_field_ids=[1, 2]) - other = Schema(*fields, schema_id=99, identifier_field_ids=identifier_field_ids) + other = Schema(*fields, schema_id=99, identifier_field_ids=[2, 1]) + + assert schema == other + assert other == schema - assert (schema == other) is expected - assert (other == schema) is expected + for identifier_field_ids in [[1], [2], []]: + other = Schema(*fields, schema_id=99, identifier_field_ids=identifier_field_ids) + assert schema != other + assert other != schema def test_schema_raise_on_duplicate_names() -> None: From 9ad652aec09c5d5b806558d8056a136c934cfc51 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 13 Sep 2026 16:16:17 -0700 Subject: [PATCH 3/3] Keep schema equality tests focused on regressions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_schema.py | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/tests/test_schema.py b/tests/test_schema.py index fbc2fddeba..f9c928c9e6 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -120,37 +120,15 @@ def test_schema_repr_two_fields() -> None: def test_empty_schema_equality() -> None: - schema = Schema() - other = Schema(schema_id=99) - nonempty = Schema(NestedField(field_id=1, name="foo", field_type=LongType())) + assert Schema() == Schema() - assert schema == other - assert other == schema - assert schema != nonempty - assert nonempty != schema - -def test_empty_schema_not_equal_to_non_schema() -> None: - non_schema_values: list[object] = [None, False, 0, [], {}, StructType()] - for other in non_schema_values: - assert Schema() != other - - -def test_schema_equality_identifier_fields() -> None: +def test_schema_equality_ignores_identifier_field_order() -> None: fields = ( NestedField(field_id=1, name="foo", field_type=LongType(), required=True), NestedField(field_id=2, name="bar", field_type=LongType(), required=True), ) - schema = Schema(*fields, identifier_field_ids=[1, 2]) - other = Schema(*fields, schema_id=99, identifier_field_ids=[2, 1]) - - assert schema == other - assert other == schema - - for identifier_field_ids in [[1], [2], []]: - other = Schema(*fields, schema_id=99, identifier_field_ids=identifier_field_ids) - assert schema != other - assert other != schema + assert Schema(*fields, identifier_field_ids=[1, 2]) == Schema(*fields, identifier_field_ids=[2, 1]) def test_schema_raise_on_duplicate_names() -> None: