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

WIP: session_test: regression test for empty collections deserialization #1007

Closed
wants to merge 1 commit into from

Conversation

wprzytula
Copy link
Collaborator

ScyllaDB does not distinguish empty collections from nulls. That is, INSERTing an empty collection is equivalent to nullifying the corresponding column.
As pointed out in #1001, it's a nice QoL feature to be able to deserialize empty CQL collections to empty Rust collections instead of None::<RustCollection>. A test is added that checks that.

NOTICE

The test fails with the current deserialization framework implementation. It should work, however, once the new deserialization framework is introduced and the test is migrated to use it.
The goal of this PR at this point is to showcase what is the desired behaviour and remind that it has not yet been reached.

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.

Copy link

github-actions bot commented May 21, 2024

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

Copy link

@Zk2u Zk2u left a comment

Choose a reason for hiding this comment

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

ACK, this behaviour matches the issue's requirements.

ScyllaDB does not distinguish empty collections from nulls. That is,
INSERTing an empty collection is equivalent to nullifying the
corresponding column.
As pointed out in
[scylladb#1001](scylladb#1001),
it's a nice QOL feature to be able to deserialize empty CQL collections
to empty Rust collections instead of `None::<RustCollection>`.
A test is added that checks.
@wprzytula wprzytula force-pushed the test-empty-collections branch from e5b3f6b to 1b457fd Compare August 14, 2024 08:55
@wprzytula
Copy link
Collaborator Author

Rebased on main.

@wprzytula wprzytula mentioned this pull request Aug 14, 2024
8 tasks
@wprzytula wprzytula modified the milestones: 0.14.0, 0.15.0 Aug 20, 2024
@wprzytula
Copy link
Collaborator Author

Done in #1057.

@wprzytula wprzytula closed this Nov 13, 2024
@wprzytula wprzytula deleted the test-empty-collections branch November 13, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants