[GitHub] [arrow] emkornfield commented on a change in pull request #7231: ARROW-6839: [Java] Add APIs to read and write "custom_metadata" field of IPC file footer

[GitBox <gi...@apache.org>]

emkornfield commented on a change in pull request #7231:
URL: https://github.com/apache/arrow/pull/7231#discussion_r433645139



##########
File path: java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowFooter.java
##########
@@ -96,17 +126,40 @@ public Schema getSchema() {
     return recordBatches;
   }
 
+  public Map<String, String> getMetaData() {
+    return metaData;
+  }
+
   @Override
   public int writeTo(FlatBufferBuilder builder) {
     int schemaIndex = schema.getSchema(builder);
     Footer.startDictionariesVector(builder, dictionaries.size());
     int dicsOffset = writeAllStructsToVector(builder, dictionaries);
     Footer.startRecordBatchesVector(builder, recordBatches.size());
     int rbsOffset = writeAllStructsToVector(builder, recordBatches);
+
+    int metaDataOffset = 0;
+    if (metaData != null) {
+      int[] metadataOffsets = new int[metaData.size()];

Review comment:
       i'm suprised this code doesn't exist else where for converted meta between string,string map

##########
File path: java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowFileReader.java
##########
@@ -112,6 +113,16 @@ public void initialize() throws IOException {
     }
   }
 
+  /**
+   * Get custom metadata.
+   */
+  public Map<String, String> getMetaData() {
+    if (footer != null) {
+      return footer.getMetaData();
+    }
+    return null;

Review comment:
       is this consistent with other methods, I think returning an empty map might be better?

##########
File path: java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowReaderWriter.java
##########
@@ -751,4 +754,43 @@ public void testChannelReadFullyEos() throws IOException {
       assertEquals(10, arrBuf.getInt(0));
     }
   }
+
+  @Test
+  public void testCustomMetaData() throws IOException {
+
+    VarCharVector vector = newVarCharVector("varchar1", allocator);
+    vector.allocateNewSafe();
+    ValueVectorDataPopulator.setVector(vector, "foo", "bar", "baz");

Review comment:
       are any values needed?  If so i imagine just one is needed?

##########
File path: java/vector/src/test/java/org/apache/arrow/vector/ipc/TestArrowReaderWriter.java
##########
@@ -751,4 +754,43 @@ public void testChannelReadFullyEos() throws IOException {
       assertEquals(10, arrBuf.getInt(0));
     }
   }
+
+  @Test
+  public void testCustomMetaData() throws IOException {
+
+    VarCharVector vector = newVarCharVector("varchar1", allocator);
+    vector.allocateNewSafe();
+    ValueVectorDataPopulator.setVector(vector, "foo", "bar", "baz");
+
+    List<Field> fields = Arrays.asList(vector.getField());
+    List<FieldVector> vectors = Collections2.asImmutableList(vector);
+    Map<String, String> metadata = new HashMap<>();
+    metadata.put("key1", "value1");
+    metadata.put("key2", "value2");
+    try (VectorSchemaRoot root = new VectorSchemaRoot(fields, vectors, vector.getValueCount());
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        ArrowFileWriter writer = new ArrowFileWriter(root, null, newChannel(out), metadata);) {
+
+      writer.start();
+      writer.writeBatch();

Review comment:
       does a batch even need to be written?




----------------------------------------------------------------
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:
users@infra.apache.org