Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(serialize): add SerializeRow impl for Box<T: SerializeRow> #1105

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

Daniel-Boll
Copy link
Contributor

@Daniel-Boll Daniel-Boll commented Oct 24, 2024

Fixes #1043

This PR implements SerializeRow for Box<T: SerializeRow>, as suggested by @piodul in Missing SerializeRow impl for Box #1043. The aim is to address the user-reported use case where they need to perform batch queries with different length tuples (e.g., one insert into Table A and N inserts into Table B).

I have also added a corresponding unit test (test_row_serialization_with_boxed_tuple) to verify the behavior, comparing the serialization of a boxed tuple with that of a regular tuple.

If there are any issues with this implementation, if I misunderstood the assignment, or if this feature would be better placed elsewhere, please let me know, and I'd be happy to make the necessary adjustments.

Looking forward to your feedback!

Best regards,
Daniel

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

Added an implementation of the `SerializeRow` trait for `Box<T:
SerializeRow>`. This
allows boxed types to be serialized using the existing serialization
framework. Included a unit test to verify the serialization of a boxed
tuple.

Signed-off-by: Daniel Boll <[email protected]>
Copy link

cargo semver-checks found no API-breaking changes in this PR! 🎉🥳
Checked commit: 782cb1f

@wprzytula
Copy link
Collaborator

Thank you for the contribution, @Daniel-Boll!

@Lorak-mmk Lorak-mmk merged commit 4649acb into scylladb:main Oct 26, 2024
11 checks passed
@Daniel-Boll Daniel-Boll deleted the feat/box-serialization branch October 27, 2024 00:03
@wprzytula wprzytula mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing SerializeRow impl for Box<dyn SerializeRow>
4 participants