GH-35553: [JAVA] Fix unwrap() in NettyArrowBuf (#35554)

Martin Traverse · web-flow · commit a26b30ded946 · 2023-05-11T14:26:26.000-04:00
Suggested fix for #35553 No API changes, this just brings the implementation in line with the API as specified by Netty. CI for Java run successfully here (not sure if this is publically visible): https://github.com/martin-traverse/arrow/actions/runs/4949700024 * Closes: #35553 Authored-by: Martin Traverse <martin.traverse@accenture.com> Signed-off-by: David Li <li.davidm96@gmail.com>
diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java b/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java
@@ -110,7 +110,14 @@ public synchronized ByteBuf capacity(int newCapacity) {
 
   @Override
   public ByteBuf unwrap() {
-    throw new UnsupportedOperationException("Unwrap not supported.");
+
+    // According to Netty's ByteBuf interface, unwrap() should return null if the buffer cannot be unwrapped
+    // https://github.com/netty/netty/blob/9fe796e10a433b6cd20ad78b2c39cd56b86ccd2e/buffer/src/main/java/io/netty/buffer/ByteBuf.java#L305
+
+    // Throwing here breaks toString() in AbstractByteBuf
+    // Since toString() is used to build debug / error messages, this can cause strange behavior
+
+    return null;
   }
 
   @Override
diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java
@@ -138,4 +138,15 @@ public void testGetCompositeBuffer() {
       byteBufs.component(0).release();
     }
   }
+
+  @Test
+  public void testUnwrapReturnsNull() {
+    try (BufferAllocator allocator = new RootAllocator(128);
+         ArrowBuf buf = allocator.buffer(20);
+    ) {
+      NettyArrowBuf nettyBuf = NettyArrowBuf.unwrapBuffer(buf);
+      // NettyArrowBuf cannot be unwrapped, so unwrap() should return null per the Netty ByteBuf API
+      Assert.assertNull(nettyBuf.unwrap());
+    }
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -138,4 +138,15 @@ public void testGetCompositeBuffer() {`
`138`	`138`	`byteBufs.component(0).release();`
`139`	`139`	`}`
`140`	`140`	`}`
	`141`	`+`
	`142`	`+ @Test`
	`143`	`+ public void testUnwrapReturnsNull() {`
	`144`	`+ try (BufferAllocator allocator = new RootAllocator(128);`
	`145`	`+ ArrowBuf buf = allocator.buffer(20);`
	`146`	`+ ) {`
	`147`	`+ NettyArrowBuf nettyBuf = NettyArrowBuf.unwrapBuffer(buf);`
	`148`	`+ // NettyArrowBuf cannot be unwrapped, so unwrap() should return null per the Netty ByteBuf API`
	`149`	`+ Assert.assertNull(nettyBuf.unwrap());`
	`150`	`+ }`
	`151`	`+ }`
`141`	`152`	`}`