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 2022/10/21 20:10:15 UTC
[GitHub] [arrow-nanoarrow] paleolimbot opened a new pull request, #61: Proof-of-concept IPC reader/writer
paleolimbot opened a new pull request, #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61
Just a public experiment for a possible IPC reader/writer. The general idea is:
- Use only the Arrow C Data and C Stream interface in the public header
- Use flatcc to handle flatbuffers IO ( https://github.com/dvidelabs/flatcc )
- Use nanoarrow to construct/read the ArrowSchema and ArrowArray objects to/from flatbuffers
I put this in an extensions/ subdirectory in this repo. I think that's a good fit...the IPC format is part of the spec, after all, and putting it in this repo would mean that the R and/or Python package could make use of it.
Any thoughts welcome!
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1287398546
That way, this would be an entirely self-contained build unless you wanted to update flatcc/the flatbuffer definitions
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1003265839
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.h:
##########
@@ -0,0 +1,153 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef NANOARROW_NANOARROW_IPC_H_INCLUDED
+#define NANOARROW_NANOARROW_IPC_H_INCLUDED
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Extra guard for versions of Arrow without the canonical guard
+#ifndef ARROW_FLAG_DICTIONARY_ORDERED
+
+/// \defgroup nanoarrow-arrow-cdata
+///
+/// The Arrow C Data (https://arrow.apache.org/docs/format/CDataInterface.html)
+/// and Arrow C Stream (https://arrow.apache.org/docs/format/CStreamInterface.html)
+/// interfaces are part of the
+/// Arrow Columnar Format specification
+/// (https://arrow.apache.org/docs/format/Columnar.html). See the Arrow documentation for
+/// documentation of these structures.
+///
+/// @{
+
+#ifndef ARROW_C_DATA_INTERFACE
+#define ARROW_C_DATA_INTERFACE
+
+#define ARROW_FLAG_DICTIONARY_ORDERED 1
+#define ARROW_FLAG_NULLABLE 2
+#define ARROW_FLAG_MAP_KEYS_SORTED 4
+
+struct ArrowSchema {
+ // Array type description
+ const char* format;
+ const char* name;
+ const char* metadata;
+ int64_t flags;
+ int64_t n_children;
+ struct ArrowSchema** children;
+ struct ArrowSchema* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowSchema*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+struct ArrowArray {
+ // Array data description
+ int64_t length;
+ int64_t null_count;
+ int64_t offset;
+ int64_t n_buffers;
+ int64_t n_children;
+ const void** buffers;
+ struct ArrowArray** children;
+ struct ArrowArray* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowArray*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_DATA_INTERFACE
+
+#ifndef ARROW_C_STREAM_INTERFACE
+#define ARROW_C_STREAM_INTERFACE
+
+struct ArrowArrayStream {
+ // Callback to get the stream type
+ // (will be the same for all arrays in the stream).
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowSchema must be released independently from the stream.
+ int (*get_schema)(struct ArrowArrayStream*, struct ArrowSchema* out);
+
+ // Callback to get the next array
+ // (if no error and the array is released, the stream has ended)
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowArray must be released independently from the stream.
+ int (*get_next)(struct ArrowArrayStream*, struct ArrowArray* out);
+
+ // Callback to get optional detailed error information.
+ // This must only be called if the last stream operation failed
+ // with a non-0 return code.
+ //
+ // Return value: pointer to a null-terminated character array describing
+ // the last error, or NULL if no description is available.
+ //
+ // The returned pointer is only valid until the next operation on this stream
+ // (including release).
+ const char* (*get_last_error)(struct ArrowArrayStream*);
+
+ // Release callback: release the stream's own resources.
+ // Note that arrays returned by `get_next` must be individually released.
+ void (*release)(struct ArrowArrayStream*);
+
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_STREAM_INTERFACE
+#endif // ARROW_FLAG_DICTIONARY_ORDERED
+
+#ifdef __cplusplus
+}
+#endif
+
+typedef int ArrowIpcErrorCode;
+
+struct ArrowIpcIO {
Review Comment:
It would be more natural for reading a file, but for something pulling in from (say) an HTTP stream or network socket, it would probably be more natural to push data instead. But yes, either can be built from the other.
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1067401507
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,297 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
+
+void ArrowIpcReaderInit(struct ArrowIpcReader* reader) {
+ memset(reader, 0, sizeof(struct ArrowIpcReader));
+}
+
+void ArrowIpcReaderReset(struct ArrowIpcReader* reader) {
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ if (reader->batch_index.release != NULL) {
+ reader->batch_index.release(&reader->batch_index);
+ }
+
+ ArrowIpcReaderInit(reader);
+}
+
+static inline uint32_t ArrowIpcReadUint32LE(struct ArrowIpcBufferView* data) {
+ uint32_t value;
+ memcpy(&value, data->data, sizeof(uint32_t));
+ // bswap32() if big endian
+ data->data += sizeof(uint32_t);
+ data->size_bytes -= sizeof(uint32_t);
+ return value;
+}
+
+static inline int32_t ArrowIpcReadInt32LE(struct ArrowIpcBufferView* data) {
+ int32_t value;
+ memcpy(&value, data->data, sizeof(int32_t));
+ // bswap32() if big endian
+ data->data += sizeof(int32_t);
+ data->size_bytes -= sizeof(int32_t);
+ return value;
+}
+
+#undef ns
+#define ns(x) FLATBUFFERS_WRAP_NAMESPACE(org_apache_arrow_flatbuf, x)
+
+static int ArrowIpcReaderDecodeSchema(struct ArrowIpcReader* reader,
+ flatbuffers_generic_t message_header,
+ struct ArrowIpcError* error) {
+ ns(Schema_table_t) schema = (ns(Schema_table_t))message_header;
+ int endianness = ns(Schema_endianness(schema));
+ switch (endianness) {
+ case ns(Endianness_Little):
+ reader->endianness = NANOARROW_IPC_ENDIANNESS_LITTLE;
+ break;
+ case ns(Endianness_Big):
+ reader->endianness = NANOARROW_IPC_ENDIANNESS_BIG;
+ break;
+ default:
+ ArrowIpcErrorSet(error,
+ "Expected Schema endianness of 0 (little) or 1 (big) but got %d",
+ (int)endianness);
+ }
+
+ ns(Feature_vec_t) features = ns(Schema_features(schema));
+ int64_t n_features = ns(Feature_vec_len(features));
+ reader->features = 0;
+
+ for (int64_t i = 0; i < n_features; i++) {
+ int feature = ns(Feature_vec_at(features, i));
+ switch (feature) {
+ case ns(Feature_COMPRESSED_BODY):
+ reader->features |= NANOARROW_IPC_FEATURE_COMPRESSED_BODY;
+ break;
+ case ns(Feature_DICTIONARY_REPLACEMENT):
+ reader->features |= NANOARROW_IPC_FEATURE_DICTIONARY_REPLACEMENT;
+ break;
+ default:
+ ArrowIpcErrorSet(error, "Unrecognized Schema feature with value %d",
+ (int)feature);
+ return EINVAL;
+ }
+ }
+
+ ns(Field_vec_t) fields = ns(Schema_fields(schema));
+ int64_t n_fields = ns(Schema_vec_len(fields));
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ ArrowSchemaInit(&reader->schema);
+ int result = ArrowSchemaSetTypeStruct(&reader->schema, n_fields);
+ if (result != NANOARROW_OK) {
+ ArrowIpcErrorSet(error, "Failed to allocate struct schema with %ld children",
+ (long)n_fields);
+ return result;
+ }
+
+ for (int64_t i = 0; i < n_fields; i++) {
+ ns(Field_table_t) field = ns(Field_vec_at(fields, i));
+ struct ArrowSchema* schema = reader->schema.children[i];
+
+ if (ns(Field_name_is_present(field))) {
+ result = ArrowSchemaSetName(schema, ns(Field_name_get(field)));
+ } else {
+ result = ArrowSchemaSetName(schema, "");
+ }
+
+ if (result != NANOARROW_OK) {
+ ArrowIpcErrorSet(error, "ArrowSchemaSetName() failed for schema field %ld",
+ (long)i);
+ return result;
+ }
+
+ if (ns(Field_nullable_get(field))) {
+ schema->flags |= ARROW_FLAG_NULLABLE;
+ }
+
+ int type_type = ns(Field_type_type(field));
+ switch (type_type) {
+ case ns(Type_Null):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_NA);
+ break;
+ case ns(Type_Int): {
+ ns(Int_table_t) field_int = (ns(Int_table_t))ns(Field_type_get(field));
+
+ int is_signed = ns(Int_is_signed_get(field_int));
+ int bitwidth = ns(Int_bitWidth_get(field_int));
+ int nanoarrow_type = NANOARROW_TYPE_UNINITIALIZED;
+
+ if (is_signed) {
+ switch (bitwidth) {
+ case 8:
+ nanoarrow_type = NANOARROW_TYPE_INT8;
+ break;
+ case 16:
+ nanoarrow_type = NANOARROW_TYPE_INT16;
+ break;
+ case 32:
+ nanoarrow_type = NANOARROW_TYPE_INT32;
+ break;
+ case 64:
+ nanoarrow_type = NANOARROW_TYPE_INT64;
+ break;
+ default:
+ break;
+ }
+ } else {
+ switch (bitwidth) {
+ case 8:
+ nanoarrow_type = NANOARROW_TYPE_UINT8;
+ break;
+ case 16:
+ nanoarrow_type = NANOARROW_TYPE_UINT16;
+ break;
+ case 32:
+ nanoarrow_type = NANOARROW_TYPE_UINT32;
+ break;
+ case 64:
+ nanoarrow_type = NANOARROW_TYPE_UINT64;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (nanoarrow_type == NANOARROW_TYPE_UNINITIALIZED) {
+ result = EINVAL;
+ } else {
+ result = ArrowSchemaSetType(schema, nanoarrow_type);
+ }
+
+ break;
+ }
+ case ns(Type_Utf8):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_STRING);
+ case ns(Type_LargeUtf8):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_LARGE_STRING);
+ break;
+ default:
+ ArrowIpcErrorSet(error, "Unrecognized Field type with value %d", (int)type_type);
+ return EINVAL;
Review Comment:
Not at all! I'd accumulated too much in one function and hadn't split them yet 🙂
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069833368
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
Review Comment:
It's a good question...probably the advantage of having it live in `arrow-nanoarrow/extensions` is that it can be distributed alongside nanoarrow.c/nanoarrow.h (i.e., all extensions get checked in to arrow-nanoarrow/dist just like the core C library). I did it this way to start off because that's how I would reccomend that another developer build a library on top of nanoarrow (vendor nanoarrow and don't expose nanoarrow.h to your user because they might be vendoring another version of it themselves).
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1380980847
I think this PR is at a good point for initial review! What it does is:
- Establishes a home for the IPC extension (the extensions/nanoarrow_ipc folder)
- Implements a minimum viable build configuration for the flatcc buffer configuration + unit testing with Arrow C++
- Implements the minimum number of type definitions to implement decoding and verifying a schema message: `ArrowIpcError`, `ArrowIpcBufferView`, `ArrowIpcMessageType`, `ArrowIpcEndianness`, and `ArrowIpcReader`.
- Implements three functions: `ArrowIpcReaderPeek()` (checks the first few bytes of the message and advances the data pointer past the end of the message), `ArrowIpcReaderVerify()` (runs flatbuffers verification and advances the data pointer past the end of the message on success), and `ArrowIpcReaderDecode()` (places information from the message into the `ArrowIpcReader` struct).
After this PR, there are a number of improvements that could be worked on more or less independently of one another:
- Make the build configuration more flexible (some of David's suggestions included ways to vendor the flatcc runtime and/or link to a shared flatcc)
- Add doxygen documentation + add it to the nanoarrow documentation build
- Implement the rest of the schema types
- Add the requisite GitHub actions (ctest, coverage, valgrind)
- Add R bindings so that actual humans can test this
- Implement RecordBatch message decoding
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1287398188
Just a quick thought, but I wonder if we might even want to go as far as vendoring the flatcc runtime in addition to checking in the generated sources
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069803263
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc_test.cc:
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <stdexcept>
+
+#include <arrow/array.h>
+#include <arrow/c/bridge.h>
+#include <gtest/gtest.h>
+
+#include "nanoarrow_ipc.h"
+
+using namespace arrow;
+
+// library(arrow, warn.conflicts = FALSE)
+
+// # R package doesn't do field metadata yet, so this hack is needed
+// field <- narrow::narrow_schema("i", "some_col", metadata = list("some_key_field" =
+// "some_value_field")) field <- Field$import_from_c(field)
+
+// schema <- arrow::schema(field)
+// schema$metadata <- list("some_key" = "some_value")
+// schema$serialize()
+static uint8_t kSimpleSchema[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x10, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x0a, 0x00, 0x0e, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x04, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x0c, 0x00,
+ 0x00, 0x00, 0x04, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x84, 0xff,
+ 0xff, 0xff, 0x18, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x00, 0x18, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x07, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x10, 0x00, 0x14, 0x00,
+ 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x14, 0x00, 0x00, 0x00, 0x70, 0x00,
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x08, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x63, 0x6f, 0x6c, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00,
+ 0x04, 0x00, 0x08, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c,
+ 0x75, 0x65, 0x5f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x5f, 0x66, 0x69, 0x65,
+ 0x6c, 0x64, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00, 0x08, 0x00, 0x07, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) {
+ struct ArrowIpcReader reader;
+ struct ArrowIpcError error;
+
+ struct ArrowIpcBufferView data;
+ data.data = kSimpleSchema;
+ data.size_bytes = 1;
+
+ ArrowIpcReaderInit(&reader);
+
+ EXPECT_EQ(ArrowIpcReaderVerify(&reader, &data, &error), EINVAL);
+ EXPECT_STREQ(error.message,
+ "Expected data of at least 8 bytes but only 1 bytes remain");
+ EXPECT_EQ(data.data, kSimpleSchema);
+ EXPECT_EQ(data.size_bytes, 1);
Review Comment:
Yeah, I was thinking one could iterate through like
```c
int result;
while ((result = ArrowIpcReaderVerify(&reader, &data, &error)) == NANOARROW_OK) {
// ...
}
```
(but maybe that's more confusing than useful)
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1384386881
Are there any more changes that would be useful in the scope of this initial 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
[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069844694
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
Review Comment:
Ah, I guess the question is, is this a separately library using nanoarrow internally, or is it just an optional part of nanoarrow?
I don't mind this not exposing nanoarrow. But this is technically undefined behavior here, as I understand.
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1002158637
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.h:
##########
@@ -0,0 +1,153 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef NANOARROW_NANOARROW_IPC_H_INCLUDED
+#define NANOARROW_NANOARROW_IPC_H_INCLUDED
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Extra guard for versions of Arrow without the canonical guard
+#ifndef ARROW_FLAG_DICTIONARY_ORDERED
+
+/// \defgroup nanoarrow-arrow-cdata
+///
+/// The Arrow C Data (https://arrow.apache.org/docs/format/CDataInterface.html)
+/// and Arrow C Stream (https://arrow.apache.org/docs/format/CStreamInterface.html)
+/// interfaces are part of the
+/// Arrow Columnar Format specification
+/// (https://arrow.apache.org/docs/format/Columnar.html). See the Arrow documentation for
+/// documentation of these structures.
+///
+/// @{
+
+#ifndef ARROW_C_DATA_INTERFACE
+#define ARROW_C_DATA_INTERFACE
+
+#define ARROW_FLAG_DICTIONARY_ORDERED 1
+#define ARROW_FLAG_NULLABLE 2
+#define ARROW_FLAG_MAP_KEYS_SORTED 4
+
+struct ArrowSchema {
+ // Array type description
+ const char* format;
+ const char* name;
+ const char* metadata;
+ int64_t flags;
+ int64_t n_children;
+ struct ArrowSchema** children;
+ struct ArrowSchema* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowSchema*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+struct ArrowArray {
+ // Array data description
+ int64_t length;
+ int64_t null_count;
+ int64_t offset;
+ int64_t n_buffers;
+ int64_t n_children;
+ const void** buffers;
+ struct ArrowArray** children;
+ struct ArrowArray* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowArray*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_DATA_INTERFACE
+
+#ifndef ARROW_C_STREAM_INTERFACE
+#define ARROW_C_STREAM_INTERFACE
+
+struct ArrowArrayStream {
+ // Callback to get the stream type
+ // (will be the same for all arrays in the stream).
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowSchema must be released independently from the stream.
+ int (*get_schema)(struct ArrowArrayStream*, struct ArrowSchema* out);
+
+ // Callback to get the next array
+ // (if no error and the array is released, the stream has ended)
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowArray must be released independently from the stream.
+ int (*get_next)(struct ArrowArrayStream*, struct ArrowArray* out);
+
+ // Callback to get optional detailed error information.
+ // This must only be called if the last stream operation failed
+ // with a non-0 return code.
+ //
+ // Return value: pointer to a null-terminated character array describing
+ // the last error, or NULL if no description is available.
+ //
+ // The returned pointer is only valid until the next operation on this stream
+ // (including release).
+ const char* (*get_last_error)(struct ArrowArrayStream*);
+
+ // Release callback: release the stream's own resources.
+ // Note that arrays returned by `get_next` must be individually released.
+ void (*release)(struct ArrowArrayStream*);
+
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_STREAM_INTERFACE
+#endif // ARROW_FLAG_DICTIONARY_ORDERED
+
+#ifdef __cplusplus
+}
+#endif
+
+typedef int ArrowIpcErrorCode;
+
+struct ArrowIpcIO {
Review Comment:
An alternative might be a push-based API, like [StreamDecoder](https://github.com/apache/arrow/blob/4ce64537e5ee223bf5eea91284efc46f214d6c76/cpp/src/arrow/ipc/reader.h#L309)? (Except maybe instead of a callback, require the user to poll the reader object)
--
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
[GitHub] [arrow-nanoarrow] paleolimbot merged pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot merged PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1003251733
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.h:
##########
@@ -0,0 +1,153 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef NANOARROW_NANOARROW_IPC_H_INCLUDED
+#define NANOARROW_NANOARROW_IPC_H_INCLUDED
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Extra guard for versions of Arrow without the canonical guard
+#ifndef ARROW_FLAG_DICTIONARY_ORDERED
+
+/// \defgroup nanoarrow-arrow-cdata
+///
+/// The Arrow C Data (https://arrow.apache.org/docs/format/CDataInterface.html)
+/// and Arrow C Stream (https://arrow.apache.org/docs/format/CStreamInterface.html)
+/// interfaces are part of the
+/// Arrow Columnar Format specification
+/// (https://arrow.apache.org/docs/format/Columnar.html). See the Arrow documentation for
+/// documentation of these structures.
+///
+/// @{
+
+#ifndef ARROW_C_DATA_INTERFACE
+#define ARROW_C_DATA_INTERFACE
+
+#define ARROW_FLAG_DICTIONARY_ORDERED 1
+#define ARROW_FLAG_NULLABLE 2
+#define ARROW_FLAG_MAP_KEYS_SORTED 4
+
+struct ArrowSchema {
+ // Array type description
+ const char* format;
+ const char* name;
+ const char* metadata;
+ int64_t flags;
+ int64_t n_children;
+ struct ArrowSchema** children;
+ struct ArrowSchema* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowSchema*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+struct ArrowArray {
+ // Array data description
+ int64_t length;
+ int64_t null_count;
+ int64_t offset;
+ int64_t n_buffers;
+ int64_t n_children;
+ const void** buffers;
+ struct ArrowArray** children;
+ struct ArrowArray* dictionary;
+
+ // Release callback
+ void (*release)(struct ArrowArray*);
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_DATA_INTERFACE
+
+#ifndef ARROW_C_STREAM_INTERFACE
+#define ARROW_C_STREAM_INTERFACE
+
+struct ArrowArrayStream {
+ // Callback to get the stream type
+ // (will be the same for all arrays in the stream).
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowSchema must be released independently from the stream.
+ int (*get_schema)(struct ArrowArrayStream*, struct ArrowSchema* out);
+
+ // Callback to get the next array
+ // (if no error and the array is released, the stream has ended)
+ //
+ // Return value: 0 if successful, an `errno`-compatible error code otherwise.
+ //
+ // If successful, the ArrowArray must be released independently from the stream.
+ int (*get_next)(struct ArrowArrayStream*, struct ArrowArray* out);
+
+ // Callback to get optional detailed error information.
+ // This must only be called if the last stream operation failed
+ // with a non-0 return code.
+ //
+ // Return value: pointer to a null-terminated character array describing
+ // the last error, or NULL if no description is available.
+ //
+ // The returned pointer is only valid until the next operation on this stream
+ // (including release).
+ const char* (*get_last_error)(struct ArrowArrayStream*);
+
+ // Release callback: release the stream's own resources.
+ // Note that arrays returned by `get_next` must be individually released.
+ void (*release)(struct ArrowArrayStream*);
+
+ // Opaque producer-specific data
+ void* private_data;
+};
+
+#endif // ARROW_C_STREAM_INTERFACE
+#endif // ARROW_FLAG_DICTIONARY_ORDERED
+
+#ifdef __cplusplus
+}
+#endif
+
+typedef int ArrowIpcErrorCode;
+
+struct ArrowIpcIO {
Review Comment:
That could be implemented using the pull-based stuff I have here, no? (Most file APIs I know about are pull based, which makes that a good target for the initial implementation).
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069961025
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
+
+void ArrowIpcReaderInit(struct ArrowIpcReader* reader) {
+ memset(reader, 0, sizeof(struct ArrowIpcReader));
+}
+
+void ArrowIpcReaderReset(struct ArrowIpcReader* reader) {
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ ArrowIpcReaderInit(reader);
+}
+
+static inline uint32_t ArrowIpcReadUint32LE(struct ArrowIpcBufferView* data) {
+ uint32_t value;
+ memcpy(&value, data->data, sizeof(uint32_t));
+ // bswap32() if big endian
+ data->data += sizeof(uint32_t);
+ data->size_bytes -= sizeof(uint32_t);
+ return value;
+}
+
+static inline int32_t ArrowIpcReadInt32LE(struct ArrowIpcBufferView* data) {
+ int32_t value;
+ memcpy(&value, data->data, sizeof(int32_t));
+ // bswap32() if big endian
+ data->data += sizeof(int32_t);
+ data->size_bytes -= sizeof(int32_t);
+ return value;
+}
+
+#undef ns
Review Comment:
I think I copied it from the documentation (but it's confusing and gone now!)
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1382318152
@sfc-gh-aling FYI!
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1067352883
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,297 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
+
+void ArrowIpcReaderInit(struct ArrowIpcReader* reader) {
+ memset(reader, 0, sizeof(struct ArrowIpcReader));
+}
+
+void ArrowIpcReaderReset(struct ArrowIpcReader* reader) {
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ if (reader->batch_index.release != NULL) {
+ reader->batch_index.release(&reader->batch_index);
+ }
+
+ ArrowIpcReaderInit(reader);
+}
+
+static inline uint32_t ArrowIpcReadUint32LE(struct ArrowIpcBufferView* data) {
+ uint32_t value;
+ memcpy(&value, data->data, sizeof(uint32_t));
+ // bswap32() if big endian
+ data->data += sizeof(uint32_t);
+ data->size_bytes -= sizeof(uint32_t);
+ return value;
+}
+
+static inline int32_t ArrowIpcReadInt32LE(struct ArrowIpcBufferView* data) {
+ int32_t value;
+ memcpy(&value, data->data, sizeof(int32_t));
+ // bswap32() if big endian
+ data->data += sizeof(int32_t);
+ data->size_bytes -= sizeof(int32_t);
+ return value;
+}
+
+#undef ns
+#define ns(x) FLATBUFFERS_WRAP_NAMESPACE(org_apache_arrow_flatbuf, x)
+
+static int ArrowIpcReaderDecodeSchema(struct ArrowIpcReader* reader,
+ flatbuffers_generic_t message_header,
+ struct ArrowIpcError* error) {
+ ns(Schema_table_t) schema = (ns(Schema_table_t))message_header;
+ int endianness = ns(Schema_endianness(schema));
+ switch (endianness) {
+ case ns(Endianness_Little):
+ reader->endianness = NANOARROW_IPC_ENDIANNESS_LITTLE;
+ break;
+ case ns(Endianness_Big):
+ reader->endianness = NANOARROW_IPC_ENDIANNESS_BIG;
+ break;
+ default:
+ ArrowIpcErrorSet(error,
+ "Expected Schema endianness of 0 (little) or 1 (big) but got %d",
+ (int)endianness);
+ }
+
+ ns(Feature_vec_t) features = ns(Schema_features(schema));
+ int64_t n_features = ns(Feature_vec_len(features));
+ reader->features = 0;
+
+ for (int64_t i = 0; i < n_features; i++) {
+ int feature = ns(Feature_vec_at(features, i));
+ switch (feature) {
+ case ns(Feature_COMPRESSED_BODY):
+ reader->features |= NANOARROW_IPC_FEATURE_COMPRESSED_BODY;
+ break;
+ case ns(Feature_DICTIONARY_REPLACEMENT):
+ reader->features |= NANOARROW_IPC_FEATURE_DICTIONARY_REPLACEMENT;
+ break;
+ default:
+ ArrowIpcErrorSet(error, "Unrecognized Schema feature with value %d",
+ (int)feature);
+ return EINVAL;
+ }
+ }
+
+ ns(Field_vec_t) fields = ns(Schema_fields(schema));
+ int64_t n_fields = ns(Schema_vec_len(fields));
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ ArrowSchemaInit(&reader->schema);
+ int result = ArrowSchemaSetTypeStruct(&reader->schema, n_fields);
+ if (result != NANOARROW_OK) {
+ ArrowIpcErrorSet(error, "Failed to allocate struct schema with %ld children",
+ (long)n_fields);
+ return result;
+ }
+
+ for (int64_t i = 0; i < n_fields; i++) {
+ ns(Field_table_t) field = ns(Field_vec_at(fields, i));
+ struct ArrowSchema* schema = reader->schema.children[i];
+
+ if (ns(Field_name_is_present(field))) {
+ result = ArrowSchemaSetName(schema, ns(Field_name_get(field)));
+ } else {
+ result = ArrowSchemaSetName(schema, "");
+ }
+
+ if (result != NANOARROW_OK) {
+ ArrowIpcErrorSet(error, "ArrowSchemaSetName() failed for schema field %ld",
+ (long)i);
+ return result;
+ }
+
+ if (ns(Field_nullable_get(field))) {
+ schema->flags |= ARROW_FLAG_NULLABLE;
+ }
+
+ int type_type = ns(Field_type_type(field));
+ switch (type_type) {
+ case ns(Type_Null):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_NA);
+ break;
+ case ns(Type_Int): {
+ ns(Int_table_t) field_int = (ns(Int_table_t))ns(Field_type_get(field));
+
+ int is_signed = ns(Int_is_signed_get(field_int));
+ int bitwidth = ns(Int_bitWidth_get(field_int));
+ int nanoarrow_type = NANOARROW_TYPE_UNINITIALIZED;
+
+ if (is_signed) {
+ switch (bitwidth) {
+ case 8:
+ nanoarrow_type = NANOARROW_TYPE_INT8;
+ break;
+ case 16:
+ nanoarrow_type = NANOARROW_TYPE_INT16;
+ break;
+ case 32:
+ nanoarrow_type = NANOARROW_TYPE_INT32;
+ break;
+ case 64:
+ nanoarrow_type = NANOARROW_TYPE_INT64;
+ break;
+ default:
+ break;
+ }
+ } else {
+ switch (bitwidth) {
+ case 8:
+ nanoarrow_type = NANOARROW_TYPE_UINT8;
+ break;
+ case 16:
+ nanoarrow_type = NANOARROW_TYPE_UINT16;
+ break;
+ case 32:
+ nanoarrow_type = NANOARROW_TYPE_UINT32;
+ break;
+ case 64:
+ nanoarrow_type = NANOARROW_TYPE_UINT64;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (nanoarrow_type == NANOARROW_TYPE_UNINITIALIZED) {
+ result = EINVAL;
+ } else {
+ result = ArrowSchemaSetType(schema, nanoarrow_type);
+ }
+
+ break;
+ }
+ case ns(Type_Utf8):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_STRING);
+ case ns(Type_LargeUtf8):
+ result = ArrowSchemaSetType(schema, NANOARROW_TYPE_LARGE_STRING);
+ break;
+ default:
+ ArrowIpcErrorSet(error, "Unrecognized Field type with value %d", (int)type_type);
+ return EINVAL;
Review Comment:
nit, but is there a reason to mix early-return and single-return error handling here?
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069803263
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc_test.cc:
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <stdexcept>
+
+#include <arrow/array.h>
+#include <arrow/c/bridge.h>
+#include <gtest/gtest.h>
+
+#include "nanoarrow_ipc.h"
+
+using namespace arrow;
+
+// library(arrow, warn.conflicts = FALSE)
+
+// # R package doesn't do field metadata yet, so this hack is needed
+// field <- narrow::narrow_schema("i", "some_col", metadata = list("some_key_field" =
+// "some_value_field")) field <- Field$import_from_c(field)
+
+// schema <- arrow::schema(field)
+// schema$metadata <- list("some_key" = "some_value")
+// schema$serialize()
+static uint8_t kSimpleSchema[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x10, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x0a, 0x00, 0x0e, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x04, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x0c, 0x00,
+ 0x00, 0x00, 0x04, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x84, 0xff,
+ 0xff, 0xff, 0x18, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x00, 0x18, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x07, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x10, 0x00, 0x14, 0x00,
+ 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x14, 0x00, 0x00, 0x00, 0x70, 0x00,
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x08, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x63, 0x6f, 0x6c, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00,
+ 0x04, 0x00, 0x08, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c,
+ 0x75, 0x65, 0x5f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x5f, 0x66, 0x69, 0x65,
+ 0x6c, 0x64, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00, 0x08, 0x00, 0x07, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) {
+ struct ArrowIpcReader reader;
+ struct ArrowIpcError error;
+
+ struct ArrowIpcBufferView data;
+ data.data = kSimpleSchema;
+ data.size_bytes = 1;
+
+ ArrowIpcReaderInit(&reader);
+
+ EXPECT_EQ(ArrowIpcReaderVerify(&reader, &data, &error), EINVAL);
+ EXPECT_STREQ(error.message,
+ "Expected data of at least 8 bytes but only 1 bytes remain");
+ EXPECT_EQ(data.data, kSimpleSchema);
+ EXPECT_EQ(data.size_bytes, 1);
Review Comment:
Yeah, I was thinking one could iterate through like
```c
int result;
while (result = ArrowIpcReaderVerify(&reader, &data, &error)) {
// ...
}
```
(but maybe that's more confusing than useful)
--
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
[GitHub] [arrow-nanoarrow] paleolimbot commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069950248
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
Review Comment:
I copied over nanoarrow's set method for now but the next PR should decide whether or not "extensions" can/should `#include "nanoarrow.h"` in their public header. It's a lot easier if they can; however, I think there was some email thread where somebody said it was a bad idea.
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#discussion_r1069589254
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc_test.cc:
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <stdexcept>
+
+#include <arrow/array.h>
+#include <arrow/c/bridge.h>
+#include <gtest/gtest.h>
+
+#include "nanoarrow_ipc.h"
+
+using namespace arrow;
+
+// library(arrow, warn.conflicts = FALSE)
+
+// # R package doesn't do field metadata yet, so this hack is needed
+// field <- narrow::narrow_schema("i", "some_col", metadata = list("some_key_field" =
+// "some_value_field")) field <- Field$import_from_c(field)
+
+// schema <- arrow::schema(field)
+// schema$metadata <- list("some_key" = "some_value")
+// schema$serialize()
+static uint8_t kSimpleSchema[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x10, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x0a, 0x00, 0x0e, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x04, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x0c, 0x00,
+ 0x00, 0x00, 0x04, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x84, 0xff,
+ 0xff, 0xff, 0x18, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x00, 0x18, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x07, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x10, 0x00, 0x14, 0x00,
+ 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x14, 0x00, 0x00, 0x00, 0x70, 0x00,
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x08, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x63, 0x6f, 0x6c, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00,
+ 0x04, 0x00, 0x08, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c,
+ 0x75, 0x65, 0x5f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x5f, 0x66, 0x69, 0x65,
+ 0x6c, 0x64, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00, 0x08, 0x00, 0x07, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) {
Review Comment:
Sometime down the line, these are the sort of tests where a fuzzer would be useful
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
+
+void ArrowIpcReaderInit(struct ArrowIpcReader* reader) {
+ memset(reader, 0, sizeof(struct ArrowIpcReader));
+}
+
+void ArrowIpcReaderReset(struct ArrowIpcReader* reader) {
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ ArrowIpcReaderInit(reader);
+}
+
+static inline uint32_t ArrowIpcReadUint32LE(struct ArrowIpcBufferView* data) {
+ uint32_t value;
+ memcpy(&value, data->data, sizeof(uint32_t));
+ // bswap32() if big endian
+ data->data += sizeof(uint32_t);
+ data->size_bytes -= sizeof(uint32_t);
+ return value;
+}
+
+static inline int32_t ArrowIpcReadInt32LE(struct ArrowIpcBufferView* data) {
+ int32_t value;
+ memcpy(&value, data->data, sizeof(int32_t));
+ // bswap32() if big endian
+ data->data += sizeof(int32_t);
+ data->size_bytes -= sizeof(int32_t);
+ return value;
+}
+
+#undef ns
Review Comment:
hmm, where did the original declaration come from?
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
Review Comment:
Hmm, is this actually useful? (Also technically, I think you're not supposed to pun between two structs just because they happen to have the same fields...)
What do you think about having nanoarrow_ipc.h just include nanoarrow.h, and just reuse existing utilities?
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc_test.cc:
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <stdexcept>
+
+#include <arrow/array.h>
+#include <arrow/c/bridge.h>
+#include <gtest/gtest.h>
+
+#include "nanoarrow_ipc.h"
+
+using namespace arrow;
+
+// library(arrow, warn.conflicts = FALSE)
+
+// # R package doesn't do field metadata yet, so this hack is needed
+// field <- narrow::narrow_schema("i", "some_col", metadata = list("some_key_field" =
+// "some_value_field")) field <- Field$import_from_c(field)
+
+// schema <- arrow::schema(field)
+// schema$metadata <- list("some_key" = "some_value")
+// schema$serialize()
+static uint8_t kSimpleSchema[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x10, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x0a, 0x00, 0x0e, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x04, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x0c, 0x00,
+ 0x00, 0x00, 0x04, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x84, 0xff,
+ 0xff, 0xff, 0x18, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x00, 0x18, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x07, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x10, 0x00, 0x14, 0x00,
+ 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x14, 0x00, 0x00, 0x00, 0x70, 0x00,
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x08, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x63, 0x6f, 0x6c, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00,
+ 0x04, 0x00, 0x08, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c,
+ 0x75, 0x65, 0x5f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x5f, 0x66, 0x69, 0x65,
+ 0x6c, 0x64, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00, 0x08, 0x00, 0x07, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) {
+ struct ArrowIpcReader reader;
+ struct ArrowIpcError error;
+
+ struct ArrowIpcBufferView data;
+ data.data = kSimpleSchema;
+ data.size_bytes = 1;
+
+ ArrowIpcReaderInit(&reader);
+
+ EXPECT_EQ(ArrowIpcReaderVerify(&reader, &data, &error), EINVAL);
+ EXPECT_STREQ(error.message,
+ "Expected data of at least 8 bytes but only 1 bytes remain");
+ EXPECT_EQ(data.data, kSimpleSchema);
+ EXPECT_EQ(data.size_bytes, 1);
Review Comment:
Declaring the pointers as `const` in the API signatures would let you make this assurance statically
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc.c:
##########
@@ -0,0 +1,352 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <errno.h>
+#include <string.h>
+
+#include "nanoarrow/nanoarrow.h"
+
+#include "File_reader.h"
+#include "Message_reader.h"
+#include "Message_verifier.h"
+#include "Schema_reader.h"
+
+#include "nanoarrow_ipc.h"
+
+#define ArrowIpcErrorSet(err, ...) ArrowErrorSet((struct ArrowError*)err, __VA_ARGS__)
+
+void ArrowIpcReaderInit(struct ArrowIpcReader* reader) {
+ memset(reader, 0, sizeof(struct ArrowIpcReader));
+}
+
+void ArrowIpcReaderReset(struct ArrowIpcReader* reader) {
+ if (reader->schema.release != NULL) {
+ reader->schema.release(&reader->schema);
+ }
+
+ ArrowIpcReaderInit(reader);
+}
+
+static inline uint32_t ArrowIpcReadUint32LE(struct ArrowIpcBufferView* data) {
+ uint32_t value;
+ memcpy(&value, data->data, sizeof(uint32_t));
+ // bswap32() if big endian
Review Comment:
did you mean to put a TODO here?
##########
extensions/nanoarrow_ipc/src/nanoarrow_ipc/nanoarrow_ipc_test.cc:
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <stdexcept>
+
+#include <arrow/array.h>
+#include <arrow/c/bridge.h>
+#include <gtest/gtest.h>
+
+#include "nanoarrow_ipc.h"
+
+using namespace arrow;
+
+// library(arrow, warn.conflicts = FALSE)
+
+// # R package doesn't do field metadata yet, so this hack is needed
+// field <- narrow::narrow_schema("i", "some_col", metadata = list("some_key_field" =
+// "some_value_field")) field <- Field$import_from_c(field)
+
+// schema <- arrow::schema(field)
+// schema$metadata <- list("some_key" = "some_value")
+// schema$serialize()
+static uint8_t kSimpleSchema[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x10, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x0a, 0x00, 0x0e, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x04, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x0c, 0x00,
+ 0x00, 0x00, 0x04, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x84, 0xff,
+ 0xff, 0xff, 0x18, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00,
+ 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x00, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x12, 0x00, 0x18, 0x00,
+ 0x08, 0x00, 0x06, 0x00, 0x07, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x10, 0x00, 0x14, 0x00,
+ 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x14, 0x00, 0x00, 0x00, 0x70, 0x00,
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x08, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x63, 0x6f, 0x6c, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00,
+ 0x04, 0x00, 0x08, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x04, 0x00,
+ 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x76, 0x61, 0x6c,
+ 0x75, 0x65, 0x5f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00,
+ 0x00, 0x00, 0x73, 0x6f, 0x6d, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x5f, 0x66, 0x69, 0x65,
+ 0x6c, 0x64, 0x00, 0x00, 0x08, 0x00, 0x0c, 0x00, 0x08, 0x00, 0x07, 0x00, 0x08, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) {
+ struct ArrowIpcReader reader;
+ struct ArrowIpcError error;
+
+ struct ArrowIpcBufferView data;
+ data.data = kSimpleSchema;
+ data.size_bytes = 1;
+
+ ArrowIpcReaderInit(&reader);
+
+ EXPECT_EQ(ArrowIpcReaderVerify(&reader, &data, &error), EINVAL);
+ EXPECT_STREQ(error.message,
+ "Expected data of at least 8 bytes but only 1 bytes remain");
+ EXPECT_EQ(data.data, kSimpleSchema);
+ EXPECT_EQ(data.size_bytes, 1);
Review Comment:
Ah, though I see down below that it does mutate the buffer in some cases, so never mind.
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1384429509
Sorry, forgot to reply here. I think let's merge this and we can continue work in follow up PRs. I think the main question to resolve is whether this should `#include <nanoarrow/nanoarrow.h>` or not.
--
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
[GitHub] [arrow-nanoarrow] lidavidm commented on pull request #61: Proof-of-concept IPC reader/writer
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #61:
URL: https://github.com/apache/arrow-nanoarrow/pull/61#issuecomment-1288977477
We could perhaps keep the flatcc runtime separate, so that you would just not include it if you're also using flatcc yourself, but otherwise it's right there and you don't need to fetch+build a third party repo (that said we should probably see how much work it is to do that in the first place, maybe it's not a big deal)
--
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