xiangfu0 commented on code in PR #18872:
URL: https://github.com/apache/pinot/pull/18872#discussion_r3740131291
##########
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/filter/PredicateRowMatcher.java:
##########
@@ -78,6 +79,8 @@ public boolean isMatch(Object[] row) {
return _predicateEvaluator.applySV((String) value);
case BYTES:
return _predicateEvaluator.applySV((byte[]) value);
+ case UUID:
+ return _predicateEvaluator.applySV(UuidUtils.toBytes(value));
Review Comment:
You were right, and it is deterministic — changed to a direct cast:
```java
case UUID:
return _predicateEvaluator.applySV(UuidUtils.toBytes((UUID) value));
```
I traced all four paths that reach `isMatch`. `GroupByDataTableReducer` runs
every column through `ColumnDataType#convert` immediately before calling it:
```java
row[i] = columnDataTypes[i].convert(value);
...
if (havingFilterHandler.isMatch(row)) {
```
and `convert` for UUID returns `UuidUtils.toUUID((ByteArray) value)`, so the
value is always a `java.util.UUID`. That also matches the neighbouring
branches, which take external forms too — `BOOLEAN` casts to `boolean` and
`TIMESTAMP` to `java.sql.Timestamp`, both of which are what `toExternal`
produces.
The other reducer path cannot deliver a UUID here at all: `getConvertedKey`
has no `case UUID` and hits its `throw new IllegalStateException("Illegal
column data type in group key: ...")` default. So there is no second input
shape to accommodate.
Added `HavingFilterHandlerTest.testHavingFilterOnUuidColumn`, since as I
noted earlier this line had no test in either direction. I checked it is not
vacuous — replacing the branch body with a throw fails that test specifically,
so it does reach `case UUID`.
Separately, that missing `case UUID` in `getConvertedKey` means `GROUP BY`
on a UUID column throws on the `DataTable` reduce path. It is pre-existing and
unrelated to predicates, so I have left it out of this PR — happy to file it as
a follow-up.
--
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]
