[GitHub] [arrow] pitrou commented on a change in pull request #8052: ARROW-9761: [C/C++] Add experimental C ArrowArrayStream ABI

2020-08-26 Thread GitBox


pitrou commented on a change in pull request #8052:
URL: https://github.com/apache/arrow/pull/8052#discussion_r477293832



##
File path: cpp/src/arrow/c/abi.h
##
@@ -60,6 +60,31 @@ struct ArrowArray {
   void* private_data;
 };
 
+// EXPERIMENTAL
+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.
+  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.

Review comment:
   I don't know how to phrase it: it returns value that are `errno` error 
codes (in case of error). A number of values are standard in C++: 
https://en.cppreference.com/w/cpp/error/errno_macros
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] pitrou commented on a change in pull request #8052: ARROW-9761: [C/C++] Add experimental C ArrowArrayStream ABI

2020-08-26 Thread GitBox


pitrou commented on a change in pull request #8052:
URL: https://github.com/apache/arrow/pull/8052#discussion_r477295669



##
File path: cpp/src/arrow/c/abi.h
##
@@ -60,6 +60,31 @@ struct ArrowArray {
   void* private_data;
 };
 
+// EXPERIMENTAL
+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.
+  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)

Review comment:
   A nullptr cannot be returned. The callback returns an `int`. However, we 
could say that returning `-1` means end of stream.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] pitrou commented on a change in pull request #8052: ARROW-9761: [C/C++] Add experimental C ArrowArrayStream ABI

2020-08-26 Thread GitBox


pitrou commented on a change in pull request #8052:
URL: https://github.com/apache/arrow/pull/8052#discussion_r477293832



##
File path: cpp/src/arrow/c/abi.h
##
@@ -60,6 +60,31 @@ struct ArrowArray {
   void* private_data;
 };
 
+// EXPERIMENTAL
+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.
+  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.

Review comment:
   I don't how to phrase it: it returns value that are `errno` error codes 
(in case of error). A number of values are standard in C++: 
https://en.cppreference.com/w/cpp/error/errno_macros
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [arrow] pitrou commented on a change in pull request #8052: ARROW-9761: [C/C++] Add experimental C ArrowArrayStream ABI

2020-08-26 Thread GitBox


pitrou commented on a change in pull request #8052:
URL: https://github.com/apache/arrow/pull/8052#discussion_r477294326



##
File path: cpp/src/arrow/c/abi.h
##
@@ -60,6 +60,31 @@ struct ArrowArray {
   void* private_data;
 };
 
+// EXPERIMENTAL
+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.
+  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.
+  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.  The returned pointer is only valid until
+  // the next operation on this stream (including release).
+  // If unavailable, NULL is returned.
+  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.

Review comment:
   Hmm... I'd say no.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org