xiangfu0 commented on code in PR #18871:
URL: https://github.com/apache/pinot/pull/18871#discussion_r3599036379
##########
pinot-common/src/main/java/org/apache/pinot/common/response/encoder/ArrowResponseEncoder.java:
##########
@@ -104,6 +106,7 @@ private VectorSchemaRoot createVectorSchemaRoot(ResultTable
resultTable, DataSch
break;
case BIG_DECIMAL:
case TIMESTAMP:
+ case UUID:
Review Comment:
Reordered so `case UUID` follows `case BYTES` in this fall-through group —
and did the same in the other two Arrow groups, `JsonResponseEncoder`, and the
group-by/reduce switches. Behavior-neutral since they share one branch.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -303,6 +324,16 @@ public RelDataType toType(RelDataTypeFactory typeFactory) {
return typeFactory.createSqlType(SqlTypeName.MAP);
}
},
+ // NOTE: UUID is placed before OBJECT and the array types, shifting their
ordinals by +1 relative to any build that
+ // does not contain this enum constant. This is safe because DataSchema
serialization uses enum names (not ordinals)
+ // via ColumnDataType.name() / ColumnDataType.valueOf(). If ordinal-based
serialization is ever added for
+ // ColumnDataType, UUID must be moved to the end of the enum (as was done
for FieldSpec.DataType.UUID).
+ UUID(BYTES, null) {
Review Comment:
Moved `ColumnDataType.UUID` to directly after `BYTES`, and applied the same
after-`BYTES` ordering elsewhere (`UUID_ARRAY` already followed `BYTES_ARRAY`;
`FieldSpec.DataType.UUID` was already after `BYTES`). ColumnDataType is
name-serialized, so order doesn't affect the wire format.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
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:
Yes — `((String) value)` threw ClassCastException for the non-String types
sharing this VarChar branch (BIG_DECIMAL/TIMESTAMP/BYTES/OBJECT, and now UUID).
`getVarCharValue(colType, value)` renders each type's canonical string; kept it
since UUID needs it here.
_🤖 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]
