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 2021/12/30 10:03:22 UTC

[GitHub] [arrow] edponce commented on pull request #11882: ARROW-9843: [C++] Implement Between ternary kernel

edponce commented on pull request #11882:
URL: https://github.com/apache/arrow/pull/11882#issuecomment-1002956781


   I have the following observation to make wrt to the API for this `between` compute function. The purpose of `between` is to support range comparisons similar to [SQL `BETWEEN`](https://www.w3schools.com/sql/sql_between.asp) and [pandas `between`](https://pandas.pydata.org/docs/reference/api/pandas.Series.between.html#pandas-series-between). These APIs are different from this PR's implementation, but the behavior is equivalent. Nevertheless, I suggest either of the following:
   1. Implement `between` compute function not as a _logical compare_ function but as an independent function that takes 3 inputs and an option for declaring bounds-inclusiveness. Similar to the Pandas API.
   2. Implement `between` as an extension to the _logical compare_ functions. Now there are two ways to go about this:
     a. As this PR does, implement 4 new compare functions `less_less`, `less_equal_less`, `less_less_equal`, and `less_equal_less_equal`.
     b. Have a single variadic `compare` function that uses options to select the type of comparison (e.g., LESS, GREATER_EQUAL, LESS_LESS)
   
   I recommend to follow option (1), for the following reasons. It is a common API and the function name clearly states its operation. The issue with (2a) is that its function names are not common-case, and if we make a single `between` with options for LESS_LESS, etc. then I would argue that the other logical comparisons need to be merged into a single compute function for symmetry/consistency purposes. This last point touches on option (2b) which I do not think is a reasonable solution because of its complexity and one could argue that an analogous case would be to have a single `arithmetic` function where the operation performed is specified via options. Obviously, this is not desired.


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