Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,6 @@ time = { version = "0.3", optional = true }
chrono = { version = "0.4", optional = true, features = ["serde"] }
bstr = { version = "1.11.0", default-features = false }
quanta = { version = "0.12", optional = true }
polonius-the-crab = "0.5.0"

bnum = "0.13.0"

tracing = { version = "0.1.44", default-features = false, features = ["std"] }
Expand Down
2 changes: 1 addition & 1 deletion rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
edition = "2021"
edition = "2024"
merge_derives = false
103 changes: 71 additions & 32 deletions src/cursors/row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::{
use bytes::Buf;
use clickhouse_types::error::TypesError;
use clickhouse_types::parse_rbwnat_columns_header;
use polonius_the_crab::prelude::*;
use std::marker::PhantomData;
use std::pin::Pin;
use std::task::{Context, Poll, ready};
Expand Down Expand Up @@ -107,6 +106,35 @@ impl<T> RowCursor<T> {
Next::new(self).await
}

// Why the unsafe reborrow?
//
// NLL (the current borrow checker) cannot see that `bytes` is dead in the
// NotEnoughData branch of this loop. The returned value borrows from
// `bytes`, so NLL extends that borrow to the function's return lifetime,
// which blocks the `bytes.extend()` call that only runs when no value
// exists. This is a known Polonius limitation:
// https://github.com/rust-lang/rust/issues/51132
//
// This used to use the `polonius-the-crab` crate, which wraps the same
// raw-pointer reborrow behind a macro. It was dropped because the crate
// and its dependency tree are unmaintained:
// - `paste` transitive dep: RUSTSEC-2024-0436 (unmaintained)
// - `polonius-the-crab`: no meaningful commits in 12+ months
// - `higher-kinded-types`, `macro_rules_attribute`: same status
// Four stagnant crates and two RustSec advisories for a macro that
// expands to one line of unsafe is a poor trade-off.
//
// Alternatives considered:
// - TryRow enum: borrow still escapes via the return type, same error.
// - async-only next() + poll_next_owned for Stream: same NLL issue.
// - interior mutability in BytesExt via UnsafeCell: roughly 3x the
// diff for the same amount of unsafe, just hidden.
// - double deserialisation / probe-then-extract: roughly 2x deser cost
// on the happy path, unacceptable for a perf-sensitive cursor.
// None compiled without unsafe somewhere, or had unacceptable costs.
//
// Once Polonius lands in stable rustc, this can be removed.

#[inline]
fn poll_next(&mut self, cx: &mut Context<'_>) -> Poll<Result<Option<T::Value<'_>>>>
where
Expand All @@ -119,31 +147,38 @@ impl<T> RowCursor<T> {

let _span = self.span.enter();

let mut bytes = &mut self.bytes;
let bytes = &mut self.bytes;

loop {
polonius!(|bytes| -> Poll<Result<Option<T::Value<'polonius>>>> {
if bytes.remaining() > 0 {
let mut slice = bytes.slice();
let result = rowbinary::deserialize_row::<T::Value<'_>>(
&mut slice,
self.row_metadata.as_ref(),
);
// SAFETY: we create a second &mut to `bytes` via raw pointer so the
// borrow checker releases the original. This is sound because:
// - On Ok: we return immediately, only one &mut is live.
// - On NotEnoughData: the deserialised value does not exist, the
// reborrow is dead, and we fall through to extend().
// - On Err: we return immediately.
// Polonius would prove this automatically; NLL cannot yet.
let reborrowed = unsafe { &mut *(bytes as *mut BytesExt) };

if reborrowed.remaining() > 0 {
let mut slice = reborrowed.slice();
let result = rowbinary::deserialize_row::<T::Value<'_>>(
&mut slice,
self.row_metadata.as_ref(),
);

match result {
Ok(value) => {
self.returned_rows += 1;
bytes.set_remaining(slice.len());
polonius_return!(Poll::Ready(Ok(Some(value))))
}
Err(Error::NotEnoughData) => {}
Err(err) => {
tracing::debug!(error=?err, "error deserializing row");
polonius_return!(Poll::Ready(Err(err)))
}
match result {
Ok(value) => {
self.returned_rows += 1;
reborrowed.set_remaining(slice.len());
return Poll::Ready(Ok(Some(value)));
}
Err(Error::NotEnoughData) => {}
Err(err) => {
tracing::debug!(error=?err, "error deserializing row");
return Poll::Ready(Err(err));
}
}
});
}

match ready!(self.raw.poll_next(cx)) {
Ok(Some(chunk)) => bytes.extend(chunk),
Expand Down Expand Up @@ -251,18 +286,22 @@ where

#[inline]
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
// Temporarily take the cursor out in order for `cursor.poll_next` to return a value with
// the correct lifetime `'a` rather than the unnamed lifetime of `&mut self`.
let mut cursor = self.cursor.take().expect("Future polled after completion");
// Take cursor out so poll_next's return value gets lifetime 'a
// (not the anonymous reborrow lifetime of &mut self).
let cursor = self.cursor.take().expect("Future polled after completion");

polonius!(|cursor| -> Poll<Result<Option<T::Value<'polonius>>>> {
match cursor.poll_next(cx) {
Poll::Ready(value) => polonius_return!(Poll::Ready(value)),
Poll::Pending => {}
}
});
// SAFETY: same pattern as poll_next above. We create a second &mut
// via raw pointer. On Ready the reborrow escapes via the return value
// and cursor is consumed. On Pending the reborrow is dead and we put
// cursor back. Sound for the same reasons; Polonius would accept this.
let reborrowed = unsafe { &mut *(cursor as *mut RowCursor<T>) };

self.cursor = Some(cursor);
Poll::Pending
match reborrowed.poll_next(cx) {
Poll::Ready(value) => Poll::Ready(value),
Poll::Pending => {
self.cursor = Some(cursor);
Poll::Pending
}
}
}
}
Loading
Loading