Jackie-Jiang commented on code in PR #19667:
URL: https://github.com/apache/pinot/pull/19667#discussion_r4113683421


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java:
##########
@@ -309,7 +312,7 @@ private Object deserializeValue(ByteBuffer buffer) {
       case BIG_DECIMAL:
         return new BigDecimal(new String(deserializeVariableBytes(buffer), 
StandardCharsets.UTF_8));
       case BYTES:
-        return deserializeVariableBytes(buffer);
+        return new ByteArray(deserializeVariableBytes(buffer));

Review Comment:
   `ANY_VALUE` still stores a raw `byte[]` when it selects a BYTES value 
(`getDictionaryValue` / `getDirectValue`), but this deserializer now returns 
`ByteArray`. That makes the intermediate type depend on whether it crossed 
ser/de. It also leaves the normal BYTES DataTable writer's `(ByteArray) result` 
cast failing for values produced directly by aggregation. Could we wrap the 
selected value when storing it in the aggregation result holder, keep 
`ByteArray` on both sides of ser/de, and update this test to serialize an 
actual aggregation result and assert its type before and after? Once that 
invariant holds, the `instanceof byte[]` serialization branch can be removed.



-- 
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]

Reply via email to