c-thiel commented on code in PR #615:
URL: https://github.com/apache/iceberg-rust/pull/615#discussion_r1774983362
##########
crates/iceberg/src/spec/schema.rs:
##########
@@ -105,7 +116,16 @@ impl SchemaBuilder {
}
/// Builds the schema.
- pub fn build(self) -> Result<Schema> {
+ pub fn build(mut self) -> Result<Schema> {
+ if let Some(start_from) = self.reassign_field_ids_from {
Review Comment:
I agree - I didn't think about the error messages.
It's unfortunately not super straightforward to switch the order, as for
example index_by_id needs a full rebuild as both keys and values changes. I
think we have two options to achieve the swap:
1. Use `id_reassigner.apply_*` for all other fields that have been build
(i.e. `id_to_field`) or
2. Re-build those fields using the same code - i.e. the build option.
I opted for 2.
While it is slightly less performant because we have to visit the structure
again, it is much less error prone as we simply re-use existing and well-tested
code.
Let me know what you think!
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
