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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to