alamb commented on code in PR #9196:
URL: https://github.com/apache/arrow-rs/pull/9196#discussion_r2699756705
##########
arrow-ipc/src/writer.rs:
##########
@@ -986,6 +986,16 @@ impl DictionaryTracker {
DictionaryComparison::Equal => unreachable!("Already checked equal
case"),
}
}
+
+ /// Resets the state of the dictionary tracker.
+ ///
+ /// This allows the dictionary tracker to be reused for a new IPC stream
while avoiding the
+ /// allocation cost of creating a new instance. This method should not be
called if
+ /// the dictionary tracker will be used to continue writing to the an
existing IPC stream.
Review Comment:
typo
```suggestion
/// the dictionary tracker will be used to continue writing to an
existing IPC stream.
```
##########
arrow-ipc/src/writer.rs:
##########
@@ -986,6 +986,16 @@ impl DictionaryTracker {
DictionaryComparison::Equal => unreachable!("Already checked equal
case"),
}
}
+
+ /// Resets the state of the dictionary tracker.
+ ///
+ /// This allows the dictionary tracker to be reused for a new IPC stream
while avoiding the
+ /// allocation cost of creating a new instance. This method should not be
called if
+ /// the dictionary tracker will be used to continue writing to the an
existing IPC stream.
+ pub fn reset(&mut self) {
Review Comment:
Some thoughts:
1. Perhaps we can call this method `clear` so that it mirrors the clear
methods on Vec, HashMap, etc?
2. Should we also reset `error_on_replacement` field?
https://github.com/apache/arrow-rs/blob/91b140564c003a8b222c9f2d2c173e8a878d192c/arrow-ipc/src/writer.rs#L832-L834.
It seems like maybe it is ok not to I just wanted to check
3. Perhaps we could leave a comment in `DictionaryTracker` fields that
reminds anyone who adds a field they have to also update reset?
--
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]
