[GitHub] [arrow] js8544 commented on pull request #36058: GH-36047: [C++][Compute] Add support for duration types to IndexIn and IsIn

["js8544 (via GitHub)" <gi...@apache.org>]

js8544 commented on PR #36058:
URL: https://github.com/apache/arrow/pull/36058#issuecomment-1600062537

   > This looks like a good to me. I tested it out a bit in python and it seems to work. I do find the casting to be a bit arbitrary here. For example, it seems odd that I can do `is_in(milliseconds, seconds)` but not `is_in(seconds, milliseconds)`.
   > 
   > However, users can always explicitly cast, and I think this is better for a follow-up if someone feels strongly about it.
   
   Actually it's possible to do `is_in(seconds, milliseconds)` if the casting is safe. For example, `is_in([1s, 2s], [1000ms, 2000ms])` would return `[true, true]`, while `is_in([1s, 2s], [1ms, 2ms])` would throw an error.
   
   Currently the kernels only try to cast `value_set` into query keys' type. It does make more sense if they cast both ways. I'll try to implement this in another PR.


-- 
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

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