paleolimbot commented on code in PR #270:
URL: https://github.com/apache/arrow-nanoarrow/pull/270#discussion_r1280043523
##########
src/nanoarrow/array_inline.h:
##########
@@ -928,6 +928,34 @@ static inline struct ArrowBufferView
ArrowArrayViewGetBytesUnsafe(
return view;
}
+static inline void ArrowArrayViewGetIntervalUnsafe(struct ArrowArrayView*
array_view,
+ int64_t i, struct
ArrowInterval* out) {
+ const uint8_t* data_view = array_view->buffer_views[1].data.as_uint8;
+ switch (array_view->storage_type) {
+ case NANOARROW_TYPE_INTERVAL_MONTHS: {
+ const size_t size = 4;
+ memcpy(&out->months, data_view + i * size, 4);
+ break;
+ }
+ case NANOARROW_TYPE_INTERVAL_DAY_TIME: {
+ const size_t size = 8;
+ memcpy(&out->days, data_view + i * size, 4);
+ memcpy(&out->ms, data_view + i * size + 4, 4);
+ break;
+ }
+ case NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO: {
+ const size_t size = 16;
+ memcpy(&out->months, data_view + i * size, 4);
+ memcpy(&out->days, data_view + i * size + 4, 4);
+ memcpy(&out->ns, data_view + i * size + 8, 8);
+ break;
+ }
+ default:
+ out = NULL;
Review Comment:
I think the motivation there was to make it more likely that some test would
fail...perhaps some future documentation could make it more clear that it's the
caller's responsibility to type check first (in theory that's what the `Unsafe`
bit is trying to communicate).
Returning the `ArrowInterval` is definitely an option...my hunch is that it
would be inefficient since it you'd have to potentially copy a few of the
fields for every item in the loop; however, I have no evidence for that. I
think what you have here is a good compromise to support all three interval
types. If there's overwhelming feedback suggesting that it's confusing or slow
it can always be changed pre 1.0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
