Skip to content

Conversation

timvermeulen
Copy link
Contributor

I only just now saw that #122 was closed as not planned, so no worries if you're not interested in merging this functionality 🙂

Summary

  • Adds impl<T: RowOwned + RowRead> Stream for RowCursor<T>
  • Removes the use of UnsafeCell from BytesExt using polonius-the-crab, which the above needed anyway

The mocked_select benchmark with validation=off has regressed 20-30% on my machine, but select_market_data/select_numbers/select_nyc_taxi_data all stayed the same, so I'm not entirely sure how significant this is.

It's worth noting that moving the fn poll_next logic out of async fn next has subtly changed the behavior of next, because the Next future doesn't track any state (other than if it has resolved) — every time RawCursor::poll_next wakes up the task, RowCursor::poll_next will first try to deserialize the next row using the available bytes before calling self.raw.poll_next. I'm not sure if this will matter in practice but if necessary I could move the self.raw.poll_next call to the start of the loop in RowCursor::poll_next.

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided so that we can include it in CHANGELOG later
  • For significant changes, documentation in README and https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

}
}

struct Next<'a, T> {
Copy link
Member

Choose a reason for hiding this comment

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

what returns this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RowCursor::next basically does, except that it immediately awaits it

@slvrtrn slvrtrn requested a review from abonander September 18, 2025 09:52
@timvermeulen timvermeulen force-pushed the rowcursor-stream branch 2 times, most recently from 7b494f0 to ae6973b Compare September 22, 2025 17:30
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