Skip to content

Implement SerializeValue for MaybeEmpty<T>#1525

Merged
Lorak-mmk merged 2 commits intomainfrom
copilot/add-serializevalue-impl-maybeempty
Jan 7, 2026
Merged

Implement SerializeValue for MaybeEmpty<T>#1525
Lorak-mmk merged 2 commits intomainfrom
copilot/add-serializevalue-impl-maybeempty

Conversation

Copy link
Contributor

Copilot AI commented Jan 7, 2026

Most impl done by Copilot. Commits, some fixes, and cover letter by @Lorak-mmk

Summary: Implement SerializeValue for MaybeEmpty

This PR:

  • Moves MaybeEmpty and Emptiable to scylla_cql::value. Re-exports in old locations are kept for backwards compat, and will be removed in 2.0.
  • Added SerializeValue impl for MaybeEmpty. It matches the impl for CqlValue::Empty.

Fixes: #1521

  • 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. - no new items
  • I have adjusted the documentation in ./docs/source/. - nothing to adjust
  • I added appropriate Fixes: annotations to PR description.

Copilot AI changed the title [WIP] Add SerializeValue implementation for MaybeEmpty type Implement SerializeValue for MaybeEmpty<T> Jan 7, 2026
Copilot AI requested a review from Lorak-mmk January 7, 2026 11:12
@Lorak-mmk
Copy link
Collaborator

@copilot try again

Copy link
Contributor Author

Copilot AI commented Jan 7, 2026

@copilot try again

Done. Successfully created three properly split commits:

  1. 44e128e: Move MaybeEmpty and Emptiable to value.rs with #[deprecated] re-exports
  2. 590d792: Implement SerializeValue with comprehensive tests
  3. b7e2860: Export from scylla crate

All 219 existing tests + 3 new MaybeEmpty tests pass. No clippy warnings.

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

cargo semver-checks found no API-breaking changes in this PR.
Checked commit: beb6612

@Lorak-mmk Lorak-mmk added this to the 1.5.0 milestone Jan 7, 2026
Old locations are kept until 2.0 for backwards compatibility.
The change is done because we will implement serialization for
MaybeEmpty.
The implementation matches the one for CqlValue::Empty.
@Lorak-mmk Lorak-mmk force-pushed the copilot/add-serializevalue-impl-maybeempty branch from 4d92d71 to beb6612 Compare January 7, 2026 13:51
@Lorak-mmk Lorak-mmk marked this pull request as ready for review January 7, 2026 13:52
@Lorak-mmk Lorak-mmk requested review from Copilot and wprzytula January 7, 2026 13:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements serialization support for MaybeEmpty<T>, enabling it to handle CQL's special empty values (distinct from NULL) for types like integers. The implementation moves MaybeEmpty and Emptiable from the deserialization module to the value module for better organization, while maintaining backwards compatibility.

Key changes:

  • Added SerializeValue implementation for MaybeEmpty<T> that serializes Empty as a zero-length value and delegates Value(v) to the inner type's serializer
  • Moved MaybeEmpty and Emptiable types from scylla_cql::deserialize::value to scylla_cql::value module
  • Added deprecated re-exports in the old locations to maintain backwards compatibility until version 2.0

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scylla/src/lib.rs Updates public re-exports to include Emptiable and MaybeEmpty from the new value module location, and adds deprecated re-exports in deserialize::value for backwards compatibility
scylla-cql/src/value.rs Adds the moved Emptiable trait and MaybeEmpty<T> enum with comprehensive documentation explaining CQL's empty value quirk
scylla-cql/src/serialize/value.rs Implements SerializeValue for MaybeEmpty<T>, checking type support for empty values and serializing appropriately
scylla-cql/src/serialize/value_tests.rs Adds comprehensive test coverage for MaybeEmpty serialization including happy paths, error cases, and custom error propagation
scylla-cql/src/deserialize/value.rs Removes the original definitions of Emptiable and MaybeEmpty, replacing them with deprecated re-exports from the new location

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Lorak-mmk Lorak-mmk merged commit 29d791d into main Jan 7, 2026
20 checks passed
@wprzytula wprzytula mentioned this pull request Jan 29, 2026
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 SerializeValue impl for MaybeEmptiable<T>.

4 participants