xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3681241270
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/InTransformFunction.java:
##########
@@ -203,11 +212,20 @@ public int[] transformToIntValuesSV(ValueBlock
valueBlock) {
}
break;
case BYTES:
- ObjectOpenHashSet<ByteArray> inBytesValues =
(ObjectOpenHashSet<ByteArray>) _valueSet;
byte[][] bytesValues =
_mainFunction.transformToBytesValuesSV(valueBlock);
- for (int i = 0; i < length; i++) {
- if (inBytesValues.contains(new ByteArray(bytesValues[i]))) {
- _intValuesSV[i] = 1;
+ if (_mainFunction.getResultMetadata().getDataType() ==
DataType.UUID) {
+ ObjectOpenHashSet<UuidKey> inUuidValues =
(ObjectOpenHashSet<UuidKey>) _valueSet;
+ for (int i = 0; i < length; i++) {
+ if (inUuidValues.contains(UuidKey.fromBytes(bytesValues[i]))) {
+ _intValuesSV[i] = 1;
Review Comment:
Good catch, fixed — and it let the UUID/BYTES split collapse rather than
just swapping the set type.
`InTransformFunction` now keys on the raw `byte[]` with
`ObjectOpenCustomHashSet` + `ByteArrays.HASH_STRATEGY`, matching what the IN /
NOT IN predicate evaluators in this PR already do. Since UUID and BYTES differ
only in how the *literal* is parsed (canonical UUID text vs hex) and not in the
stored form, the two branches merge into one:
```java
boolean isUuid = _mainFunction.getResultMetadata().getDataType() ==
DataType.UUID;
Set<byte[]> bytesValues = new ObjectOpenCustomHashSet<>(numValues,
ByteArrays.HASH_STRATEGY);
for (String stringValue : stringValues) {
bytesValues.add(isUuid ? UuidUtils.toBytes(stringValue) :
BytesUtils.toBytes(stringValue));
}
```
and the scan loop becomes a single allocation-free path:
```java
Set<byte[]> inBytesValues = (Set<byte[]>) _valueSet;
byte[][] bytesValues = _mainFunction.transformToBytesValuesSV(valueBlock);
for (int i = 0; i < length; i++) {
if (inBytesValues.contains(bytesValues[i])) {
_intValuesSV[i] = 1;
}
}
```
This also removes the pre-existing `new ByteArray(value)` per document on
the plain BYTES path, so it is a win beyond UUID.
Verified the strategy is load-bearing rather than incidental: swapping it
for a plain `ObjectOpenHashSet<byte[]>` fails both
`testUuidInTransformFunction` and the pre-existing
`testBytesInTransformFunction`, since arrays fall back to identity semantics
without it.
--
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]
