xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3706705253
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java:
##########
@@ -872,6 +886,22 @@ protected byte[][]
transformToBytesValuesSVUsingValueAndNull(ValueBlock valueBlo
return _bytesValuesSV;
}
+ private byte[][] getBytesValues(TransformFunction transformFunction,
ValueBlock valueBlock) {
+ if (_resultMetadata.getDataType() != DataType.UUID || !(transformFunction
instanceof LiteralTransformFunction)) {
Review Comment:
Implemented, following the TIMESTAMP precedent rather than adding a literal
type.
`RequestUtils#getLiteral(Object)` now folds a `java.util.UUID` to its
16-byte stored form, right beside the existing TIMESTAMP branch that folds to
millis:
```java
if (object instanceof Timestamp) {
return getLiteral(((Timestamp) object).getTime());
}
if (object instanceof UUID) {
return getLiteral(UuidUtils.toBytes((UUID) object));
}
```
I deliberately did **not** add a `uuidValue` field to the thrift `Literal`
union. `LiteralContext(Literal)` ends in `default: throw new
IllegalStateException("Unsupported field type: ...")`, and brokers upgrade
before servers, so a new broker emitting an unknown literal field would fail
every UUID query with a CAST literal for the whole rolling-upgrade window.
`binaryValue` already exists and old servers understand it, so this needs no
wire-format change and no version gate.
Two things fell out of it:
- The predicate now carries the stored form (hex), exactly as a BYTES
literal does. Nothing else needed changing, because `UuidUtils#toBytes(String)`
already accepts both that and the canonical dashed form — so a bare
`'550e8400-...'` string literal still works. Updated the
`RequestContextUtilsTest` assertions to the stored form.
- The `CASE` handling shrank but did not disappear. A `CAST(... AS UUID)`
branch now arrives as a BYTES literal and takes the normal path, so it no
longer needs help. A **bare STRING literal** branch still does — `CASE WHEN c <
2 THEN '550e8400-...' ELSE CAST(...) END` types that literal as STRING, so
`getBytesLiteral()` would hex-decode it.
`CaseTransformFunctionTest.testCaseTransformFunctionWithUuidStringLiteralBranch`
catches exactly that; I tried deleting the helper first and that test failed.
The comment on it now says precisely which case remains and why.
Full `pinot-common` suite (2131) and the affected `pinot-core` tests are
green.
--
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]
