From c28d868b25079fe3bb48e595a7442a0d4a146567 Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Fri, 31 Jul 2026 17:50:30 +0100 Subject: [PATCH 1/6] Index snapshots by ID for snapshot_by_id lookups snapshot_by_id did a linear scan over the snapshots list, and is called once per manifest entry, making inspect.partitions() O(data_files x snapshots). Memoize an id-to-snapshot index instead. A cached_property is not usable here: model_copy carries __dict__ over, so a copy replacing the snapshots would inherit a stale index. The index is tied to the list it was built from and recomputed whenever snapshots is a different list. --- pyiceberg/table/metadata.py | 19 +++++++++++++- tests/table/test_metadata.py | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index 26b6e3d3ad..430f29d3ce 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -236,9 +236,26 @@ class TableMetadataCommonFields(IcebergBaseModel): def transform_properties_dict_value_to_str(cls, properties: Properties) -> dict[str, str]: return transform_dict_value_to_str(properties) + @property + def _lazy_id_to_snapshot(self) -> dict[int, Snapshot]: + """Return an index of snapshot ID to Snapshot instance. + + This is calculated once per snapshots list and cached. A plain `cached_property` cannot be + used here: `model_copy` carries `__dict__` over to the new instance, so a copy that replaces + the snapshots would inherit a stale index. The index is therefore tied to the list it was + built from, and recomputed whenever `snapshots` is a different list. Keeping a reference to + that list also keeps it alive, so its identity cannot be reused by another object. + """ + cached = self.__dict__.get("_id_to_snapshot") + if cached is None or cached[0] is not self.snapshots: + cached = (self.snapshots, {snapshot.snapshot_id: snapshot for snapshot in self.snapshots}) + # The model is frozen, so bypass the pydantic __setattr__ to memoize. + object.__setattr__(self, "_id_to_snapshot", cached) + return cached[1] + def snapshot_by_id(self, snapshot_id: int) -> Snapshot | None: """Get the snapshot by snapshot_id.""" - return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None) + return self._lazy_id_to_snapshot.get(snapshot_id) def schema_by_id(self, schema_id: int) -> Schema | None: """Get the schema by schema_id.""" diff --git a/tests/table/test_metadata.py b/tests/table/test_metadata.py index c163c90626..3583296791 100644 --- a/tests/table/test_metadata.py +++ b/tests/table/test_metadata.py @@ -37,6 +37,7 @@ new_table_metadata, ) from pyiceberg.table.refs import SnapshotRef, SnapshotRefType +from pyiceberg.table.snapshots import Operation, Snapshot, Summary from pyiceberg.table.sorting import NullOrder, SortDirection, SortField, SortOrder from pyiceberg.transforms import IdentityTransform from pyiceberg.typedef import UTF8 @@ -145,6 +146,56 @@ def test_parsing_correct_types(example_table_metadata_v2: dict[str, Any]) -> Non assert isinstance(table_metadata.schemas[0].fields[0].field_type, LongType) +def test_snapshot_by_id(example_table_metadata_v2: dict[str, Any]) -> None: + table_metadata = TableMetadataV2(**example_table_metadata_v2) + + # Returns the same instance that is in the snapshots list, not a copy + assert table_metadata.snapshot_by_id(3051729675574597004) is table_metadata.snapshots[0] + assert table_metadata.snapshot_by_id(3055729675574597004) is table_metadata.snapshots[1] + assert table_metadata.snapshot_by_id(-1) is None + + +def test_snapshot_by_id_index_is_invalidated_on_model_copy(example_table_metadata_v2: dict[str, Any]) -> None: + """The snapshot lookup index must not survive a model_copy that replaces the snapshots.""" + table_metadata = TableMetadataV2(**example_table_metadata_v2) + + # Build the index before copying, so a stale one would be carried over + assert table_metadata.snapshot_by_id(3051729675574597004) is not None + + new_snapshot = Snapshot( + snapshot_id=1, + parent_snapshot_id=3055729675574597004, + sequence_number=35, + timestamp_ms=1602638573591, + manifest_list="s3://bucket/test/manifest-list", + summary=Summary(Operation.APPEND), + schema_id=1, + ) + with_added = table_metadata.model_copy(update={"snapshots": table_metadata.snapshots + [new_snapshot]}) + assert with_added.snapshot_by_id(1) is new_snapshot + assert with_added.snapshot_by_id(3051729675574597004) is not None + + without_first = with_added.model_copy(update={"snapshots": with_added.snapshots[1:]}) + assert without_first.snapshot_by_id(3051729675574597004) is None + assert without_first.snapshot_by_id(1) is new_snapshot + + # The original is unaffected by either copy + assert table_metadata.snapshot_by_id(1) is None + assert table_metadata.snapshot_by_id(3051729675574597004) is not None + + +def test_snapshot_by_id_index_is_not_serialized(example_table_metadata_v2: dict[str, Any]) -> None: + """The memoized index is an implementation detail and must stay out of the serialized form.""" + table_metadata = TableMetadataV2(**example_table_metadata_v2) + assert table_metadata.snapshot_by_id(3051729675574597004) is not None + + assert "_id_to_snapshot" not in table_metadata.model_dump() + assert "_id_to_snapshot" not in table_metadata.model_dump_json() + assert "_id_to_snapshot" not in TableMetadataV2.model_fields + # Two equal instances stay equal when only one of them has built the index + assert table_metadata == TableMetadataV2(**example_table_metadata_v2) + + def test_updating_metadata(example_table_metadata_v2: dict[str, Any]) -> None: """Test creating a new TableMetadata instance that's an updated version of an existing TableMetadata instance""" From 7b122f64148c302f2fc07fc8591230bf4002f99a Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Fri, 31 Jul 2026 17:51:27 +0100 Subject: [PATCH 2/6] reducing comments to follow the standard --- pyiceberg/table/metadata.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index 430f29d3ce..ac5c3ca1eb 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -238,14 +238,7 @@ def transform_properties_dict_value_to_str(cls, properties: Properties) -> dict[ @property def _lazy_id_to_snapshot(self) -> dict[int, Snapshot]: - """Return an index of snapshot ID to Snapshot instance. - - This is calculated once per snapshots list and cached. A plain `cached_property` cannot be - used here: `model_copy` carries `__dict__` over to the new instance, so a copy that replaces - the snapshots would inherit a stale index. The index is therefore tied to the list it was - built from, and recomputed whenever `snapshots` is a different list. Keeping a reference to - that list also keeps it alive, so its identity cannot be reused by another object. - """ + """Return an index of snapshot ID to Snapshot instance.This is calculated once per snapshots list and cached.""" cached = self.__dict__.get("_id_to_snapshot") if cached is None or cached[0] is not self.snapshots: cached = (self.snapshots, {snapshot.snapshot_id: snapshot for snapshot in self.snapshots}) From fbec3a5902f0e863b66f3793a4da9a7177e0e6f9 Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Mon, 3 Aug 2026 10:26:13 +0100 Subject: [PATCH 3/6] Cache snapshot positions so in-place list mutation is picked up Caching Snapshot instances went stale when the snapshots list was mutated in place: `table.snapshots()` returns the live list, so replacing an entry and passing the same list to `model_copy` did not change its identity and the index was never rebuilt. Cache positions instead and validate on read, falling back to a scan when they no longer line up. Adds a regression test for in-place mutation. --- pyiceberg/table/metadata.py | 31 +++++++++++++++++++++++++------ tests/table/test_metadata.py | 29 ++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index ac5c3ca1eb..b6440da905 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -237,18 +237,37 @@ def transform_properties_dict_value_to_str(cls, properties: Properties) -> dict[ return transform_dict_value_to_str(properties) @property - def _lazy_id_to_snapshot(self) -> dict[int, Snapshot]: - """Return an index of snapshot ID to Snapshot instance.This is calculated once per snapshots list and cached.""" - cached = self.__dict__.get("_id_to_snapshot") + def _lazy_id_to_snapshot_position(self) -> dict[int, int]: + """Return an index of snapshot ID to its position in the snapshots list. + + Positions are cached rather than Snapshot instances, so that replacing an entry of the list + in place is still picked up by the next lookup. + + A plain `cached_property` cannot be used here: `model_copy` carries `__dict__` over to the + new instance, so a copy that replaces the snapshots would inherit a stale index. The index is + tied to the list it was built from, and rebuilt whenever `snapshots` is a different list. + Keeping a reference to that list also keeps it alive, so its identity cannot be reused by + another object. + """ + cached = self.__dict__.get("_id_to_snapshot_position") if cached is None or cached[0] is not self.snapshots: - cached = (self.snapshots, {snapshot.snapshot_id: snapshot for snapshot in self.snapshots}) + cached = ( + self.snapshots, + {snapshot.snapshot_id: position for position, snapshot in enumerate(self.snapshots)}, + ) # The model is frozen, so bypass the pydantic __setattr__ to memoize. - object.__setattr__(self, "_id_to_snapshot", cached) + object.__setattr__(self, "_id_to_snapshot_position", cached) return cached[1] def snapshot_by_id(self, snapshot_id: int) -> Snapshot | None: """Get the snapshot by snapshot_id.""" - return self._lazy_id_to_snapshot.get(snapshot_id) + snapshots = self.snapshots + if (position := self._lazy_id_to_snapshot_position.get(snapshot_id)) is not None and position < len(snapshots): + if (snapshot := snapshots[position]).snapshot_id == snapshot_id: + return snapshot + # Either the id is absent, or the list was mutated in place and the cached positions + # no longer line up. Fall back to a scan, which is always correct. + return next((snapshot for snapshot in snapshots if snapshot.snapshot_id == snapshot_id), None) def schema_by_id(self, schema_id: int) -> Schema | None: """Get the schema by schema_id.""" diff --git a/tests/table/test_metadata.py b/tests/table/test_metadata.py index 3583296791..2372ff8e1a 100644 --- a/tests/table/test_metadata.py +++ b/tests/table/test_metadata.py @@ -184,14 +184,37 @@ def test_snapshot_by_id_index_is_invalidated_on_model_copy(example_table_metadat assert table_metadata.snapshot_by_id(3051729675574597004) is not None +def test_snapshot_by_id_reflects_in_place_snapshot_list_mutation(example_table_metadata_v2: dict[str, Any]) -> None: + """The snapshot lookup must not go stale when the snapshots list itself is mutated in place.""" + table_metadata = TableMetadataV2(**example_table_metadata_v2) + snapshot_id = 3051729675574597004 + + # Build the index before mutating, so a stale one would still be in place + assert table_metadata.snapshot_by_id(snapshot_id) is not None + + # Replacing an entry: the lookup must return the new instance, not the replaced one + altered = table_metadata.snapshots[0].model_copy(update={"summary": Summary(Operation.DELETE)}) + table_metadata.snapshots[0] = altered + assert table_metadata.snapshot_by_id(snapshot_id) is altered + + # Reordering: ids still resolve to the right snapshots + table_metadata.snapshots.reverse() + assert table_metadata.snapshot_by_id(snapshot_id) is altered + assert table_metadata.snapshot_by_id(3055729675574597004) is table_metadata.snapshots[0] + + # Removing: the id is gone + table_metadata.snapshots.clear() + assert table_metadata.snapshot_by_id(snapshot_id) is None + + def test_snapshot_by_id_index_is_not_serialized(example_table_metadata_v2: dict[str, Any]) -> None: """The memoized index is an implementation detail and must stay out of the serialized form.""" table_metadata = TableMetadataV2(**example_table_metadata_v2) assert table_metadata.snapshot_by_id(3051729675574597004) is not None - assert "_id_to_snapshot" not in table_metadata.model_dump() - assert "_id_to_snapshot" not in table_metadata.model_dump_json() - assert "_id_to_snapshot" not in TableMetadataV2.model_fields + assert "_id_to_snapshot_position" not in table_metadata.model_dump() + assert "_id_to_snapshot_position" not in table_metadata.model_dump_json() + assert "_id_to_snapshot_position" not in TableMetadataV2.model_fields # Two equal instances stay equal when only one of them has built the index assert table_metadata == TableMetadataV2(**example_table_metadata_v2) From 3e9cea4495bd81447c8776adaeb6222174e29d26 Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Mon, 3 Aug 2026 10:31:25 +0100 Subject: [PATCH 4/6] reducing comments to follow the standard --- pyiceberg/table/metadata.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index b6440da905..0565678c4f 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -240,14 +240,7 @@ def transform_properties_dict_value_to_str(cls, properties: Properties) -> dict[ def _lazy_id_to_snapshot_position(self) -> dict[int, int]: """Return an index of snapshot ID to its position in the snapshots list. - Positions are cached rather than Snapshot instances, so that replacing an entry of the list - in place is still picked up by the next lookup. - - A plain `cached_property` cannot be used here: `model_copy` carries `__dict__` over to the - new instance, so a copy that replaces the snapshots would inherit a stale index. The index is - tied to the list it was built from, and rebuilt whenever `snapshots` is a different list. - Keeping a reference to that list also keeps it alive, so its identity cannot be reused by - another object. + This is calculated once per snapshots list and cached. """ cached = self.__dict__.get("_id_to_snapshot_position") if cached is None or cached[0] is not self.snapshots: @@ -265,8 +258,7 @@ def snapshot_by_id(self, snapshot_id: int) -> Snapshot | None: if (position := self._lazy_id_to_snapshot_position.get(snapshot_id)) is not None and position < len(snapshots): if (snapshot := snapshots[position]).snapshot_id == snapshot_id: return snapshot - # Either the id is absent, or the list was mutated in place and the cached positions - # no longer line up. Fall back to a scan, which is always correct. + # Absent id, or the list was mutated in place and the cached positions no longer line up. return next((snapshot for snapshot in snapshots if snapshot.snapshot_id == snapshot_id), None) def schema_by_id(self, schema_id: int) -> Schema | None: From b98103ed91feea811bd6dab4cbf4cae40b39469a Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Mon, 3 Aug 2026 12:24:14 +0100 Subject: [PATCH 5/6] changing approach to avoid errors and keeping the snapshot_by_id the same --- pyiceberg/table/inspect.py | 8 +++- pyiceberg/table/metadata.py | 23 +---------- tests/table/test_metadata.py | 74 ------------------------------------ 3 files changed, 7 insertions(+), 98 deletions(-) diff --git a/pyiceberg/table/inspect.py b/pyiceberg/table/inspect.py index 5cae743313..6506f46afa 100644 --- a/pyiceberg/table/inspect.py +++ b/pyiceberg/table/inspect.py @@ -310,13 +310,15 @@ def partitions( ) partitions_map: dict[tuple[str, Any], Any] = {} + # snapshot_by_id is a linear scan, and there is one lookup per manifest entry + snapshots_by_id = {snapshot.snapshot_id: snapshot for snapshot in self.tbl.metadata.snapshots} for entry in itertools.chain.from_iterable(scan._plan_manifest_entries()): partition = entry.data_file.partition partition_record_dict = { field.name: partition[pos] for pos, field in enumerate(self.tbl.metadata.specs()[entry.data_file.spec_id].fields) } - entry_snapshot = self.tbl.snapshot_by_id(entry.snapshot_id) if entry.snapshot_id is not None else None + entry_snapshot = snapshots_by_id.get(entry.snapshot_id) if entry.snapshot_id is not None else None self._update_partitions_map_from_manifest_entry( partitions_map, entry.data_file, partition_record_dict, entry_snapshot ) @@ -532,9 +534,11 @@ def history(self) -> pa.Table: history = [] metadata = self.tbl.metadata + # snapshot_by_id is a linear scan, and there is one lookup per snapshot log entry + snapshots_by_id = {snapshot.snapshot_id: snapshot for snapshot in metadata.snapshots} for snapshot_entry in metadata.snapshot_log: - snapshot = metadata.snapshot_by_id(snapshot_entry.snapshot_id) + snapshot = snapshots_by_id.get(snapshot_entry.snapshot_id) history.append( { diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index 0565678c4f..26b6e3d3ad 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -236,30 +236,9 @@ class TableMetadataCommonFields(IcebergBaseModel): def transform_properties_dict_value_to_str(cls, properties: Properties) -> dict[str, str]: return transform_dict_value_to_str(properties) - @property - def _lazy_id_to_snapshot_position(self) -> dict[int, int]: - """Return an index of snapshot ID to its position in the snapshots list. - - This is calculated once per snapshots list and cached. - """ - cached = self.__dict__.get("_id_to_snapshot_position") - if cached is None or cached[0] is not self.snapshots: - cached = ( - self.snapshots, - {snapshot.snapshot_id: position for position, snapshot in enumerate(self.snapshots)}, - ) - # The model is frozen, so bypass the pydantic __setattr__ to memoize. - object.__setattr__(self, "_id_to_snapshot_position", cached) - return cached[1] - def snapshot_by_id(self, snapshot_id: int) -> Snapshot | None: """Get the snapshot by snapshot_id.""" - snapshots = self.snapshots - if (position := self._lazy_id_to_snapshot_position.get(snapshot_id)) is not None and position < len(snapshots): - if (snapshot := snapshots[position]).snapshot_id == snapshot_id: - return snapshot - # Absent id, or the list was mutated in place and the cached positions no longer line up. - return next((snapshot for snapshot in snapshots if snapshot.snapshot_id == snapshot_id), None) + return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None) def schema_by_id(self, schema_id: int) -> Schema | None: """Get the schema by schema_id.""" diff --git a/tests/table/test_metadata.py b/tests/table/test_metadata.py index 2372ff8e1a..c163c90626 100644 --- a/tests/table/test_metadata.py +++ b/tests/table/test_metadata.py @@ -37,7 +37,6 @@ new_table_metadata, ) from pyiceberg.table.refs import SnapshotRef, SnapshotRefType -from pyiceberg.table.snapshots import Operation, Snapshot, Summary from pyiceberg.table.sorting import NullOrder, SortDirection, SortField, SortOrder from pyiceberg.transforms import IdentityTransform from pyiceberg.typedef import UTF8 @@ -146,79 +145,6 @@ def test_parsing_correct_types(example_table_metadata_v2: dict[str, Any]) -> Non assert isinstance(table_metadata.schemas[0].fields[0].field_type, LongType) -def test_snapshot_by_id(example_table_metadata_v2: dict[str, Any]) -> None: - table_metadata = TableMetadataV2(**example_table_metadata_v2) - - # Returns the same instance that is in the snapshots list, not a copy - assert table_metadata.snapshot_by_id(3051729675574597004) is table_metadata.snapshots[0] - assert table_metadata.snapshot_by_id(3055729675574597004) is table_metadata.snapshots[1] - assert table_metadata.snapshot_by_id(-1) is None - - -def test_snapshot_by_id_index_is_invalidated_on_model_copy(example_table_metadata_v2: dict[str, Any]) -> None: - """The snapshot lookup index must not survive a model_copy that replaces the snapshots.""" - table_metadata = TableMetadataV2(**example_table_metadata_v2) - - # Build the index before copying, so a stale one would be carried over - assert table_metadata.snapshot_by_id(3051729675574597004) is not None - - new_snapshot = Snapshot( - snapshot_id=1, - parent_snapshot_id=3055729675574597004, - sequence_number=35, - timestamp_ms=1602638573591, - manifest_list="s3://bucket/test/manifest-list", - summary=Summary(Operation.APPEND), - schema_id=1, - ) - with_added = table_metadata.model_copy(update={"snapshots": table_metadata.snapshots + [new_snapshot]}) - assert with_added.snapshot_by_id(1) is new_snapshot - assert with_added.snapshot_by_id(3051729675574597004) is not None - - without_first = with_added.model_copy(update={"snapshots": with_added.snapshots[1:]}) - assert without_first.snapshot_by_id(3051729675574597004) is None - assert without_first.snapshot_by_id(1) is new_snapshot - - # The original is unaffected by either copy - assert table_metadata.snapshot_by_id(1) is None - assert table_metadata.snapshot_by_id(3051729675574597004) is not None - - -def test_snapshot_by_id_reflects_in_place_snapshot_list_mutation(example_table_metadata_v2: dict[str, Any]) -> None: - """The snapshot lookup must not go stale when the snapshots list itself is mutated in place.""" - table_metadata = TableMetadataV2(**example_table_metadata_v2) - snapshot_id = 3051729675574597004 - - # Build the index before mutating, so a stale one would still be in place - assert table_metadata.snapshot_by_id(snapshot_id) is not None - - # Replacing an entry: the lookup must return the new instance, not the replaced one - altered = table_metadata.snapshots[0].model_copy(update={"summary": Summary(Operation.DELETE)}) - table_metadata.snapshots[0] = altered - assert table_metadata.snapshot_by_id(snapshot_id) is altered - - # Reordering: ids still resolve to the right snapshots - table_metadata.snapshots.reverse() - assert table_metadata.snapshot_by_id(snapshot_id) is altered - assert table_metadata.snapshot_by_id(3055729675574597004) is table_metadata.snapshots[0] - - # Removing: the id is gone - table_metadata.snapshots.clear() - assert table_metadata.snapshot_by_id(snapshot_id) is None - - -def test_snapshot_by_id_index_is_not_serialized(example_table_metadata_v2: dict[str, Any]) -> None: - """The memoized index is an implementation detail and must stay out of the serialized form.""" - table_metadata = TableMetadataV2(**example_table_metadata_v2) - assert table_metadata.snapshot_by_id(3051729675574597004) is not None - - assert "_id_to_snapshot_position" not in table_metadata.model_dump() - assert "_id_to_snapshot_position" not in table_metadata.model_dump_json() - assert "_id_to_snapshot_position" not in TableMetadataV2.model_fields - # Two equal instances stay equal when only one of them has built the index - assert table_metadata == TableMetadataV2(**example_table_metadata_v2) - - def test_updating_metadata(example_table_metadata_v2: dict[str, Any]) -> None: """Test creating a new TableMetadata instance that's an updated version of an existing TableMetadata instance""" From fca3639d4e5e37f4ab0621da7d8001e44241f90d Mon Sep 17 00:00:00 2001 From: Matheus de Freitas Andrade Date: Mon, 3 Aug 2026 13:25:32 +0100 Subject: [PATCH 6/6] creating function to avoid duplicate code --- pyiceberg/table/inspect.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pyiceberg/table/inspect.py b/pyiceberg/table/inspect.py index 6506f46afa..a50759d3a0 100644 --- a/pyiceberg/table/inspect.py +++ b/pyiceberg/table/inspect.py @@ -65,6 +65,13 @@ def _get_snapshot(self, snapshot_id: int | None = None) -> Snapshot: else: raise ValueError("Cannot get a snapshot as the table does not have any.") + def _get_snapshots_by_id(self) -> dict[int, Snapshot]: + """Index the snapshots by ID, for methods that look up many of them. + + snapshot_by_id is a linear scan, so calling it once per row is quadratic. + """ + return {snapshot.snapshot_id: snapshot for snapshot in self.tbl.metadata.snapshots} + def snapshots(self) -> pa.Table: import pyarrow as pa @@ -310,8 +317,7 @@ def partitions( ) partitions_map: dict[tuple[str, Any], Any] = {} - # snapshot_by_id is a linear scan, and there is one lookup per manifest entry - snapshots_by_id = {snapshot.snapshot_id: snapshot for snapshot in self.tbl.metadata.snapshots} + snapshots_by_id = self._get_snapshots_by_id() for entry in itertools.chain.from_iterable(scan._plan_manifest_entries()): partition = entry.data_file.partition @@ -534,8 +540,7 @@ def history(self) -> pa.Table: history = [] metadata = self.tbl.metadata - # snapshot_by_id is a linear scan, and there is one lookup per snapshot log entry - snapshots_by_id = {snapshot.snapshot_id: snapshot for snapshot in metadata.snapshots} + snapshots_by_id = self._get_snapshots_by_id() for snapshot_entry in metadata.snapshot_log: snapshot = snapshots_by_id.get(snapshot_entry.snapshot_id)