xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3680151149
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java:
##########
@@ -171,6 +171,9 @@ private static PinotDataType getPinotDataType(DataType
type, @Nullable Object va
return PinotDataType.BIG_DECIMAL;
case STRING:
return singleValue ? PinotDataType.STRING : PinotDataType.STRING_ARRAY;
+ case UUID:
+ Preconditions.checkState(singleValue, "UUID array is not supported");
Review Comment:
You are right, fixed — it now mirrors the STRING/INT/LONG shape rather than
rejecting the array form:
```java
case UUID:
return singleValue ? PinotDataType.UUID : PinotDataType.UUID_ARRAY;
```
Added `testUuidArrayLiteral` to cover it. I had copied the
`Preconditions.checkState(singleValue, ...)` pattern from the `BOOLEAN` and
`BIG_DECIMAL` cases above without checking whether it was warranted for UUID —
it was not, since `PinotDataType.UUID_ARRAY` exists.
On your follow-up: yes, the same restriction looks stale for the neighbours.
`BOOLEAN` and `BIG_DECIMAL` both `checkState(singleValue, ...)` even though
`PinotDataType.BOOLEAN_ARRAY` / `BIG_DECIMAL_ARRAY` exist, and `BYTES` is
rejected earlier via `value.getClass().getComponentType() == byte.class`, which
excludes `byte[][]`. I left those alone to keep this PR scoped to UUID — happy
to fix all three in a separate PR if you want, since they are pre-existing and
touch non-UUID behavior.
--
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]
