lidavidm commented on code in PR #82:
URL: https://github.com/apache/arrow-nanoarrow/pull/82#discussion_r1042616798
##########
src/nanoarrow/array_inline.h:
##########
@@ -48,10 +48,87 @@ static inline struct ArrowBuffer* ArrowArrayBuffer(struct
ArrowArray* array, int
}
}
+// We don't currently support the case of unions where type_id != child_index;
+// however, these functions are used to keep track of where that assumption
+// is made.
+static inline int8_t _ArrowArrayUnionChildIndex(struct ArrowArray* array,
+ int8_t type_id) {
+ return type_id;
+}
+
+static inline int8_t _ArrowArrayUnionTypeId(struct ArrowArray* array,
+ int8_t child_index) {
+ return child_index;
+}
+
+static inline int8_t _ArrowParseUnionTypeIds(const char* type_ids, int8_t*
out) {
+ if (*type_ids == '\0') {
+ return 0;
+ }
+
+ int32_t i = 0;
+ long type_id;
+ char* end_ptr;
+ do {
+ type_id = strtol(type_ids, &end_ptr, 10);
Review Comment:
Should we check that type_id is >= 0 and <128?
##########
src/nanoarrow/schema_test.cc:
##########
@@ -1192,9 +1197,7 @@ TEST(SchemaViewTest, SchemaViewInitNestedUnion) {
EXPECT_EQ(schema_view.layout.element_size_bits[0], 8);
EXPECT_EQ(schema_view.layout.element_size_bits[1], 32);
EXPECT_EQ(schema_view.layout.element_size_bits[2], 0);
- EXPECT_EQ(
- std::string(schema_view.union_type_ids.data,
schema_view.union_type_ids.n_bytes),
- std::string("0"));
+ EXPECT_EQ(std::string(schema_view.union_type_ids), std::string("0"));
Review Comment:
nit: if they're both null-terminated strings, you can `EXPECT_STREQ` (not a
big deal here though)
##########
src/nanoarrow/array_inline.h:
##########
@@ -48,10 +48,87 @@ static inline struct ArrowBuffer* ArrowArrayBuffer(struct
ArrowArray* array, int
}
}
+// We don't currently support the case of unions where type_id != child_index;
+// however, these functions are used to keep track of where that assumption
+// is made.
+static inline int8_t _ArrowArrayUnionChildIndex(struct ArrowArray* array,
+ int8_t type_id) {
+ return type_id;
+}
+
+static inline int8_t _ArrowArrayUnionTypeId(struct ArrowArray* array,
+ int8_t child_index) {
+ return child_index;
+}
+
+static inline int8_t _ArrowParseUnionTypeIds(const char* type_ids, int8_t*
out) {
+ if (*type_ids == '\0') {
+ return 0;
+ }
+
+ int32_t i = 0;
+ long type_id;
+ char* end_ptr;
+ do {
+ type_id = strtol(type_ids, &end_ptr, 10);
+ if (end_ptr == type_ids) {
+ return -1;
+ }
+
+ if (out != NULL) {
+ out[i] = type_id;
+ }
+
+ i++;
+
+ type_ids = end_ptr;
+ if (*type_ids == '\0') {
+ return i;
+ } else if (*type_ids != ',') {
+ return -1;
+ } else {
+ type_ids++;
+ }
+ } while (1);
+
+ return -1;
+}
+
+static inline int8_t _ArrowUnionTypeIdsWillEqualChildIndices(const char*
type_id_str,
+ int64_t
n_children) {
+ int8_t type_ids[128];
+ int8_t n_type_ids = _ArrowParseUnionTypeIds(type_id_str, type_ids);
+ if (n_type_ids != n_children) {
Review Comment:
That seems more like a hard error, though I suppose it's caught elsewhere?
--
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]
