Mark methods that do not perform bounds checking as unsafe

[alamb]

Note: migrated from original JIRA: https://issues.apache.org/jira/browse/ARROW-3776

None

[alamb]

Comment from Paul Kernfeld(paulkernfeld) @ 2019-01-24T00:13:03.622+0000:

I'm interested in working on this, although there could be a lot of downstream effects. A good example of a tricky function is arrow::array::PrimitiveArray::value, which appears to be used in a couple dozen places. A few possible strategies are:
 # Add in bounds checking so that we don't need to deal with unsafe at all.
 # Propagate the unsafes up through the code.
 # Maintain a safe and unsafe version of each function that is currently unsafe.

Personally I'm a fan of #1 because I think that reducing unsafe code will help developers and users avoid mistakes (I [accidentally wrote|https://github.com/apache/arrow/pull/3448] a nondeterministic unit test earlier this week). However, I'm new to the project so I'm happy to do what others think is best.

Comment from Paddy Horan(paddyhoran) @ 2019-01-24T01:35:09.674+0000:

I was actually thinking we would need #3.  Taking `value` as an example I would be in favor of adding bounds checking to `value` and having a `value_unchecked` that does no bounds checking and is unsafe.

I think that we need to provide the unsafe versions for maximum performance due to Arrow often being described as a development platform rather than a front end API.  i.e. people will use it as the foundation of other higher level libraries and so developers will want the option to avoid bounds checking for performance reasons.

 

[alamb]

For anyone else reading this issue, all the calls to *Array::value() I checked (below) had bounds checking in them and would panic if an out of bounds request was made.

As written, the ticket seems more broadly defined and refers to more than just value() so I'll leave it open, but at the moment this seems unactionable to me (as the methods needing bounds checking are not clear).

---

BinaryArray: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/array/array_binary.rs?L104&subtree=true

ListArray: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/array/array_list.rs?L83:9&subtree=true

PrimitiveArray: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/array/array_primitive.rs?L119:9&subtree=true

StringArray: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/array/array_string.rs?L108:9&subtree=true

BooleanArray: https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/arrow/src/array/array_boolean.rs?L119:9&subtree=true

[vertexclique]

I have mentioned this in the arrow mailing list as "exposed" api to expose unsafe methods and pointers (if needed) to some performance hungry users.
@alamb

Ref: https://lists.apache.org/thread/vk6m5lwljsv9sd7f9zm4z3o6l9997nw4

[alamb]

I think this ticket is not tracking anything actionable anymore so closing it down