You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/03 03:08:28 UTC

[GitHub] [arrow-datafusion] liukun4515 opened a new issue, #2832: InList: add more integration test for Inlist

liukun4515 opened a new issue, #2832:
URL: https://github.com/apache/arrow-datafusion/issues/2832

   Thank you @liukun4515  -- I reviewed the tests carefully and I think this looks good.
   
   The code "feels" a little more complicated than needed but I think it is doing what it needs to and the test coverage is good.
   
   I wonder if it is worth just 1 test in `sql_integration` somewhere that ensures this implementation is hooked up correctly rather than just relying on tests in `in_list.rs`
   
   I think it would be good if @Ted-Jiang  was also able to review this PR prior to merging it, but it isn't necessary.
   
   _Originally posted by @alamb in https://github.com/apache/arrow-datafusion/pull/2809#pullrequestreview-1026740587_


-- 
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-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org