jonahgao commented on code in PR #9619:
URL: https://github.com/apache/arrow-datafusion/pull/9619#discussion_r1526400001
##########
datafusion/physical-plan/src/memory.rs:
##########
@@ -236,6 +237,8 @@ impl MemoryExec {
pub struct MemoryStream {
/// Vector of record batches
data: Vec<RecordBatch>,
+ /// Optional memory reservation bound to the data, freed on drop
Review Comment:
This concern is quite reasonable; previously there was no need to add a
`MemoryReservation` to the `MemoryStream`, and the `with_reservation` function
is a bit confused.
I plan to no longer use `MemoryStream` for future recursive CTEs and instead
use a new stream class that can read the `WorkTable` multiple times, in order
to support
https://github.com/apache/arrow-datafusion/blob/81b0a011705b17a09f494f550a5382b0c3414597/datafusion/physical-plan/src/recursive_query.rs#L316
So that the `MemoryStream::with_reservation` API can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
