Copilot commented on code in PR #875:
URL: https://github.com/apache/iceberg-cpp/pull/875#discussion_r3728565883
##########
src/iceberg/snapshot.h:
##########
@@ -413,6 +417,10 @@ struct ICEBERG_EXPORT Snapshot {
/// The upper bound of rows with assigned row IDs in this snapshot.
std::optional<int64_t> added_rows;
+ /// Internal lazy state shared by Snapshot copies so manifest lists are
parsed once.
+ mutable std::shared_ptr<internal::SnapshotCacheData> cache_data =
+ internal::MakeSnapshotCacheData();
Review Comment:
Adding `cache_data` as a new data member to the exported `Snapshot` struct
changes its size/layout and can break ABI for downstream binaries that link
against the Iceberg C++ library. If ABI stability is a goal, consider moving
this cache state behind an internal indirection that doesn't alter the public
struct layout (e.g., keep cache in `SnapshotCache` with an external map keyed
by snapshot_id/manifest_list, or a pimpl-style sidecar owned by
`TableMetadata`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
