rustyconover commented on code in PR #924:
URL: https://github.com/apache/arrow-nanoarrow/pull/924#discussion_r3830391848
##########
src/nanoarrow/nanoarrow_ipc.h:
##########
@@ -778,6 +842,23 @@ NANOARROW_DLL void ArrowIpcEncoderReset(struct
ArrowIpcEncoder* encoder);
NANOARROW_DLL ArrowErrorCode ArrowIpcEncoderFinalizeBuffer(
struct ArrowIpcEncoder* encoder, char encapsulate, struct ArrowBuffer*
out);
+/// \brief Set the custom metadata of the next encoded message
+///
+/// Attaches metadata to the next message encoded by
ArrowIpcEncoderEncodeSchema() or
+/// ArrowIpcEncoderEncodeSimpleRecordBatch() (i.e., Message::custom_metadata,
which is
+/// distinct from the metadata of the Schema or Field that the message may
contain).
+/// The metadata applies to exactly one message: after a message is encoded the
+/// encoder's message metadata is cleared. Any metadata that was set but not
yet
+/// encoded is replaced by this call; pass NULL to clear it.
+///
+/// metadata uses the same representation as ArrowSchema::metadata and may be
built
+/// with ArrowMetadataBuilderInit()/ArrowMetadataBuilderAppend(). It is copied
by this
+/// call and need not outlive it.
+///
+/// Returns ENOMEM if allocation fails, NANOARROW_OK otherwise.
+NANOARROW_DLL ArrowErrorCode ArrowIpcEncoderSetMessageMetadata(
+ struct ArrowIpcEncoder* encoder, const char* metadata, struct ArrowError*
error);
Review Comment:
Done — it now takes a `struct ArrowBuffer*` and moves it into the encoder
(`ArrowBufferMove()`), so the common case of "caller just built one with
`ArrowMetadataBuilder*`" no longer copies. Callers pass NULL to clear, and a
buffer too small to hold a key count is treated as empty.
##########
src/nanoarrow/ipc/decoder.c:
##########
@@ -1692,6 +1711,70 @@ ArrowErrorCode ArrowIpcDecoderDecodeHeader(struct
ArrowIpcDecoder* decoder,
}
private_data->last_message = message_header;
+ private_data->last_message_metadata = ns(Message_custom_metadata(message));
+ return NANOARROW_OK;
+}
+
+ArrowErrorCode ArrowIpcDecoderGetMessageMetadata(struct ArrowIpcDecoder*
decoder,
+ struct ArrowBuffer* out,
+ struct ArrowError* error) {
+ NANOARROW_DCHECK(decoder != NULL && decoder->private_data != NULL && out !=
NULL);
+ struct ArrowIpcDecoderPrivate* private_data =
+ (struct ArrowIpcDecoderPrivate*)decoder->private_data;
+
+ return ArrowIpcDecoderBuildMetadata(private_data->last_message_metadata,
out, error);
+}
+
+ArrowErrorCode ArrowIpcDecoderGetMessageMetadataValue(struct ArrowIpcDecoder*
decoder,
+ struct ArrowStringView
key,
+ struct ArrowStringView*
value_out,
+ struct ArrowError*
error) {
Review Comment:
Done — both accessors and the packing used for Schema/Field metadata now go
through one internal `ArrowIpcDecoderVisitMetadata()` over the KeyValue vector.
That turned out to matter for more than tidiness: the shared visitor uses
`flatbuffers_string_len()` where the old copying path used `strlen()`. Since
`KeyValue.key` and `KeyValue.value` are optional fields, a flatbuffer-verified
message can contain a KeyValue with no key, and `ArrowIpcDecoderDecodeSchema()`
segfaulted on `strlen(NULL)` — reproducible on `main`, and reachable from an
untrusted stream through `ArrowIpcArrayStreamReader`. An absent key or value
now decodes as an empty string, and values with embedded nulls are no longer
truncated. `NanoarrowIpcDecodeMetadataWithoutKey` covers it (it crashes without
the fix). Glad to move that part to a separate PR if you would rather keep this
one purely additive.
--
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]
