c-thiel commented on code in PR #615:
URL: https://github.com/apache/iceberg-rust/pull/615#discussion_r1774977970
##########
crates/iceberg/src/spec/schema.rs:
##########
@@ -943,6 +965,122 @@ impl SchemaVisitor for PruneColumn {
}
}
+struct ReassignFieldIds {
+ next_field_id: i32,
+ old_to_new_id: HashMap<i32, i32>,
+}
+
+// We are not using the visitor here, as post order traversal is not desired.
+// Instead we want to re-assign all fields on one level first before diving
deeper.
+impl ReassignFieldIds {
+ fn new(start_from: i32) -> Self {
+ Self {
+ next_field_id: start_from,
+ old_to_new_id: HashMap::new(),
+ }
+ }
+
+ fn reassign_field_ids(&mut self, fields: Vec<NestedFieldRef>) ->
Vec<NestedFieldRef> {
+ // Visit fields on the same level first
+ let outer_fields = fields
+ .into_iter()
+ .map(|field| {
+ self.old_to_new_id.insert(field.id, self.next_field_id);
Review Comment:
I added the check. It should never trigger currently, because I switched the
build method order as you suggested.
Nevertheless the check is good if Reassignment is ever used outside of the
builder in the future.
--
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
