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.

Sign up for GitHub

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

Jump to bottom

errors: replace RowsParseError and narrow the return types after deserialization refactor #1117

Merged
merged 14 commits into from
Nov 13, 2024
Merged

errors: replace RowsParseError and narrow the return types after deserialization refactor #1117

merged 14 commits into from
Nov 13, 2024

Commits on Nov 13, 2024

  1. f_errors: introduce PreparedMetadataParseError 

    Previously, the `ResultMetadataParseError` error type was common for both
Result and Prepared metadata.

It does not make much sense, though. Obviously, some of the error variants
are shared between these two types, but not all of them.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    52f0103 View commit details
    Browse the repository at this point in the history

  2. f_errors: RawRowsAndPagingStateResponseParseError 

    An error that occurred during initial deserialization of
`RESULT:Rows` response. Since the deserialization of rows is lazy,
we initially only need to deserialize:
- result metadata flags
- column count (result metadata)
- paging state response

We ultimately want to get rid of current `RowsParseError` since its usage
is abused in a lot of places throughout the scylla crate. We start by narrowing
the return type of `deser_rows()` function.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    962ae50 View commit details
    Browse the repository at this point in the history

  3. errors: IntoLegacyQueryResultError 

    An error returned by `QueryResult::into_legacy_result()`.

Previously, the `QueryResult::into_legacy_result()` would return
`RowsParseError`. Note that `RowsParseError` is (for now) included in
new error type. This is because `RawMetadataAndRawRows::deserialize_metadata()`
still returns `RowsParseError`. This will be adjusted in following commit once
we introduce a new error type for `deserialize_metadata` method.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    256705d View commit details
    Browse the repository at this point in the history

  4. metadata: remove column count mismath check 

    This could only happen if there is a serious bug in the driver.

`deser_col_specs_generic` takes `column_count` as argument and deserializes
the col specs `column_count` times in a loop. Thus, this check does not make sense.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    9b625ef View commit details
    Browse the repository at this point in the history

  5. metadata: return ResultMetadataAndRowsCountParseError in deserialize_… 

    …metadata

Adjusted the error returned by `RawMetadataAndRawRows::deserialize_metadata()`.

Again, to narrow the return error type of following methods:
- RawMetadataAndRawRows::deserialize_metadata()
- QueryResult::into_rows_result()

Adjusted the callers of `QueryResult::into_rows_result` in topology.rs and session.rs
and added corresponding variants for `TracingProtocolError` and `SchemaVersionFetchError`

Temporarily, we need to introduce a corresponding variant to `QueryError`
and `NewSessionError`. This is because internal API makes use of `into_rows_result()`
in multiple places. The callers will be adjusted later in this PR, allowing us
to remove the variant from `Query/NSError`.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    b2d60b3 View commit details
    Browse the repository at this point in the history

  6. topology: remove RowsParseError usage from query_peers

    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    e0e27c0 View commit details
    Browse the repository at this point in the history

  7. NextRowError -> LegacyNextRowError 

    We will re-introduce `NewRowError` as an error type returned by current
async iterator API.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    6a137fd View commit details
    Browse the repository at this point in the history

  8. errors: NextPageError and NextRowError 

    These are errors returned by async iterator API.

Ultimately, we want them to be independent types returned by the public API.
However, as of now, NextRowError needs to be wrapped in QueryError - to
address that, further changes to iter API need to applied. These, however,
are not in a scope of this PR.

This partial change of iter API is introduced because we want to get rid
of RowsParseError dependency in this module.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    4717ac1 View commit details
    Browse the repository at this point in the history

  9. errors: remove QueryError::ResultMetadataParseError variant 

    It's not used anymore after adjustments to iter API.
It was introduced temporarily earlier, and can now be safely removed.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    c2a700f View commit details
    Browse the repository at this point in the history

  10. legacy: adjust LegacyNextRowError 

    So we do not depend on RowsParseError anymore, new variant is introduced
that represents a row deserialization failure.
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    827dcb5 View commit details
    Browse the repository at this point in the history

  11. errors: remove RowsParseError (finally!)

    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    e9c446c View commit details
    Browse the repository at this point in the history

  12. treewide: fix typo occure... -> occurre...

    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    9f87616 View commit details
    Browse the repository at this point in the history

  13. qr: flatten return type of QueryResult::into_rows_result to Result<...> 

    Instead of returning Result<Option<_>, _>, we will simply return a Result.
IntoRowsResultError is introduced specifically for this method.

Adjusted all of usages of this API. Most of the changes were simply
to replace `unwrap().unwrap()` to single `unwrap()`.

There are 4 places that require more focus during review:
- print_result() method in `cql-sh.rs` example.
- changes to SchemaVersionFetchError (and corresponding code in connection.rs)
- changes to TracingProtocolError (and corresponding code in session.rs)
- adjustment to `scylla_supports_tablets` in test_utils.rs
    @muzarski
    muzarski committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    ed4a427 View commit details
    Browse the repository at this point in the history

  14. qr: return self in into_rows_result if response is not Rows 

    Co-authored-by: Karol Baryła <[email protected]>
    @muzarski @Lorak-mmk
    muzarski and Lorak-mmk committed Nov 13, 2024
    Configuration menu
    Copy the full SHA
    2054163 View commit details
    Browse the repository at this point in the history