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/03 18:21:13 UTC

[GitHub] [arrow-nanoarrow] paleolimbot opened a new pull request, #12: Add metadata builder functions

paleolimbot opened a new pull request, #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12

   Fixes #6.
   
   This PR adds functions `ArrowMetadataBuilderAppend()` (for blindly but efficiently adding a key/value pair to the end of some metadata) and `ArrowMetadataBuilderSet()` (to less efficiently replace or remove a value for key).
   
   The use case I had in mind is building extension type metadata from some input (i.e., make an output schema like the input except with new extension type or with new serialized extension type metadata). It's rather difficult to replicate the "replace" or "remove" behaviour otherwise.


-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940548127


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   Wonder if a helper macro to convert `const char*` to `struct ArrowStringView` might help work around that?



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940552993


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   I added `ArrowCharView()`:
   
   https://github.com/apache/arrow-nanoarrow/blob/1a4bb88165ad659c0db274df24a44987c8a81bf3/src/nanoarrow/nanoarrow.h#L237-L238
   
   It's still a little awkward feeling if I convert keys to use `struct ArrowStringView` but I don't really mind either way.



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r941545043


##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -166,6 +166,34 @@ enum ArrowType {
   NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO
 };
 
+/// \brief An non-owning view of a string
+struct ArrowStringView {
+  /// \brief A pointer to the start of the string
+  ///
+  /// If n_bytes is 0, this value may be NULL.
+  const char* data;
+
+  /// \brief The size of the string in bytes,
+  ///
+  /// (Not including the null terminator.)
+  int64_t n_bytes;
+};
+
+ /// \brief Create a string view from a null-terminated string
+static inline struct ArrowStringView ArrowCharView(const char* value) {
+  struct ArrowStringView out;
+
+  out.data = value;
+  out.n_bytes = 0;
+  if (value) {
+    while (*value++) {

Review Comment:
   +I don't think it's a big deal to include string.h (it'll certainly raise less eyebrows than this)



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#issuecomment-1206757501

   # [Codecov](https://codecov.io/gh/apache/arrow-nanoarrow/pull/12?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 [#12](https://codecov.io/gh/apache/arrow-nanoarrow/pull/12?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (66073ee) into [main](https://codecov.io/gh/apache/arrow-nanoarrow/commit/51e5052ddd08fb424d8c20c86f9d5ea7d7b4ff51?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51e5052) will **decrease** coverage by `1.77%`.
   > The diff coverage is `75.94%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##             main      #12      +/-   ##
   ==========================================
   - Coverage   91.97%   90.20%   -1.78%     
   ==========================================
     Files           5        6       +1     
     Lines         798      919     +121     
     Branches       30       38       +8     
   ==========================================
   + Hits          734      829      +95     
   - Misses         41       59      +18     
   - Partials       23       31       +8     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-nanoarrow/pull/12?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/metadata.c](https://codecov.io/gh/apache/arrow-nanoarrow/pull/12/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-c3JjL25hbm9hcnJvdy9tZXRhZGF0YS5j) | `85.03% <75.94%> (-14.97%)` | :arrow_down: |
   | [src/nanoarrow/buffer\_inline.h](https://codecov.io/gh/apache/arrow-nanoarrow/pull/12/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-c3JjL25hbm9hcnJvdy9idWZmZXJfaW5saW5lLmg=) | `84.78% <0.00%> (ø)` | |
   
   :mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?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] lidavidm commented on a diff in pull request #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940553688


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   not a big deal for me either, though it is a little awkward to have the types differ between key/value even if there is a pragmatic reason



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940454918


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   I pushed a compromise of sorts...all values are now `ArrowStringView` but all keys stay `const char*`, which is somewhere inbetween the programmability and non-null terminated-ness of the `ArrowStringView` and the reality that keys are almost always `"a literal string"`. I tried having both be `ArrowStringView` but that was rather painful to actually use.



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r939160612


##########
src/nanoarrow/nanoarrow.h:
##########
@@ -261,6 +261,24 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
                                      const char* default_value,
                                      struct ArrowStringView* value_out);
 
+/// \brief Initialize a builder for schema metadata from key/value pairs
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer, const char* metadata);

Review Comment:
   The `metadata` param is an existing metadata buffer? (It's also not tested)



##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,
+                                              struct ArrowStringView* key,
+                                              struct ArrowStringView* value) {
+  if (value == NULL) {
+    return NANOARROW_OK;
+  }

Review Comment:
   Hmm, to me it's a little weird to accept NULL as the value and then just do nothing with it. If we just considered `append(key, NULL)` to be an error, we could drop this, and then we could pass the views by value instead of indirecting through a pointer



##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   Worth possibly exposing this variant too?



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot merged PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12


-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940564227


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

Review Comment:
   I'll change them to be the string views...anybody looking for convenience isn't writing C code anyway!



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940456577


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,
+                                              struct ArrowStringView* key,
+                                              struct ArrowStringView* value) {
+  if (value == NULL) {
+    return NANOARROW_OK;
+  }

Review Comment:
   I moved these to be internal implementation details...the `NULL` mostly just helps with minimizing the number of times one has to copy a metadata string (correspondingly, I added `ArrowMetadatBuilderRemove()` to make it more explicit).



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r941390810


##########
src/nanoarrow/metadata.c:
##########
@@ -114,8 +114,156 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
   return NANOARROW_OK;
 }
 
+ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
+                                     const char* default_value,
+                                     struct ArrowStringView* value_out) {
+  struct ArrowStringView key_view = {key, strlen(key)};
+  return ArrowMetadataGetValueView(metadata, &key_view, default_value, value_out);
+}
+
 char ArrowMetadataHasKey(const char* metadata, const char* key) {
   struct ArrowStringView value;
   ArrowMetadataGetValue(metadata, key, NULL, &value);
   return value.data != NULL;
 }
+
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer,
+                                        const char* metadata) {
+  ArrowBufferInit(buffer);
+  int result = ArrowBufferAppend(buffer, metadata, ArrowMetadataSizeOf(metadata));
+  if (result != NANOARROW_OK) {
+    return result;
+  }
+
+  return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowMetadataBuilderAppendView(struct ArrowBuffer* buffer,

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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r941444065


##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -166,6 +166,34 @@ enum ArrowType {
   NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO
 };
 
+/// \brief An non-owning view of a string
+struct ArrowStringView {
+  /// \brief A pointer to the start of the string
+  ///
+  /// If n_bytes is 0, this value may be NULL.
+  const char* data;
+
+  /// \brief The size of the string in bytes,
+  ///
+  /// (Not including the null terminator.)
+  int64_t n_bytes;
+};
+
+ /// \brief Create a string view from a null-terminated string
+static inline struct ArrowStringView ArrowCharView(const char* value) {
+  struct ArrowStringView out;
+
+  out.data = value;
+  out.n_bytes = 0;
+  if (value) {
+    while (*value++) {

Review Comment:
   why not strlen? to avoid the include?



-- 
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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r940384176


##########
src/nanoarrow/nanoarrow.h:
##########
@@ -261,6 +261,24 @@ ArrowErrorCode ArrowMetadataGetValue(const char* metadata, const char* key,
                                      const char* default_value,
                                      struct ArrowStringView* value_out);
 
+/// \brief Initialize a builder for schema metadata from key/value pairs
+ArrowErrorCode ArrowMetadataBuilderInit(struct ArrowBuffer* buffer, const char* metadata);

Review Comment:
   (Documented and tested 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: 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 #12: Add metadata builder functions

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #12:
URL: https://github.com/apache/arrow-nanoarrow/pull/12#discussion_r941544035


##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -166,6 +166,34 @@ enum ArrowType {
   NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO
 };
 
+/// \brief An non-owning view of a string
+struct ArrowStringView {
+  /// \brief A pointer to the start of the string
+  ///
+  /// If n_bytes is 0, this value may be NULL.
+  const char* data;
+
+  /// \brief The size of the string in bytes,
+  ///
+  /// (Not including the null terminator.)
+  int64_t n_bytes;
+};
+
+ /// \brief Create a string view from a null-terminated string
+static inline struct ArrowStringView ArrowCharView(const char* value) {
+  struct ArrowStringView out;
+
+  out.data = value;
+  out.n_bytes = 0;
+  if (value) {
+    while (*value++) {

Review Comment:
   Yes...too much of a hack? I'll check if string.h gets included in another inline file anyway (it probably does).



-- 
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