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 is covered by existing tests. Although I think we've found in the past that it is easy to accidentally load too much from the disk and still pass the tests. R tests cover this already, from #19706. 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 the projection and the scanner would pull out the fields. Turns out though that the scanner couldn't handle nested field refs, so I took the R binding changes from #19706 and adapted them to go in scanner.cc. But the R test uses InMemoryDataset so doesn't touch parquet, and of course it can't make any assertions about whether the right amount of data is loaded. -- 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]
