You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/11 20:18:35 UTC
[GitHub] [arrow-nanoarrow] paleolimbot opened a new pull request, #19: ArrowArray consumer buffer helpers
paleolimbot opened a new pull request, #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19
Implements a few types whose goal is to provide access to buffers. This will require a `switch(storage_type)` by the consumer and requires some knowledge of the spec (getters to parallel the setters in #16 is another option for a future 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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-nanoarrow] paleolimbot merged pull request #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
paleolimbot merged PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19
--
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: issues-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 #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r944649830
##########
src/nanoarrow/utils_inline.h:
##########
@@ -26,6 +26,115 @@
extern "C" {
#endif
+static inline void ArrowLayoutInit(struct ArrowLayout* layout,
Review Comment:
Done! I'm sure it *could* be header-only but that's a discussion/battle for another day that I'm not all that qualified to weigh in on.
--
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: issues-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 #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r944654967
##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -166,6 +166,32 @@ enum ArrowType {
NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO
};
+/// \brief Functional types of buffers as described in the Arrow Columnar Specification
+enum ArrowBufferType {
+ NANOARROW_BUFFER_TYPE_NONE,
+ NANOARROW_BUFFER_TYPE_VALIDITY,
+ NANOARROW_BUFFER_TYPE_TYPE_ID,
+ NANOARROW_BUFFER_TYPE_UNION_OFFSET,
+ NANOARROW_BUFFER_TYPE_DATA_OFFSET,
+ NANOARROW_BUFFER_TYPE_DATA
+};
+
+/// \brief A description of an arrangement of buffers
+///
+/// Contains the minimum amount of information required to
+/// calculate the size of each buffer in an ArrowArray knowing only
+/// the length and offset of the array.
+struct ArrowLayout {
+ /// \brief The function of each buffer
+ enum ArrowBufferType buffer_type[3];
+
+ /// \brief The size of an element each buffer or 0 if this size is variable or unknown
+ int64_t element_size_bits[3];
+
+ /// \brief The fixed size of a child element
+ int64_t child_size_elements;
Review Comment:
How is this different from `element_size_bits`?
--
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: issues-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 #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r944646506
##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -179,6 +217,19 @@ struct ArrowStringView {
int64_t n_bytes;
};
+/// \brief An non-owning view of a buffer
+struct ArrowBufferView {
+ /// \brief A pointer to the start of the buffer
+ ///
+ /// If n_bytes is 0, this value may be NULL.
+ const union ArrowBufferDataPointer data;
+
+ /// \brief The size of the string in bytes,
+ ///
+ /// (Not including the null terminator.)
+ int64_t n_bytes;
Review Comment:
Done!
--
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: issues-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 #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r943929415
##########
src/nanoarrow/utils_inline.h:
##########
@@ -26,6 +26,115 @@
extern "C" {
#endif
+static inline void ArrowLayoutInit(struct ArrowLayout* layout,
Review Comment:
Just an observation, but I think this sort of function doesn't necessarily have to be inline - it shouldn't be called frequently and shouldn't be a bottleneck
##########
src/nanoarrow/utils_inline.h:
##########
@@ -26,6 +26,115 @@
extern "C" {
#endif
+static inline void ArrowLayoutInit(struct ArrowLayout* layout,
Review Comment:
Or, of course, you could go the other way and put _everything_ in headers, making this a header-only library
##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -179,6 +217,19 @@ struct ArrowStringView {
int64_t n_bytes;
};
+/// \brief An non-owning view of a buffer
+struct ArrowBufferView {
+ /// \brief A pointer to the start of the buffer
+ ///
+ /// If n_bytes is 0, this value may be NULL.
+ const union ArrowBufferDataPointer data;
+
+ /// \brief The size of the string in bytes,
+ ///
+ /// (Not including the null terminator.)
+ int64_t n_bytes;
Review Comment:
```suggestion
/// \brief The size of the buffer in bytes.
int64_t n_bytes;
```
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-nanoarrow] codecov-commenter commented on pull request #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#issuecomment-1213263144
# [Codecov](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e393e32) into [main](https://codecov.io/gh/apache/arrow-nanoarrow/commit/3b305075d2c4c8489ac0e6288edb58a52a64884d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3b30507) will **increase** coverage by `0.44%`.
> The diff coverage is `95.37%`.
```diff
@@ Coverage Diff @@
## main #19 +/- ##
==========================================
+ Coverage 90.64% 91.09% +0.44%
==========================================
Files 9 10 +1
Lines 1037 1145 +108
Branches 43 46 +3
==========================================
+ Hits 940 1043 +103
- Misses 63 66 +3
- Partials 34 36 +2
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/nanoarrow/array\_view.c](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL25hbm9hcnJvdy9hcnJheV92aWV3LmM=) | `82.75% <82.75%> (ø)` | |
| [src/nanoarrow/schema\_view.c](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL25hbm9hcnJvdy9zY2hlbWFfdmlldy5j) | `98.88% <100.00%> (+0.01%)` | :arrow_up: |
| [src/nanoarrow/utils\_inline.h](https://codecov.io/gh/apache/arrow-nanoarrow/pull/19/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL25hbm9hcnJvdy91dGlsc19pbmxpbmUuaA==) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-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 #19: ArrowArray consumer buffer helpers
Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r944665393
##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -166,6 +166,32 @@ enum ArrowType {
NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO
};
+/// \brief Functional types of buffers as described in the Arrow Columnar Specification
+enum ArrowBufferType {
+ NANOARROW_BUFFER_TYPE_NONE,
+ NANOARROW_BUFFER_TYPE_VALIDITY,
+ NANOARROW_BUFFER_TYPE_TYPE_ID,
+ NANOARROW_BUFFER_TYPE_UNION_OFFSET,
+ NANOARROW_BUFFER_TYPE_DATA_OFFSET,
+ NANOARROW_BUFFER_TYPE_DATA
+};
+
+/// \brief A description of an arrangement of buffers
+///
+/// Contains the minimum amount of information required to
+/// calculate the size of each buffer in an ArrowArray knowing only
+/// the length and offset of the array.
+struct ArrowLayout {
+ /// \brief The function of each buffer
+ enum ArrowBufferType buffer_type[3];
+
+ /// \brief The size of an element each buffer or 0 if this size is variable or unknown
+ int64_t element_size_bits[3];
+
+ /// \brief The fixed size of a child element
+ int64_t child_size_elements;
Review Comment:
It's needed to calculate the length of a child of a fixed-size list (I should clarify + add a test for that, though, to make sure).
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org