xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3669801561
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java:
##########
@@ -140,8 +141,11 @@ public LiteralContext(Literal literal) {
@VisibleForTesting
public LiteralContext(DataType type, @Nullable Object value) {
_type = type;
- _value = value;
- _pinotDataType = getPinotDataType(type, value);
+ // UUID values flow through the type system as java.util.UUID (canonical
form). Normalize string/byte inputs here
+ // so getStringValue() renders canonical lowercase and getBytesValue()
(PinotDataType.UUID.toBytes casts to UUID)
+ // does not throw.
+ _value = (type == DataType.UUID && value != null) ?
UuidUtils.toUUID(value) : value;
Review Comment:
Good catch — removed. The conversion is gone and the constructor is back to
`_value = value`.
Checking the callers confirmed your point: this constructor is
`@VisibleForTesting`, and the production `LiteralContext(Literal)` path has no
UUID case at all (thrift has no UUID literal — `CAST(... AS UUID)` folds to a
plain string literal). So the only caller passing `DataType.UUID` was my own
test, which was leaning on the constructor to normalize. It now passes a
`java.util.UUID`, i.e. the canonical in-memory form, and this class only
renders it.
The `case UUID` in `getPinotDataType` stays, since that is genuinely type
mapping rather than value conversion.
--
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]
