Dandandan commented on PR #16647: URL: https://github.com/apache/datafusion/pull/16647#issuecomment-3029069816
> This is quite cool @Dandandan > > My only real concern is that this code will be tricky to maintain and could easily get reverted / regressed as part of a follow on change > > I suggest we try and encapsulate it into a struct (I left a suggestion) > > Thank you (as always) for the reviews @zhuqi-lucas > > cc @tustvold and @crepererum for your amusement Yeah - I agree. Another option might be to panic or return an error on not maintaining the condition of keeping max 1 (extra) batch buffered in the consumer. This way the tests would fail, but it would be a small breaking change for `RowCursorStream`. Now that I think about it, `RowCursorStream::new(` has also a minor breaking change. I also think probably not many will use `RowCursorStream` directly. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
