nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1400840547
Alright then, so should we merge this as is? Or close it without merging and
bother with it later whenever V2 is ready? I'm fine either way.
--
This is an automated message from
nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1400753926
> The new scan node API simply accepts a vector of field refs.
Where exactly? `MaterializedFields()` appears to be a method of
`ScanOptions`, not something I can set
nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1400625313
> @nealrichardson Ok, I did some investigation.
>
> First, the reason this is not being encountered from pyarrow:
>
> The scanner options currently takes both a
nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1399086990
> > I thought this was just going to be deleting code from the R package:
instead of finding the top-level field names in the projection and sending them
in the ScanNode, I'd send
nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1398797483
> > I haven't run C++ unit tests in forever, so figured I'd get some
feedback before diving in there.
>
> Sorry, I was thinking of R e2e tests. I would hope the C++ change
nealrichardson commented on PR #33770:
URL: https://github.com/apache/arrow/pull/33770#issuecomment-1398606967
> Do you want some unit tests?
Of course, this needs some. The tests that were added for this function when
it was introduced
