xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3700757908
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java:
##########
@@ -174,6 +174,8 @@ private static PinotDataType getPinotDataType(DataType
type, @Nullable Object va
return PinotDataType.BIG_DECIMAL;
case STRING:
return singleValue ? PinotDataType.STRING : PinotDataType.STRING_ARRAY;
+ case UUID:
Review Comment:
Fair point — reverted to rejecting the array form, so UUID now matches
`BYTES` (and `BOOLEAN` / `BIG_DECIMAL`) in this class.
To answer directly: `BYTES_ARRAY` is not supported here — `getPinotDataType`
rejects it up front via `value.getClass().getComponentType() == byte.class`. So
supporting `UUID_ARRAY` while `BYTES_ARRAY` is rejected was inconsistent, given
UUID is stored as BYTES. I had added it in response to your earlier comment on
this line, but matching `BYTES` is the more defensible reading.
Note the constructor is `@VisibleForTesting` and no UUID literal reaches it
in production — thrift has no UUID literal type, and `CAST(... AS UUID)` folds
to a string literal — so this only affects tests either way. Whether these
array forms *should* be supported looks like a pre-existing question across
`BOOLEAN` / `BIG_DECIMAL` / `BYTES`; happy to take that on separately.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -300,13 +310,91 @@ public String[] transformToStringValuesSV(ValueBlock
valueBlock) {
byte[][] bytesValues = transformToBytesValuesSV(valueBlock);
ArrayCopyUtils.copy(bytesValues, _stringValuesSV, length);
break;
+ case UUID:
+ byte[][] uuidValues = transformToBytesValuesSV(valueBlock);
+ for (int i = 0; i < length; i++) {
+ _stringValuesSV[i] = UuidUtils.toString(uuidValues[i]);
+ }
Review Comment:
Done — added `ArrayCopyUtils.copyFromUuid()` and `copyToUuid()`, SV and MV,
sitting next to `copyFromTimestamp` which they mirror (a logical type rendered
from / parsed into its stored form). `CastTransformFunction` now just calls
them, which removed four inline loops.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
