Improve data chunk

[sferik]

• Replaced the manual implementation of Clone for Chunk<T> with #[derive(Clone)] so the struct relies on the automatic, efficient clone implementation provided for Arc types
• Fixed a typo in DataChunkIterator

[propel-code-bot]

This PR removes the hand-written Clone implementation for Chunk in favor of #[derive(Clone)], relying on Rust's more efficient automatic derivation for Arc-based structs. Additionally, it fixes a typo in the iterator type DataChunkIterator and adds #[derive(Clone)] for the internal MaterializedLogRecord struct.

This summary was automatically generated by @propel-code-bot

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[HammadB]

Thanks

[HammadB]

I saw this was added after I hit approve. We generally don't expose Clone on types that we don't intend to clone, as a defensive measure. Was there a strong reason for this change, would you be ok to remove it?

[sferik]

The MaterializeLogsResult struct contains a Chunk<MaterializedLogRecord> field (on line 481).

Without this commit, tests were failing with the following error:

   Compiling chroma-segment v0.1.0 (/Users/erik/Code/Rust/chroma/rust/segment)
error[E0277]: the trait bound `MaterializedLogRecord: Clone` is not satisfied
   --> rust/segment/src/types.rs:481:5
    |
478 | #[derive(Debug, Clone)]
    |                 ----- in this derive macro expansion
...
481 |     materialized: Chunk<MaterializedLogRecord>,
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MaterializedLogRecord`
    |
    = note: required for `Chunk<MaterializedLogRecord>` to implement `Clone`
help: consider annotating `MaterializedLogRecord` with `#[derive(Clone)]`
    |
126 + #[derive(Clone)]
127 | struct MaterializedLogRecord {
    |

For more information about this error, try `rustc --explain E0277`.
error: could not compile `chroma-segment` (lib) due to 1 previous error
error: command `/opt/homebrew/bin/cargo test --no-run --message-format json-render-diagnostics` exited with code 101

This patch fixes that problem so the tests all pass. (Sorry for not running the tests locally before submitting my initial patch.)

[HammadB]

Oh whoops! I missed that it was an implicit use through Chunk. Nevermind

…

On Sat, Jul 12, 2025 at 1:08 AM Erik Berlin ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In rust/segment/src/types.rs <#5082 (comment)>: > @@ -122,7 +122,7 @@ impl ChromaError for LogMaterializerError { /// Instead of cloning or holding references to log records/segment data, this struct contains owned values that can be resolved to the referenced data. /// E.x. `final_document_at_log_index: Option<usize>` is used instead of `final_document: Option<&str>` to avoid holding references to the data. /// This allows `MaterializedLogRecord` (and types above it) to be trivially Send'able. -#[derive(Debug)] +#[derive(Debug, Clone)] The MaterializeLogsResult struct contains a Chunk<MaterializedLogRecord> field (on line 481). For Rust to automatically implement Clone for MaterializeLogsResult, all of its fields must implement Clone. Without this commit, tests were failing with the following error: Compiling chroma-segment v0.1.0 (/Users/erik/Code/Rust/chroma/rust/segment) error[E0277]: the trait bound `MaterializedLogRecord: Clone` is not satisfied --> rust/segment/src/types.rs:481:5 | 478 | #[derive(Debug, Clone)] | ----- in this derive macro expansion ... 481 | materialized: Chunk<MaterializedLogRecord>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `MaterializedLogRecord` | = note: required for `Chunk<MaterializedLogRecord>` to implement `Clone` help: consider annotating `MaterializedLogRecord` with `#[derive(Clone)]` | 126 + #[derive(Clone)] 127 | struct MaterializedLogRecord { | For more information about this error, try `rustc --explain E0277`. error: could not compile `chroma-segment` (lib) due to 1 previous error error: command `/opt/homebrew/bin/cargo test --no-run --message-format json-render-diagnostics` exited with code 101 This patch fixes that problem so the tests all pass. — Reply to this email directly, view it on GitHub <#5082 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKW32KXKFNYA3SY4JJWMQL3IC7BJAVCNFSM6AAAAACBJ7OUUSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMJSHAZDEMBQGU> . You are receiving this because you commented.Message ID: ***@***.***>