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

SerializeRow: Add is_empty method #869

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

Lorak-mmk
Copy link
Collaborator

Currently ongoing serialization refactor requires to only perform queries with values using prepared statements. Session interface still allows to pass values in query* methods, so we need a way to know if we can perform unprepared query or not.

One way is to always perform prepared queries, but that is not optimal. Before ongoing refactor we could serialize values and check if they are empty. This will no longer be possible soon, because serializing will require knowing column types, which in this case means having a prepared statement.

This commit implements third way - additional method on SerializeRow trait that allows us to check if values are empty (in which case we can skip the prepare).

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.

@Lorak-mmk Lorak-mmk requested a review from piodul December 5, 2023 22:57
Copy link
Collaborator

@piodul piodul left a comment

Choose a reason for hiding this comment

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

In order to test the new method, I suggest modifying the serialize_values/serialize_values_only_new function in scylla-cql/src/frame/value_tests.rs to call SerializeRow::is_empty and compare its result with what <T as SerializeRow>::serialize has produced.

scylla-cql/src/types/serialize/row.rs Show resolved Hide resolved
scylla-cql/src/types/serialize/row.rs Outdated Show resolved Hide resolved
Currently ongoing serialization refactor requires to only perform
queries with values using prepared statements. Session interface still
allows to pass values in query* methods, so we need a way to know if we
can perform unprepared query or not.

One way is to always perform prepared queries, but that is not optimal.
Before ongoing refactor we could serialize values and check if they are
empty. This will no longer be possible soon, because serializing will
require knowing column types, which in this case means having a prepared
statement.

This commit implements third way - additional method on SerializeRow
trait that allows us to check if values are empty (in which case we can
skip the prepare).
@Lorak-mmk Lorak-mmk force-pushed the serialize_row_is_empty branch from cd6536b to eb10f3b Compare December 6, 2023 09:37
@Lorak-mmk
Copy link
Collaborator Author

Resolved comments, added asserts to serialize_values/serialize_values_only_new

@piodul piodul merged commit b158aca into scylladb:main Dec 6, 2023
8 checks passed
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.

2 participants