Spark: Add read/write support for UUIDs

[nastra]

fixes #4581

[RussellSpitzer]

This is the default right? Just setting it to be sure?

[nastra]

I was mainly aligning with other places in the code that also used a Thread local ByteBuffer when reading UUIDs

[rdblue]

Big endian is correct. See UUIDUtil for another implementation.

[RussellSpitzer]

Not that it's incorrect, it's just the default for all new ByteBuffers. Just wondering why we were setting it explicitly

[rdblue]

I think it's usually good to be explicit. Are we sure this the default, or is it the default on certain architectures?

[RussellSpitzer]

https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html#order()

The byte order is used when reading or writing multibyte values, and when creating buffers that are views of this byte buffer. The order of a newly-created byte buffer is always [BIG_ENDIAN](https://docs.oracle.com/javase/7/docs/api/java/nio/ByteOrder.html#BIG_ENDIAN).

[RussellSpitzer]

LGTM

[Fokko]

This is awesome!

[RussellSpitzer]

Lgtm! Thanks for doing this!

[rdblue]

This allocates a buffer. We may want to have a buffer here as a thread-local or a field to avoid allocation in a tight loop.

[nastra]

I agree with that observation and I initially used a Thread local to reduce byte[] allocation but couldn't get it to work because ((BytesColumnVector) output).setRef(..) would just store a reference to the passed byte[] and on subsequent writes we would end up overwriting previous values.
Worth mentioning that GenericOrcWriters does the same thing when writing UUIDs.

[rdblue]

Probably worth mentioning in a comment?

[nastra]

makes sense, I've added a comment to this as part of #7496

[rdblue]

In other places, like UUIDUtil, we use putLong(int offset, long value) instead of putLong(long value) so that the position is not updated and we don't need to worry about the buffer's internal state. I think that's usually a better approach.

Also, we might want to update UUIDUtil to share this code:

  public static ByteBuffer convertToByteBuffer(UUID value) {
    return convertToByteBuffer(value, null);
  }

  public static ByteBuffer convertToByteBuffer(UUID value, ByteBuffer reuse) {
    ByteBuffer buffer;
    if (reuse != null) {
      buffer = reuse;
    } else {
      buffer = ByteBuffer.allocate(16);
    }

    buffer.order(ByteOrder.BIG_ENDIAN);
    buffer.putLong(0, value.getMostSignificantBits());
    buffer.putLong(8, value.getLeastSignificantBits());
    return buffer;
  }

[nastra]

that makes sense, I've updated that.
We still have a few places in the code that do buffer.putLong(uuid.getMostSignificantBits());. I'll follow up on those and update them independently.

[nastra]

I've opened #7525 to address those other places in Spark

[rdblue]

Is there a way to get the underlying array and offset?

[nastra]

vector.get(rowId) will return the byte[] with a length of 16 for the given rowId. I think we could get underlying array and offset from the underlying ArrowBuf, but we would need to read it into a new byte[], which is what vector.get(rowId) is doing underneath

[rdblue]

Why does generatePrimitive provide byte[]? Shouldn't it create a String for Spark already?

[nastra]

my guess would be because RandomUtil.generatePrimitive(..) is used in other places where UUIDs are expected to be byte[]

[rdblue]

Thanks, @nastra!