Conversation
5d2c5e9 to
b3a149d
Compare
| /// \brief The batch size to read. Only applies to implementations that support | ||
| /// batching. | ||
| int64_t batch_size; |
There was a problem hiding this comment.
Is the unit bytes or rows?
(Should this also be optional?)
There was a problem hiding this comment.
(Why does Split use size_t but this uses int64_t?)
There was a problem hiding this comment.
I was referring to number of rows in a batch (a.k.a. ArrowArray). It seems that we can use std::optional<size_t> and define a default batch size if non provided.
There was a problem hiding this comment.
Ah, ok. int64_t might be appropriate as that's then consistent with C Data Interface. Clarifying the unit might be helpful.
| DataLayout data_layout() const final { return DataLayout::kStructLike; } | ||
|
|
||
| private: | ||
| std::unique_ptr<Reader> reader_; |
There was a problem hiding this comment.
I’m a bit confused about why keeping a Reader object here.
There was a problem hiding this comment.
Because this is just an adapter. If the wrapped reader directly returns kStructLike, it does nothing. Otherwise it will try to aggregate them into an ArrowArray.
No description provided.