From 69c8a42cbcc0a3684647d2b3e873974944cb0c81 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Tue, 7 Jul 2026 14:27:42 -0700 Subject: [PATCH] [UUID 3/8] UUID result rendering (DataSchema, Arrow/JSON encoders) Part 3/8 of splitting apache/pinot#18140 (logical UUID type). Rebased onto master, which now includes the merged #18869 type foundation and #18870 ingest/storage layer. Renders UUID result columns as canonical lowercase RFC-4122 strings: - DataSchema.ColumnDataType.UUID / UUID_ARRAY plus the conversion and format helpers, backed by the UuidUtils class merged in #18869. - Arrow and JSON broker response encoders. UUID null placeholder: ColumnDataType.UUID overrides getNullPlaceholder() to return the nil UUID, matching the default null sentinel FieldSpec already uses for UUID columns. Its stored type BYTES supplies a zero-length placeholder, which is not a valid 16-byte UUID and fails to render, so the three call sites that build null-aware blocks (DataBlockBuilder, GroupByResultsBlock, GroupByDataTableReducer) now resolve the placeholder on the logical type rather than the stored type. For every other logical type the two are identical, an invariant pinned by a new DataSchemaTest case. DataSchema.fromBytes now reports an unrecognized ColumnDataType token with a message naming the mixed-version cause instead of a bare IllegalArgumentException. Rolling-upgrade note: DataSchema serializes ColumnDataType by name, so an old broker/server that receives a UUID token from a new node cannot parse it. Treat as an atomic-upgrade feature - UUID columns should only be queried once the whole cluster is upgraded. No existing (non-UUID) column is affected. --- .../encoder/ArrowResponseEncoder.java | 6 + .../response/encoder/JsonResponseEncoder.java | 2 + .../apache/pinot/common/utils/DataSchema.java | 172 +++++++++++++++++- .../encoder/ArrowResponseEncoderTest.java | 79 +++++++- .../encoder/JsonResponseEncoderTest.java | 52 +++++- .../pinot/common/utils/DataSchemaTest.java | 88 ++++++++- .../common/datablock/DataBlockBuilder.java | 40 ++-- .../blocks/results/GroupByResultsBlock.java | 4 +- .../query/reduce/GroupByDataTableReducer.java | 4 +- .../datablock/DataBlockBuilderTest.java | 23 +++ .../common/datablock/DataBlockTestUtils.java | 13 ++ .../common/datatable/DataTableSerDeTest.java | 26 +++ .../selection/SelectionOperatorUtilsTest.java | 23 +++ 13 files changed, 497 insertions(+), 35 deletions(-) diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java b/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java index f96027398c52..efd9098991e3 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java @@ -107,6 +107,7 @@ private VectorSchemaRoot createVectorSchemaRoot(ResultTable resultTable, DataSch case STRING: case JSON: case BYTES: + case UUID: case OBJECT: field = new Field(colName, FieldType.nullable(new ArrowType.Utf8()), null); vector = new VarCharVector(colName, ALLOCATOR); @@ -181,6 +182,7 @@ private VectorSchemaRoot createVectorSchemaRoot(ResultTable resultTable, DataSch case TIMESTAMP_ARRAY: case STRING_ARRAY: case BYTES_ARRAY: + case UUID_ARRAY: // Define the inner field for a string element. children = List.of(new Field("element", FieldType.nullable(new ArrowType.Utf8()), null)); // Define the field for the list column. @@ -234,6 +236,7 @@ private VectorSchemaRoot createVectorSchemaRoot(ResultTable resultTable, DataSch case STRING: case JSON: case BYTES: + case UUID: case OBJECT: byte[] bytes = ((String) value).getBytes(StandardCharsets.UTF_8); ((VarCharVector) vector).setSafe(rowIndex, bytes); @@ -345,6 +348,7 @@ private VectorSchemaRoot createVectorSchemaRoot(ResultTable resultTable, DataSch case TIMESTAMP_ARRAY: case STRING_ARRAY: case BYTES_ARRAY: + case UUID_ARRAY: ListVector listVector = (ListVector) vector; String[] stringArray = (String[]) value; // Start a new list entry for the current row. @@ -411,6 +415,7 @@ public ResultTable decodeResultTable(byte[] bytes, int rowSize, DataSchema schem case STRING: case JSON: case BYTES: + case UUID: case OBJECT: row[col] = new String(((VarCharVector) vector).get(i), StandardCharsets.UTF_8); break; @@ -469,6 +474,7 @@ public ResultTable decodeResultTable(byte[] bytes, int rowSize, DataSchema schem case TIMESTAMP_ARRAY: case STRING_ARRAY: case BYTES_ARRAY: + case UUID_ARRAY: ListVector listVector = (ListVector) vector; List arrayValues = listVector.getObject(i); String[] array = new String[arrayValues.size()]; diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/JsonResponseEncoder.java b/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/JsonResponseEncoder.java index 0f82daab9e95..aa86400de852 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/JsonResponseEncoder.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/encoder/JsonResponseEncoder.java @@ -194,6 +194,7 @@ private static Object extractArray(DataSchema.ColumnDataType columnDataType, Jso case TIMESTAMP_ARRAY: case STRING_ARRAY: case BYTES_ARRAY: + case UUID_ARRAY: String[] stringArray = new String[jsonValue.size()]; for (int k = 0; k < jsonValue.size(); k++) { stringArray[k] = jsonValue.get(k).textValue(); @@ -224,6 +225,7 @@ private static Object extractValue(DataSchema.ColumnDataType columnDataType, Jso case STRING: case JSON: case BYTES: + case UUID: case OBJECT: return jsonValue.textValue(); case UNKNOWN: diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java index 5149195d68c7..978b33aed80c 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java @@ -40,6 +40,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.sql.type.SqlTypeName; @@ -51,6 +52,7 @@ import org.apache.pinot.spi.utils.CommonConstants.NullValuePlaceHolder; import org.apache.pinot.spi.utils.EqualityUtils; import org.apache.pinot.spi.utils.PinotDataType; +import org.apache.pinot.spi.utils.UuidUtils; import static java.nio.charset.StandardCharsets.UTF_8; @@ -135,7 +137,7 @@ public byte[] toBytes() // Write the column types. for (ColumnDataType columnDataType : _columnDataTypes) { // We don't want to use ordinal of the enum since adding a new data type will break things if server and broker - // use different versions of DataType class. + // use different versions of DataType class. See parseColumnDataType() for the mixed-version read side. byte[] bytes = columnDataType.name().getBytes(UTF_8); dataOutputStream.writeInt(bytes.length); dataOutputStream.write(bytes); @@ -164,7 +166,7 @@ public static DataSchema fromBytes(ByteBuffer buffer) int length = buffer.getInt(); byte[] bytes = new byte[length]; buffer.get(bytes); - columnDataTypes[i] = ColumnDataType.valueOf(new String(bytes, UTF_8)); + columnDataTypes[i] = parseColumnDataType(new String(bytes, UTF_8)); } return new DataSchema(columnNames, columnDataTypes); } @@ -187,11 +189,31 @@ public static DataSchema fromBytes(PinotInputStream buffer) int length = buffer.readInt(); byte[] bytes = new byte[length]; buffer.readFully(bytes); - columnDataTypes[i] = ColumnDataType.valueOf(new String(bytes, UTF_8)); + columnDataTypes[i] = parseColumnDataType(new String(bytes, UTF_8)); } return new DataSchema(columnNames, columnDataTypes); } + /// Resolves a [ColumnDataType] token read off the wire, turning the raw [IllegalArgumentException] from + /// [ColumnDataType#valueOf] into a message that names the mixed-version cause. + /// + /// Rolling-upgrade limitation: once a node on this build emits a `UUID` token, an older peer that does not know + /// the [ColumnDataType#UUID] constant fails here. There is no version-negotiation shim or fallback to `BYTES` + /// today, so brokers and servers must be upgraded atomically (or UUID columns kept out of queries) until the + /// whole cluster is on this build. Rolling back to a pre-UUID build is likewise unsafe while UUID-typed query + /// results are in flight. No existing (non-UUID) column is affected. + private static ColumnDataType parseColumnDataType(String name) { + try { + return ColumnDataType.valueOf(name); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + "Unrecognized ColumnDataType '" + name + "' received from a peer node. This typically means the peer is " + + "running a newer build that introduced a data type not yet known to this node. Upgrade all brokers " + + "and servers to the same build before querying columns of that type, or keep those columns out of " + + "queries until the rolling upgrade is complete.", e); + } + } + @SuppressWarnings("MethodDoesntCallSuperMethod") @Override public DataSchema clone() { @@ -297,6 +319,25 @@ public RelDataType toType(RelDataTypeFactory typeFactory) { return typeFactory.createSqlType(SqlTypeName.VARBINARY); } }, + // UUID is a logical type backed by BYTES; keep it directly after BYTES. ColumnDataType is serialized by name (not + // ordinal) via name()/valueOf(), so enum order does not affect wire compatibility. + UUID(BYTES, null) { + @Override + public RelDataType toType(RelDataTypeFactory typeFactory) { + return typeFactory.createSqlType(SqlTypeName.UUID); + } + + /// Returns the nil UUID, matching the default null sentinel that [FieldSpec#getDefaultNullValue] uses for + /// UUID columns. This is the one type whose placeholder differs from its stored type's: `BYTES` uses a shared + /// zero-length [ByteArray], which is not a valid 16-byte UUID and would fail to render. + /// + /// A fresh instance is returned per call because, unlike every other placeholder (all empty or immutable), + /// this one wraps a mutable 16-byte array that callers hand out as a column fill value. + @Override + public Object getNullPlaceholder() { + return new ByteArray(UuidUtils.nullUuidBytes()); + } + }, MAP(NullValuePlaceHolder.MAP) { @Override public RelDataType toType(RelDataTypeFactory typeFactory) { @@ -363,6 +404,12 @@ public RelDataType toType(RelDataTypeFactory typeFactory) { return typeFactory.createArrayType(BYTES.toType(typeFactory), -1); } }, + UUID_ARRAY(BYTES_ARRAY, NullValuePlaceHolder.INTERNAL_BYTES_ARRAY) { + @Override + public RelDataType toType(RelDataTypeFactory typeFactory) { + return typeFactory.createArrayType(UUID.toType(typeFactory), -1); + } + }, UNKNOWN(null) { @Override public RelDataType toType(RelDataTypeFactory typeFactory) { @@ -374,7 +421,7 @@ public RelDataType toType(RelDataTypeFactory typeFactory) { private static final EnumSet INTEGRAL_TYPES = EnumSet.of(INT, LONG); private static final EnumSet ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, BIG_DECIMAL_ARRAY, BOOLEAN_ARRAY, TIMESTAMP_ARRAY, - STRING_ARRAY, BYTES_ARRAY); + STRING_ARRAY, BYTES_ARRAY, UUID_ARRAY); private static final EnumSet NUMERIC_ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, BIG_DECIMAL_ARRAY); private static final EnumSet INTEGRAL_ARRAY_TYPES = EnumSet.of(INT_ARRAY, LONG_ARRAY); @@ -395,6 +442,10 @@ public RelDataType toType(RelDataTypeFactory typeFactory) { _nullPlaceholder = nullPlaceHolder; } + /// Returns the value used to fill null entries in the serialized column, masked on read by the null bitmap. + /// + /// Callers must resolve this on the *logical* type, not on [#getStoredType], because [#UUID] overrides it (see + /// that constant). For every other logical type the two agree, an invariant pinned by `DataSchemaTest`. public Object getNullPlaceholder() { return _nullPlaceholder; } @@ -463,6 +514,9 @@ public DataType toDataType() { case BYTES: case BYTES_ARRAY: return DataType.BYTES; + case UUID: + case UUID_ARRAY: + return DataType.UUID; case UNKNOWN: return DataType.UNKNOWN; default: @@ -490,6 +544,8 @@ public DataType toDataType() { *
  • BYTES: byte[] -> ByteArray
    • *
  • BOOLEAN_ARRAY: boolean[] -> int[]
    • *
  • TIMESTAMP_ARRAY: Timestamp[] -> long[]
    • + *
  • UUID: UUID/String/byte[]/ByteArray -> ByteArray
    • + *
  • UUID_ARRAY: UUID[]/String[]/byte[][]/ByteArray[] -> ByteArray[]
    • * */ public Object toInternal(Object value) { @@ -500,10 +556,14 @@ public Object toInternal(Object value) { return ((Timestamp) value).getTime(); case BYTES: return new ByteArray((byte[]) value); + case UUID: + return new ByteArray(UuidUtils.toBytes(value)); case BOOLEAN_ARRAY: return fromBooleanArray((boolean[]) value); case TIMESTAMP_ARRAY: return fromTimestampArray((Timestamp[]) value); + case UUID_ARRAY: + return fromUuidArray(value); case OBJECT: // For OBJECT type, we need to convert based on the actual type of the value. This can happen when the scalar // function returns Object type, e.g. cast function. @@ -516,6 +576,9 @@ public Object toInternal(Object value) { if (value instanceof byte[]) { return new ByteArray((byte[]) value); } + if (value instanceof UUID) { + return new ByteArray(UuidUtils.toBytes((UUID) value)); + } if (value instanceof boolean[]) { return fromBooleanArray((boolean[]) value); } @@ -539,6 +602,8 @@ public Object toInternal(Object value) { *
  • BOOLEAN_ARRAY: int[] -> boolean[]
    • *
  • TIMESTAMP_ARRAY: long[] -> Timestamp[]
    • *
  • BYTES_ARRAY: ByteArray[] -> byte[][]
    • + *
  • UUID: ByteArray -> UUID
    • + *
  • UUID_ARRAY: ByteArray[] -> UUID[]
    • * */ public Object toExternal(Object value) { @@ -549,12 +614,16 @@ public Object toExternal(Object value) { return new Timestamp((long) value); case BYTES: return ((ByteArray) value).getBytes(); + case UUID: + return UuidUtils.toUUID((ByteArray) value); case BOOLEAN_ARRAY: return toBooleanArray((int[]) value); case TIMESTAMP_ARRAY: return toTimestampArray((long[]) value); case BYTES_ARRAY: return toBytesArray(value); + case UUID_ARRAY: + return toUuidArray(value); default: return value; } @@ -585,6 +654,8 @@ public Serializable convert(Object value) { return value.toString(); case BYTES: return ((ByteArray) value).getBytes(); + case UUID: + return UuidUtils.toUUID((ByteArray) value); case INT_ARRAY: return toIntArray(value); case LONG_ARRAY: @@ -603,6 +674,8 @@ public Serializable convert(Object value) { return toStringArray(value); case BYTES_ARRAY: return toBytesArray(value); + case UUID_ARRAY: + return toUuidArray(value); case UNKNOWN: // fall through case OBJECT: return (Serializable) value; @@ -624,12 +697,16 @@ public Serializable format(Object value) { return value.toString(); case BYTES: return BytesUtils.toHexString((byte[]) value); + case UUID: + return formatUuid(value); case BIG_DECIMAL_ARRAY: return formatBigDecimalArray((BigDecimal[]) value); case TIMESTAMP_ARRAY: return formatTimestampArray((Timestamp[]) value); case BYTES_ARRAY: return formatBytesArray((byte[][]) value); + case UUID_ARRAY: + return formatUuidArray(value); default: return (Serializable) value; } @@ -659,6 +736,8 @@ public Serializable convertAndFormat(Object value) { return value.toString(); case BYTES: return ((ByteArray) value).toHexString(); + case UUID: + return UuidUtils.toString((ByteArray) value); case MAP: return toMap(value); case INT_ARRAY: @@ -679,6 +758,8 @@ public Serializable convertAndFormat(Object value) { return (String[]) value; case BYTES_ARRAY: return formatBytesArray((ByteArray[]) value); + case UUID_ARRAY: + return formatUuidArray(value); default: throw new IllegalStateException(String.format("Cannot convert and format: '%s' to type: %s", value, this)); } @@ -869,6 +950,54 @@ private static byte[][] toBytesArray(Object value) { throw new IllegalStateException(String.format("Cannot convert: '%s' to byte[][]", value)); } + /// Converts any supported UUID array representation to `UUID[]`. Elements may be `UUID`, `byte[]`, [ByteArray] + /// or `CharSequence`; per-element dispatch is delegated to [UuidUtils#toUUID(Object)]. Note that `byte[][]`, + /// [ByteArray]`[]`, `String[]` and `UUID[]` are all `Object[]`, so one branch covers every array form. + private static UUID[] toUuidArray(Object value) { + if (value instanceof UUID[]) { + return (UUID[]) value; + } + if (value instanceof ObjectArrayList) { + ObjectArrayList list = (ObjectArrayList) value; + int size = list.size(); + UUID[] uuidArray = new UUID[size]; + for (int i = 0; i < size; i++) { + uuidArray[i] = UuidUtils.toUUID(list.get(i)); + } + return uuidArray; + } + Object[] valueArray = (Object[]) value; + int length = valueArray.length; + UUID[] uuidArray = new UUID[length]; + for (int i = 0; i < length; i++) { + uuidArray[i] = UuidUtils.toUUID(valueArray[i]); + } + return uuidArray; + } + + /// Inverse of [#toUuidArray]: converts any supported UUID array representation to the internal [ByteArray]`[]`. + private static ByteArray[] fromUuidArray(Object value) { + if (value instanceof ByteArray[]) { + return (ByteArray[]) value; + } + if (value instanceof ObjectArrayList) { + ObjectArrayList list = (ObjectArrayList) value; + int size = list.size(); + ByteArray[] wrapped = new ByteArray[size]; + for (int i = 0; i < size; i++) { + wrapped[i] = new ByteArray(UuidUtils.toBytes(list.get(i))); + } + return wrapped; + } + Object[] valueArray = (Object[]) value; + int length = valueArray.length; + ByteArray[] wrapped = new ByteArray[length]; + for (int i = 0; i < length; i++) { + wrapped[i] = new ByteArray(UuidUtils.toBytes(valueArray[i])); + } + return wrapped; + } + private static String[] formatBytesArray(byte[][] bytesArray) { int length = bytesArray.length; String[] formattedBytesArray = new String[length]; @@ -887,6 +1016,28 @@ private static String[] formatBytesArray(ByteArray[] byteArray) { return formattedBytesArray; } + /// Renders any supported UUID array representation as canonical lowercase RFC 4122 strings. Per-element + /// dispatch is delegated to [#formatUuid], so `byte[][]`, [ByteArray]`[]`, `UUID[]` and `String[]` are all + /// handled by the single `Object[]` branch. + private static String[] formatUuidArray(Object value) { + if (value instanceof ObjectArrayList) { + ObjectArrayList list = (ObjectArrayList) value; + int size = list.size(); + String[] formattedUuidArray = new String[size]; + for (int i = 0; i < size; i++) { + formattedUuidArray[i] = formatUuid(list.get(i)); + } + return formattedUuidArray; + } + Object[] valueArray = (Object[]) value; + int length = valueArray.length; + String[] formattedUuidArray = new String[length]; + for (int i = 0; i < length; i++) { + formattedUuidArray[i] = formatUuid(valueArray[i]); + } + return formattedUuidArray; + } + public static ColumnDataType fromDataType(DataType dataType, boolean isSingleValue) { return isSingleValue ? fromDataTypeSV(dataType) : fromDataTypeMV(dataType); } @@ -913,6 +1064,8 @@ public static ColumnDataType fromDataTypeSV(DataType dataType) { return JSON; case BYTES: return BYTES; + case UUID: + return UUID; case MAP: return MAP; case OPEN_STRUCT: @@ -944,6 +1097,8 @@ public static ColumnDataType fromDataTypeMV(DataType dataType) { return STRING_ARRAY; case BYTES: return BYTES_ARRAY; + case UUID: + return UUID_ARRAY; default: throw new IllegalStateException("Unsupported data type: " + dataType); } @@ -973,6 +1128,8 @@ public PinotDataType toPinotDataType() { return PinotDataType.JSON; case BYTES: return PinotDataType.BYTES; + case UUID: + return PinotDataType.UUID; case MAP: return PinotDataType.MAP; case OBJECT: @@ -1000,6 +1157,13 @@ public PinotDataType toPinotDataType() { } } + /// Renders a single UUID as its canonical lowercase RFC 4122 string. Accepts every representation + /// [UuidUtils#toUUID(Object)] does: `UUID`, `byte[]`, [ByteArray] and `CharSequence`. A non-canonical (e.g. + /// upper-case) string input is re-canonicalized rather than passed through. + private static String formatUuid(Object value) { + return UuidUtils.toUUID(value).toString(); + } + public abstract RelDataType toType(RelDataTypeFactory typeFactory); } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoderTest.java b/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoderTest.java index 0d65e3e9f0b9..ea141a293987 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoderTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoderTest.java @@ -23,15 +23,19 @@ import java.sql.Timestamp; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import org.apache.pinot.common.response.broker.ResultTable; import org.apache.pinot.common.utils.DataSchema; import org.apache.pinot.common.utils.DataSchema.ColumnDataType; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import static org.testng.Assert.fail; @@ -114,6 +118,72 @@ public void testEncodeDecodeMultipleRows() } } + @Test + public void testEncodeDecodeUuidColumn() + throws IOException { + DataSchema schema = new DataSchema(new String[]{"uuidCol"}, new ColumnDataType[]{ColumnDataType.UUID}); + List rows = Arrays.asList( + new Object[]{"550e8400-e29b-41d4-a716-446655440000"}, + new Object[]{"f81d4fae-7dec-11d0-a765-00a0c91e6bf6"} + ); + + ResultTable resultTable = new ResultTable(schema, rows); + ArrowResponseEncoder encoder = new ArrowResponseEncoder(); + byte[] encodedBytes = encoder.encodeResultTable(resultTable, 0, rows.size()); + ResultTable decodedTable = encoder.decodeResultTable(encodedBytes, rows.size(), schema); + + assertEquals(decodedTable.getRows().size(), rows.size(), "Row count should match"); + for (int i = 0; i < rows.size(); i++) { + assertEquals(decodedTable.getRows().get(i)[0], rows.get(i)[0], "UUID row " + i + " should match"); + } + } + + /// Mirrors the real broker path: a UUID column is rendered to its canonical string *before* it reaches the + /// encoder -- via `convertAndFormat` in the single-stage engine, and `format(toExternal(..))` in the multi-stage + /// engine (see QueryDispatcher#toExternalList). The encoder therefore only ever sees Strings in this group. + @Test + public void testEncodeDecodeUuidColumnRenderedFromInternalValue() + throws IOException { + String uuidValue = "550e8400-e29b-41d4-a716-446655440000"; + DataSchema schema = new DataSchema(new String[]{"uuidCol", "cnt"}, + new ColumnDataType[]{ColumnDataType.UUID, ColumnDataType.LONG}); + Object internalValue = ColumnDataType.UUID.toInternal(UUID.fromString(uuidValue)); + List rows = + Collections.singletonList(new Object[]{ColumnDataType.UUID.convertAndFormat(internalValue), 3L}); + + ResultTable resultTable = new ResultTable(schema, rows); + ArrowResponseEncoder encoder = new ArrowResponseEncoder(); + byte[] encodedBytes = encoder.encodeResultTable(resultTable, 0, rows.size()); + ResultTable decodedTable = encoder.decodeResultTable(encodedBytes, rows.size(), schema); + + assertEquals(decodedTable.getRows().size(), 1, "Row count should match"); + assertEquals(decodedTable.getRows().get(0)[0], uuidValue, "UUID value should round-trip as canonical string"); + assertEquals(decodedTable.getRows().get(0)[1], 3L, "Non-UUID columns should be preserved"); + } + + @Test + public void testEncodeDecodeUuidColumnWithNulls() + throws IOException { + String uuidValue = "550e8400-e29b-41d4-a716-446655440000"; + DataSchema schema = new DataSchema(new String[]{"uuidCol", "uuidArrayCol"}, + new ColumnDataType[]{ColumnDataType.UUID, ColumnDataType.UUID_ARRAY}); + List rows = Arrays.asList( + new Object[]{uuidValue, new String[]{uuidValue}}, + new Object[]{null, null} + ); + + ResultTable resultTable = new ResultTable(schema, rows); + ArrowResponseEncoder encoder = new ArrowResponseEncoder(); + byte[] encodedBytes = encoder.encodeResultTable(resultTable, 0, rows.size()); + ResultTable decodedTable = encoder.decodeResultTable(encodedBytes, rows.size(), schema); + + assertEquals(decodedTable.getRows().size(), 2, "Row count should match"); + assertEquals(decodedTable.getRows().get(0)[0], uuidValue, "Non-null UUID should round-trip"); + assertEquals(decodedTable.getRows().get(0)[1], new String[]{uuidValue}, "Non-null UUID array should round-trip"); + assertNull(decodedTable.getRows().get(1)[0], "Null UUID should round-trip as null"); + assertNull(decodedTable.getRows().get(1)[1], "Null UUID array should round-trip as null"); + } + @Test public void testEncodeDecodeAllDataTypes() throws IOException { @@ -122,7 +192,7 @@ public void testEncodeDecodeAllDataTypes() "intCol", "longCol", "floatCol", "doubleCol", "bigDecimalCol", "booleanCol", "timestampCol", "stringCol", "jsonCol", "mapCol", "bytesCol", "objectCol", "intArrayCol", "longArrayCol", "floatArrayCol", "doubleArrayCol", "booleanArrayCol", "timestampArrayCol", "stringArrayCol", - "bytesArrayCol", "unknownCol" + "bytesArrayCol", "uuidArrayCol", "unknownCol" }; DataSchema.ColumnDataType[] columnTypes = { @@ -146,6 +216,7 @@ public void testEncodeDecodeAllDataTypes() ColumnDataType.TIMESTAMP_ARRAY, ColumnDataType.STRING_ARRAY, ColumnDataType.BYTES_ARRAY, + ColumnDataType.UUID_ARRAY, ColumnDataType.UNKNOWN }; @@ -178,6 +249,10 @@ public void testEncodeDecodeAllDataTypes() byte[][] bytesArrayVal = new byte[][]{ new byte[]{1, 2}, new byte[]{3, 4} }; + byte[][] uuidArrayVal = new byte[][]{ + UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440000"), + UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440001") + }; Object unknownVal = null; // UNKNOWN is represented as null in this example. // Build a single row that contains all the above values. @@ -185,7 +260,7 @@ public void testEncodeDecodeAllDataTypes() Object[] row = new Object[]{ intVal, longVal, floatVal, doubleVal, bigDecimalVal, booleanVal, timestampVal, stringVal, jsonVal, mapVal, bytesVal, objectVal, intArrayVal, longArrayVal, floatArrayVal, doubleArrayVal, - booleanArrayVal, timestampArrayVal, stringArrayVal, bytesArrayVal, unknownVal + booleanArrayVal, timestampArrayVal, stringArrayVal, bytesArrayVal, uuidArrayVal, unknownVal }; for (int i = 0; i < row.length; i++) { row[i] = columnTypes[i].format(row[i]); // Convert to internal representation. diff --git a/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/JsonResponseEncoderTest.java b/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/JsonResponseEncoderTest.java index f23088fca01b..4625d618be10 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/JsonResponseEncoderTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/response/encoder/JsonResponseEncoderTest.java @@ -26,12 +26,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import org.apache.pinot.common.response.broker.ResultTable; import org.apache.pinot.common.utils.DataSchema; import org.apache.pinot.common.utils.DataSchema.ColumnDataType; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import static org.testng.Assert.fail; public class JsonResponseEncoderTest { @@ -110,6 +113,46 @@ public void testEncodeDecodeMultipleRows() throws IOException { } } + @Test + public void testEncodeDecodeUuidColumn() throws IOException { + DataSchema schema = new DataSchema( + new String[] {"uuidCol"}, + new ColumnDataType[] {ColumnDataType.UUID}); + String uuidValue = "550e8400-e29b-41d4-a716-446655440000"; + + List rows = new ArrayList<>(); + rows.add(new Object[] {ColumnDataType.UUID.format(UUID.fromString(uuidValue))}); + + ResultTable resultTable = new ResultTable(schema, rows); + JsonResponseEncoder encoder = new JsonResponseEncoder(); + + byte[] encodedBytes = encoder.encodeResultTable(resultTable, 0, rows.size()); + ResultTable decodedTable = encoder.decodeResultTable(encodedBytes, rows.size(), schema); + + assertEquals(decodedTable.getRows().size(), 1, "Row count should be 1"); + assertEquals(decodedTable.getRows().get(0)[0], uuidValue, "UUID value should round-trip as canonical string"); + } + + @Test + public void testEncodeDecodeUuidColumnWithNulls() throws IOException { + String uuidValue = "550e8400-e29b-41d4-a716-446655440000"; + DataSchema schema = new DataSchema(new String[] {"uuidCol"}, new ColumnDataType[] {ColumnDataType.UUID}); + + List rows = new ArrayList<>(); + rows.add(new Object[] {uuidValue}); + rows.add(new Object[] {null}); + + ResultTable resultTable = new ResultTable(schema, rows); + JsonResponseEncoder encoder = new JsonResponseEncoder(); + + byte[] encodedBytes = encoder.encodeResultTable(resultTable, 0, rows.size()); + ResultTable decodedTable = encoder.decodeResultTable(encodedBytes, rows.size(), schema); + + assertEquals(decodedTable.getRows().size(), 2, "Row count should be 2"); + assertEquals(decodedTable.getRows().get(0)[0], uuidValue, "Non-null UUID should round-trip"); + assertNull(decodedTable.getRows().get(1)[0], "Null UUID should round-trip as null"); + } + @Test public void testEncodeDecodeAllDataTypes() throws IOException { // Define the column names and corresponding data types. @@ -117,7 +160,7 @@ public void testEncodeDecodeAllDataTypes() throws IOException { "intCol", "longCol", "floatCol", "doubleCol", "bigDecimalCol", "booleanCol", "timestampCol", "stringCol", "jsonCol", "mapCol", "bytesCol", "objectCol", "intArrayCol", "longArrayCol", "floatArrayCol", "doubleArrayCol", "booleanArrayCol", "timestampArrayCol", "stringArrayCol", - "bytesArrayCol", "unknownCol" + "bytesArrayCol", "uuidArrayCol", "unknownCol" }; DataSchema.ColumnDataType[] columnTypes = { @@ -141,6 +184,7 @@ public void testEncodeDecodeAllDataTypes() throws IOException { ColumnDataType.TIMESTAMP_ARRAY, ColumnDataType.STRING_ARRAY, ColumnDataType.BYTES_ARRAY, + ColumnDataType.UUID_ARRAY, ColumnDataType.UNKNOWN }; @@ -173,6 +217,10 @@ public void testEncodeDecodeAllDataTypes() throws IOException { byte[][] bytesArrayVal = new byte[][] { new byte[] {1, 2}, new byte[] {3, 4} }; + byte[][] uuidArrayVal = new byte[][] { + UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440000"), + UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440001") + }; Object unknownVal = null; // UNKNOWN is represented as null in this example. // Build a single row that contains all the above values. @@ -180,7 +228,7 @@ public void testEncodeDecodeAllDataTypes() throws IOException { Object[] row = new Object[] { intVal, longVal, floatVal, doubleVal, bigDecimalVal, booleanVal, timestampVal, stringVal, jsonVal, mapVal, bytesVal, objectVal, intArrayVal, longArrayVal, floatArrayVal, doubleArrayVal, - booleanArrayVal, timestampArrayVal, stringArrayVal, bytesArrayVal, unknownVal + booleanArrayVal, timestampArrayVal, stringArrayVal, bytesArrayVal, uuidArrayVal, unknownVal }; // Convert each value using the schema's formatting (if needed). diff --git a/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java b/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java index 3a22b1d30cc1..ba3bdceef4e5 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java @@ -21,8 +21,11 @@ import java.math.BigDecimal; import java.nio.ByteBuffer; import java.sql.Timestamp; +import java.util.Locale; import org.apache.pinot.spi.data.FieldSpec; +import org.apache.pinot.spi.utils.ByteArray; import org.apache.pinot.spi.utils.BytesUtils; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.Assert; import org.testng.annotations.Test; @@ -31,14 +34,19 @@ public class DataSchemaTest { private static final String[] COLUMN_NAMES = { - "int", "long", "float", "double", "string", "object", "int_array", "long_array", "float_array", "double_array", - "string_array", "boolean_array", "timestamp_array", "bytes_array" + "int", "long", "float", "double", "string", "uuid", "object", "int_array", "long_array", "float_array", + "double_array", "string_array", "boolean_array", "timestamp_array", "bytes_array", "uuid_array" }; private static final int NUM_COLUMNS = COLUMN_NAMES.length; private static final DataSchema.ColumnDataType[] COLUMN_DATA_TYPES = { - INT, LONG, FLOAT, DOUBLE, STRING, OBJECT, INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY, - BOOLEAN_ARRAY, TIMESTAMP_ARRAY, BYTES_ARRAY + INT, LONG, FLOAT, DOUBLE, STRING, UUID, OBJECT, INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY, + BOOLEAN_ARRAY, TIMESTAMP_ARRAY, BYTES_ARRAY, UUID_ARRAY }; + private static final String UUID_VALUE = "550e8400-e29b-41d4-a716-446655440000"; + private static final String UUID_VALUE_2 = "550e8400-e29b-41d4-a716-446655440001"; + // Fully qualified: the static ColumnDataType.* import below binds the simple name UUID to the enum constant. + private static final java.util.UUID JAVA_UUID = java.util.UUID.fromString(UUID_VALUE); + private static final java.util.UUID JAVA_UUID_2 = java.util.UUID.fromString(UUID_VALUE_2); @Test public void testGetters() { @@ -71,9 +79,10 @@ public void testSerDe() public void testToString() { DataSchema dataSchema = new DataSchema(COLUMN_NAMES, COLUMN_DATA_TYPES); Assert.assertEquals(dataSchema.toString(), - "[int(INT),long(LONG),float(FLOAT),double(DOUBLE),string(STRING),object(OBJECT),int_array(INT_ARRAY)," - + "long_array(LONG_ARRAY),float_array(FLOAT_ARRAY),double_array(DOUBLE_ARRAY),string_array(STRING_ARRAY)," - + "boolean_array(BOOLEAN_ARRAY),timestamp_array(TIMESTAMP_ARRAY),bytes_array(BYTES_ARRAY)]"); + "[int(INT),long(LONG),float(FLOAT),double(DOUBLE),string(STRING),uuid(UUID),object(OBJECT)," + + "int_array(INT_ARRAY),long_array(LONG_ARRAY),float_array(FLOAT_ARRAY),double_array(DOUBLE_ARRAY)," + + "string_array(STRING_ARRAY),boolean_array(BOOLEAN_ARRAY),timestamp_array(TIMESTAMP_ARRAY)," + + "bytes_array(BYTES_ARRAY),uuid_array(UUID_ARRAY)]"); } @Test @@ -115,6 +124,16 @@ public void testColumnDataType() { Assert.assertFalse(STRING.isCompatible(STRING_ARRAY)); Assert.assertFalse(STRING.isCompatible(BYTES_ARRAY)); + Assert.assertFalse(UUID.isNumber()); + Assert.assertFalse(UUID.isWholeNumber()); + Assert.assertFalse(UUID.isArray()); + Assert.assertFalse(UUID.isNumberArray()); + Assert.assertFalse(UUID.isWholeNumberArray()); + Assert.assertFalse(UUID.isCompatible(DOUBLE)); + Assert.assertTrue(UUID.isCompatible(UUID)); + Assert.assertFalse(UUID.isCompatible(BYTES)); + Assert.assertFalse(UUID.isCompatible(STRING)); + Assert.assertFalse(OBJECT.isNumber()); Assert.assertFalse(OBJECT.isWholeNumber()); Assert.assertFalse(OBJECT.isArray()); @@ -154,7 +173,7 @@ public void testColumnDataType() { } for (DataSchema.ColumnDataType columnDataType : new DataSchema.ColumnDataType[]{ - STRING_ARRAY, BOOLEAN_ARRAY, TIMESTAMP_ARRAY, BYTES_ARRAY + STRING_ARRAY, BOOLEAN_ARRAY, TIMESTAMP_ARRAY, BYTES_ARRAY, UUID_ARRAY }) { Assert.assertFalse(columnDataType.isNumber()); Assert.assertFalse(columnDataType.isWholeNumber()); @@ -178,6 +197,8 @@ public void testColumnDataType() { Assert.assertEquals(fromDataType(FieldSpec.DataType.DOUBLE, false), DOUBLE_ARRAY); Assert.assertEquals(fromDataType(FieldSpec.DataType.STRING, true), STRING); Assert.assertEquals(fromDataType(FieldSpec.DataType.STRING, false), STRING_ARRAY); + Assert.assertEquals(fromDataType(FieldSpec.DataType.UUID, true), UUID); + Assert.assertEquals(fromDataType(FieldSpec.DataType.UUID, false), UUID_ARRAY); Assert.assertEquals(fromDataType(FieldSpec.DataType.BOOLEAN, false), BOOLEAN_ARRAY); Assert.assertEquals(fromDataType(FieldSpec.DataType.TIMESTAMP, false), TIMESTAMP_ARRAY); Assert.assertEquals(fromDataType(FieldSpec.DataType.BYTES, false), BYTES_ARRAY); @@ -186,7 +207,58 @@ public void testColumnDataType() { Assert.assertEquals(BIG_DECIMAL.format(bigDecimalValue), bigDecimalValue.toPlainString()); Timestamp timestampValue = new Timestamp(1234567890123L); Assert.assertEquals(TIMESTAMP.format(timestampValue), timestampValue.toString()); + ByteArray uuidValue = new ByteArray(UuidUtils.toBytes(UUID_VALUE)); + Assert.assertEquals(UUID.convert(uuidValue), JAVA_UUID); + Assert.assertEquals(UUID.format(uuidValue), UUID_VALUE); + Assert.assertEquals(UUID.convertAndFormat(uuidValue), UUID_VALUE); + // format() also accepts the external form and re-canonicalizes non-canonical strings. + Assert.assertEquals(UUID.format(JAVA_UUID), UUID_VALUE); + Assert.assertEquals(UUID.format(UUID_VALUE.toUpperCase(Locale.ROOT)), UUID_VALUE); + byte[][] uuidArrayBytesValue = {UuidUtils.toBytes(UUID_VALUE), UuidUtils.toBytes(UUID_VALUE_2)}; + ByteArray[] uuidArrayValue = (ByteArray[]) UUID_ARRAY.toInternal(new String[]{UUID_VALUE, UUID_VALUE_2}); + java.util.UUID[] expectedUuidArray = {JAVA_UUID, JAVA_UUID_2}; + String[] expectedFormatted = {UUID_VALUE, UUID_VALUE_2}; + Assert.assertEquals(UUID_ARRAY.toExternal(uuidArrayValue), expectedUuidArray); + Assert.assertEquals(UUID_ARRAY.toExternal(uuidArrayBytesValue), expectedUuidArray); + Assert.assertEquals(UUID_ARRAY.convert(uuidArrayValue), expectedUuidArray); + Assert.assertEquals(UUID_ARRAY.toInternal(expectedUuidArray), uuidArrayValue); + Assert.assertEquals(UUID_ARRAY.toInternal(uuidArrayBytesValue), uuidArrayValue); + Assert.assertEquals(UUID_ARRAY.format(uuidArrayBytesValue), expectedFormatted); + Assert.assertEquals(UUID_ARRAY.format(expectedUuidArray), expectedFormatted); + Assert.assertEquals(UUID_ARRAY.format(uuidArrayValue), expectedFormatted); + Assert.assertEquals(UUID_ARRAY.convertAndFormat(uuidArrayValue), expectedFormatted); + Assert.assertEquals(UUID_ARRAY.convertAndFormat(uuidArrayBytesValue), expectedFormatted); byte[] bytesValue = {12, 34, 56}; Assert.assertEquals(BYTES.format(bytesValue), BytesUtils.toHexString(bytesValue)); } + + /// The null placeholder must be resolved on the *logical* type. UUID is the only type whose placeholder differs + /// from its stored type's: it needs the 16-byte nil UUID, while BYTES supplies a zero-length one. Every other + /// logical type must agree with its stored type, otherwise callers that resolve the stored type (DataBlockBuilder, + /// GroupByResultsBlock, GroupByDataTableReducer) would silently write the wrong placeholder. + @Test + public void testNullPlaceholderMatchesStoredTypeExceptUuid() { + for (DataSchema.ColumnDataType columnDataType : DataSchema.ColumnDataType.values()) { + DataSchema.ColumnDataType storedType = columnDataType.getStoredType(); + if (columnDataType == UUID) { + Assert.assertEquals(storedType, BYTES); + Assert.assertEquals(columnDataType.getNullPlaceholder(), new ByteArray(UuidUtils.nullUuidBytes())); + Assert.assertNotEquals(columnDataType.getNullPlaceholder(), storedType.getNullPlaceholder()); + } else { + Assert.assertEquals(columnDataType.getNullPlaceholder(), storedType.getNullPlaceholder(), + "Null placeholder mismatch between " + columnDataType + " and its stored type " + storedType); + } + } + } + + /// The nil-UUID placeholder wraps a mutable 16-byte array, unlike every other placeholder (all empty or + /// immutable), so each call must hand back a fresh instance. + @Test + public void testUuidNullPlaceholderIsNotShared() { + ByteArray first = (ByteArray) UUID.getNullPlaceholder(); + ByteArray second = (ByteArray) UUID.getNullPlaceholder(); + Assert.assertNotSame(first, second); + first.getBytes()[0] = 1; + Assert.assertEquals(second, new ByteArray(UuidUtils.nullUuidBytes())); + } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java b/pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java index 3a15b30184ed..6e418e572c4f 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java @@ -81,7 +81,9 @@ public static RowDataBlock buildFromRows(List rows, DataSchema dataSch Object[] nullPlaceholders = new Object[numColumns]; for (int colId = 0; colId < numColumns; colId++) { nullBitmaps[colId] = new RoaringBitmap(); - nullPlaceholders[colId] = storedTypes[colId].getNullPlaceholder(); + // Resolved on the logical type, not the stored type: UUID overrides getNullPlaceholder() to return the nil + // UUID, whereas its stored type BYTES would yield a zero-length placeholder that is not a valid UUID. + nullPlaceholders[colId] = dataSchema.getColumnDataType(colId).getNullPlaceholder(); } int nullFixedBytes = numColumns * Integer.BYTES * 2; int rowSizeInBytes = calculateBytesPerRow(dataSchema); @@ -254,7 +256,11 @@ private static void serializeColumnData(List columns, DataSchema dataS ByteBuffer fixedSize, PagedPinotOutputStream varSize, RoaringBitmap nullBitmap, Object2IntOpenHashMap dictionary, @Nullable AggregationFunction aggFunction) throws IOException { - ColumnDataType storedType = dataSchema.getColumnDataType(colId).getStoredType(); + // Dispatch on the stored type, but read null placeholders off the logical type: UUID overrides + // getNullPlaceholder() to return the nil UUID, whereas its stored type BYTES would yield a zero-length + // placeholder that is not a valid UUID. The two agree for every other type (see DataSchemaTest). + ColumnDataType columnDataType = dataSchema.getColumnDataType(colId); + ColumnDataType storedType = columnDataType.getStoredType(); int numRows = columns.get(colId).length; Object[] column = columns.get(colId); @@ -266,7 +272,7 @@ private static void serializeColumnData(List columns, DataSchema dataS switch (storedType) { // Single-value column case INT: { - int nullPlaceholder = (int) storedType.getNullPlaceholder(); + int nullPlaceholder = (int) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -281,7 +287,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case LONG: { - long nullPlaceholder = (long) storedType.getNullPlaceholder(); + long nullPlaceholder = (long) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -296,7 +302,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case FLOAT: { - float nullPlaceholder = (float) storedType.getNullPlaceholder(); + float nullPlaceholder = (float) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -311,7 +317,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case DOUBLE: { - double nullPlaceholder = (double) storedType.getNullPlaceholder(); + double nullPlaceholder = (double) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -326,7 +332,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case BIG_DECIMAL: { - BigDecimal nullPlaceholder = (BigDecimal) storedType.getNullPlaceholder(); + BigDecimal nullPlaceholder = (BigDecimal) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -342,7 +348,7 @@ private static void serializeColumnData(List columns, DataSchema dataS } case STRING: { ToIntFunction didSupplier = k -> dictionary.size(); - int nullPlaceHolder = dictionary.computeIfAbsent((String) storedType.getNullPlaceholder(), didSupplier); + int nullPlaceHolder = dictionary.computeIfAbsent((String) columnDataType.getNullPlaceholder(), didSupplier); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -358,7 +364,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case BYTES: { - ByteArray nullPlaceholder = (ByteArray) storedType.getNullPlaceholder(); + ByteArray nullPlaceholder = (ByteArray) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -373,7 +379,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case MAP: { - Map nullPlaceholder = (Map) storedType.getNullPlaceholder(); + Map nullPlaceholder = (Map) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -389,7 +395,7 @@ private static void serializeColumnData(List columns, DataSchema dataS } // Multi-value column case INT_ARRAY: { - int[] nullPlaceholder = (int[]) storedType.getNullPlaceholder(); + int[] nullPlaceholder = (int[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -404,7 +410,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case LONG_ARRAY: { - long[] nullPlaceholder = (long[]) storedType.getNullPlaceholder(); + long[] nullPlaceholder = (long[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -419,7 +425,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case FLOAT_ARRAY: { - float[] nullPlaceholder = (float[]) storedType.getNullPlaceholder(); + float[] nullPlaceholder = (float[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -434,7 +440,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case DOUBLE_ARRAY: { - double[] nullPlaceholder = (double[]) storedType.getNullPlaceholder(); + double[] nullPlaceholder = (double[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -449,7 +455,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case BIG_DECIMAL_ARRAY: { - BigDecimal[] nullPlaceholder = (BigDecimal[]) storedType.getNullPlaceholder(); + BigDecimal[] nullPlaceholder = (BigDecimal[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -464,7 +470,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case STRING_ARRAY: { - String[] nullPlaceholder = (String[]) storedType.getNullPlaceholder(); + String[] nullPlaceholder = (String[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; @@ -479,7 +485,7 @@ private static void serializeColumnData(List columns, DataSchema dataS break; } case BYTES_ARRAY: { - ByteArray[] nullPlaceholder = (ByteArray[]) storedType.getNullPlaceholder(); + ByteArray[] nullPlaceholder = (ByteArray[]) columnDataType.getNullPlaceholder(); interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> { for (int rowId = start; rowId < end; rowId++) { Object value = column[rowId]; diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/results/GroupByResultsBlock.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/results/GroupByResultsBlock.java index 6cfc85118187..0a5565be7b99 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/results/GroupByResultsBlock.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/results/GroupByResultsBlock.java @@ -220,7 +220,9 @@ public DataTable getDataTable() Object[] nullPlaceholders = new Object[numColumns]; for (int colId = 0; colId < numColumns; colId++) { nullBitmaps[colId] = new RoaringBitmap(); - nullPlaceholders[colId] = storedColumnDataTypes[colId].getNullPlaceholder(); + // Resolved on the logical type, not the stored type: UUID overrides getNullPlaceholder() to return the nil + // UUID, whereas its stored type BYTES would yield a zero-length placeholder that is not a valid UUID. + nullPlaceholders[colId] = _dataSchema.getColumnDataType(colId).getNullPlaceholder(); } int rowId = 0; while (iterator.hasNext()) { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java index bfbd8bb8486d..85b8f614f3d9 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java @@ -564,7 +564,9 @@ private DataTable buildIntermediateDataTable(DataSchema dataSchema, IndexedTable Object[] nullPlaceholders = new Object[_numColumns]; for (int colId = 0; colId < _numColumns; colId++) { nullBitmaps[colId] = new RoaringBitmap(); - nullPlaceholders[colId] = storedColumnDataTypes[colId].getNullPlaceholder(); + // Resolved on the logical type, not the stored type: UUID overrides getNullPlaceholder() to return the nil + // UUID, whereas its stored type BYTES would yield a zero-length placeholder that is not a valid UUID. + nullPlaceholders[colId] = dataSchema.getColumnDataType(colId).getNullPlaceholder(); } int rowId = 0; while (iterator.hasNext()) { diff --git a/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockBuilderTest.java b/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockBuilderTest.java index f7206f591899..24e4c03b11cf 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockBuilderTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockBuilderTest.java @@ -32,6 +32,7 @@ import org.apache.pinot.common.utils.DataSchema.ColumnDataType; import org.apache.pinot.core.query.aggregation.function.AggregationFunction; import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.RoaringBitmap; import org.testng.Assert; import org.testng.annotations.DataProvider; @@ -194,6 +195,28 @@ void testColumnBlockMultiBatch(ColumnDataType type) runColumnBlockTest(type, 25_000); } + /// A null in a UUID column must serialize as the nil UUID, not as the zero-length placeholder its stored type + /// (BYTES) supplies. The value is normally masked by the null bitmap, but it must still decode as a valid 16-byte + /// UUID for any consumer that renders the raw column, and [UuidUtils#toString] rejects any other width. + @Test + void testUuidNullPlaceholderIsNilUuid() + throws IOException { + ByteArray uuid = new ByteArray(UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440000")); + DataSchema dataSchema = new DataSchema(new String[]{"uuidCol"}, new ColumnDataType[]{ColumnDataType.UUID}); + Object[] column = {uuid, null}; + List rows = List.of(new Object[]{uuid}, new Object[]{null}); + + List blocks = List.of(DataBlockBuilder.buildFromRows(rows, dataSchema), + DataBlockBuilder.buildFromColumns(List.of(column), dataSchema)); + for (DataBlock block : blocks) { + assertEquals(block.getNumberOfRows(), 2); + assertEquals(new ByteArray(block.getBytes(0, 0).getBytes()), uuid); + // Row 1 is null: the bitmap flags it, and the placeholder underneath still renders as the nil UUID. + assertEquals(block.getNullRowIds(0), RoaringBitmap.bitmapOf(1)); + assertEquals(UuidUtils.toString(block.getBytes(1, 0).getBytes()), "00000000-0000-0000-0000-000000000000"); + } + } + private void runColumnBlockTest(ColumnDataType type, int numRows) throws IOException { Object[] column = generateColumns(type, numRows); diff --git a/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockTestUtils.java b/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockTestUtils.java index c96a21b5e86c..a9fd19f09004 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockTestUtils.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/common/datablock/DataBlockTestUtils.java @@ -24,11 +24,13 @@ import java.util.List; import java.util.Map; import java.util.Random; +import java.util.UUID; import org.apache.commons.lang3.RandomStringUtils; import org.apache.pinot.common.datablock.DataBlock; import org.apache.pinot.common.utils.DataSchema; import org.apache.pinot.common.utils.DataSchema.ColumnDataType; import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.RoaringBitmap; @@ -73,6 +75,9 @@ public static Object[] getRandomRow(DataSchema dataSchema, int nullPercentile) { case BYTES: row[colId] = new ByteArray(RandomStringUtils.secure().next(RANDOM.nextInt(20)).getBytes()); break; + case UUID: + row[colId] = new ByteArray(UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong()))); + break; case INT_ARRAY: int length = RANDOM.nextInt(ARRAY_SIZE); int[] intArray = new int[length]; @@ -147,6 +152,14 @@ public static Object[] getRandomRow(DataSchema dataSchema, int nullPercentile) { } row[colId] = bytesArray; break; + case UUID_ARRAY: + length = RANDOM.nextInt(ARRAY_SIZE); + ByteArray[] uuidArray = new ByteArray[length]; + for (int i = 0; i < length; i++) { + uuidArray[i] = new ByteArray(UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong()))); + } + row[colId] = uuidArray; + break; case MAP: length = RANDOM.nextInt(ARRAY_SIZE); Map map = new HashMap<>(); diff --git a/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java b/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java index 7f416c9f4148..f2ac88110352 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Random; +import java.util.UUID; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.apache.pinot.common.datatable.DataTable; @@ -34,6 +35,7 @@ import org.apache.pinot.spi.accounting.ThreadResourceUsageProvider; import org.apache.pinot.spi.exception.QueryErrorCode; import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.RoaringBitmap; import org.testng.Assert; import org.testng.annotations.DataProvider; @@ -60,6 +62,7 @@ public class DataTableSerDeTest { private static final String[] STRINGS = new String[NUM_ROWS]; private static final String[] JSONS = new String[NUM_ROWS]; private static final byte[][] BYTES = new byte[NUM_ROWS][]; + private static final byte[][] UUIDS = new byte[NUM_ROWS][]; private static final Object[] OBJECTS = new Object[NUM_ROWS]; private static final int[][] INT_ARRAYS = new int[NUM_ROWS][]; private static final long[][] LONG_ARRAYS = new long[NUM_ROWS][]; @@ -69,6 +72,7 @@ public class DataTableSerDeTest { private static final long[][] TIMESTAMP_ARRAYS = new long[NUM_ROWS][]; private static final String[][] STRING_ARRAYS = new String[NUM_ROWS][]; private static final ByteArray[][] BYTES_ARRAYS = new ByteArray[NUM_ROWS][]; + private static final ByteArray[][] UUID_ARRAYS = new ByteArray[NUM_ROWS][]; private static final BigDecimal[][] BIG_DECIMAL_ARRAYS = new BigDecimal[NUM_ROWS][]; private static final Map[] MAPS = new Map[NUM_ROWS]; @@ -367,6 +371,11 @@ private void fillDataTableWithRandomData(DataTableBuilder dataTableBuilder, BYTES[rowId] = isNull ? new byte[0] : RandomStringUtils.secure().next(RANDOM.nextInt(20)).getBytes(); dataTableBuilder.setColumn(colId, new ByteArray(BYTES[rowId])); break; + case UUID: + UUIDS[rowId] = isNull ? UuidUtils.nullUuidBytes() + : UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong())); + dataTableBuilder.setColumn(colId, new ByteArray(UUIDS[rowId])); + break; case INT_ARRAY: int length = RANDOM.nextInt(20); int[] intArray = new int[length]; @@ -440,6 +449,15 @@ private void fillDataTableWithRandomData(DataTableBuilder dataTableBuilder, BYTES_ARRAYS[rowId] = bytesArray; dataTableBuilder.setColumn(colId, bytesArray); break; + case UUID_ARRAY: + length = RANDOM.nextInt(20); + ByteArray[] uuidArray = new ByteArray[length]; + for (int i = 0; i < length; i++) { + uuidArray[i] = new ByteArray(UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong()))); + } + UUID_ARRAYS[rowId] = uuidArray; + dataTableBuilder.setColumn(colId, uuidArray); + break; case STRING_ARRAY: length = RANDOM.nextInt(20); String[] stringArray = new String[length]; @@ -520,6 +538,10 @@ private void verifyDataIsSame(DataTable newDataTable, DataSchema.ColumnDataType[ Assert.assertEquals(newDataTable.getBytes(rowId, colId).getBytes(), isNull ? new byte[0] : BYTES[rowId], ERROR_MESSAGE); break; + case UUID: + Assert.assertEquals(newDataTable.getBytes(rowId, colId).getBytes(), + isNull ? UuidUtils.nullUuidBytes() : UUIDS[rowId], ERROR_MESSAGE); + break; case INT_ARRAY: Assert.assertTrue(Arrays.equals(newDataTable.getIntArray(rowId, colId), INT_ARRAYS[rowId]), ERROR_MESSAGE); break; @@ -551,6 +573,10 @@ private void verifyDataIsSame(DataTable newDataTable, DataSchema.ColumnDataType[ Assert.assertTrue(Arrays.equals(newDataTable.getBytesArray(rowId, colId), BYTES_ARRAYS[rowId]), ERROR_MESSAGE); break; + case UUID_ARRAY: + Assert.assertTrue(Arrays.equals(newDataTable.getBytesArray(rowId, colId), UUID_ARRAYS[rowId]), + ERROR_MESSAGE); + break; case STRING_ARRAY: Assert.assertTrue(Arrays.equals(newDataTable.getStringArray(rowId, colId), STRING_ARRAYS[rowId]), ERROR_MESSAGE); diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorUtilsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorUtilsTest.java index 196f4dd168e9..1df11e8cf866 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorUtilsTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorUtilsTest.java @@ -18,17 +18,23 @@ */ package org.apache.pinot.core.query.selection; +import java.util.Collections; import org.apache.commons.lang3.tuple.Pair; +import org.apache.pinot.common.response.broker.ResultTable; import org.apache.pinot.common.utils.DataSchema; import org.apache.pinot.common.utils.DataSchema.ColumnDataType; import org.apache.pinot.core.query.request.context.QueryContext; import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils; +import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.BytesUtils; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; public class SelectionOperatorUtilsTest { + private static final String UUID_VALUE = "550e8400-e29b-41d4-a716-446655440000"; @Test public void testGetResultTableColumnIndices() { @@ -207,4 +213,21 @@ public void testGetResultTableColumnIndices() { ColumnDataType.STRING, ColumnDataType.STRING, ColumnDataType.STRING })); } + + @Test + public void testRenderResultTableWithoutOrderingFormatsUUIDAndBytes() { + byte[] bytesValue = new byte[]{0x01, 0x23, 0x45}; + DataSchema dataSchema = new DataSchema(new String[]{"uuidCol", "bytesCol"}, + new ColumnDataType[]{ColumnDataType.UUID, ColumnDataType.BYTES}); + + ResultTable resultTable = SelectionOperatorUtils.renderResultTableWithoutOrdering( + Collections.singletonList( + new Object[]{new ByteArray(UuidUtils.toBytes(UUID_VALUE)), new ByteArray(bytesValue)}), + dataSchema, + new int[]{0, 1}); + + Object[] row = resultTable.getRows().get(0); + assertEquals(row[0], UUID_VALUE); + assertEquals(row[1], BytesUtils.toHexString(bytesValue)); + } }