xiangfu0 commented on code in PR #18871:
URL: https://github.com/apache/pinot/pull/18871#discussion_r3642743232
##########
pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java:
##########
@@ -231,11 +235,12 @@ private VectorSchemaRoot
createVectorSchemaRoot(ResultTable resultTable, DataSch
break;
case BIG_DECIMAL:
case TIMESTAMP:
+ case UUID:
case STRING:
case JSON:
case BYTES:
case OBJECT:
- byte[] bytes = ((String) value).getBytes(StandardCharsets.UTF_8);
+ byte[] bytes = getVarCharValue(colType,
value).getBytes(StandardCharsets.UTF_8);
Review Comment:
Correcting my earlier answer: **no, it was not a bug fix** — I could not
produce a reachable failing case, so I have backed it out.
`encodeResultTable` has exactly one production caller, `BrokerGrpcServer`,
and it encodes an already-rendered `ResultTable`:
- single-stage → `SelectionOperatorUtils.renderResultTable*` →
`convertAndFormat(..)`
- multi-stage → `QueryDispatcher#toExternalList` → `format(toExternal(..))`
Both produce a `String` for every type in this VarChar group, UUID included.
So `((String) value)` was never reachable with a non-String, and
`getVarCharValue` was speculative hardening with no test behind it — exactly
the kind of thing that should not ride along in this PR.
`case UUID` now just joins the shared branch, which also resolves the
ordering you flagged above.
One consequence worth calling out: the test I had added fed a raw
`ByteArray` straight into the encoder, i.e. it asserted a state the broker
cannot actually produce. It now renders through `convertAndFormat` first,
matching the real path, and is renamed accordingly.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
