xiangfu0 commented on code in PR #18876:
URL: https://github.com/apache/pinot/pull/18876#discussion_r3491640708
##########
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:
This makes `UUID` a new broker/server wire-visible `ColumnDataType`.
`DataSchema.toBytes()` emits the enum name, and older peers still throw in
`parseColumnDataType(...)` once they see `UUID`, so rolling upgrades and
rollback are unsafe as soon as UUID-typed results are in flight. Please keep
the wire representation on an existing type until all peers are upgraded, or
add an explicit mixed-version compatibility path plus coverage.
##########
pinot-common/src/main/proto/expressions.proto:
##########
@@ -44,6 +44,12 @@ enum ColumnDataType {
UNKNOWN = 19;
MAP = 20;
BIG_DECIMAL_ARRAY = 21;
+ // Rolling-upgrade limitation for UUID columns: in a mixed-version
multi-stage query, an older broker/server that
+ // does not know UUID = 22 / UUID_ARRAY = 23 will fail planning with
UnknownEnumValueException when receiving a plan
+ // that includes a UUID literal. Avoid issuing UUID queries until all
brokers and servers are upgraded. See the
+ // matching note on DataSchema.toBytes and
ProtoExpressionToRexExpression#convertColumnDataType.
+ UUID = 22;
Review Comment:
This introduces new `UUID` / `UUID_ARRAY` proto enum values with no
compatibility path for older MSQ peers. Older brokers/servers decode them as
`UNRECOGNIZED` and throw in `convertColumnDataType(...)`, so a UUID literal can
break mixed-version planning before execution even starts. Please encode UUID
literals using an existing wire type until the cluster is homogeneous, or add
version-gated dual-read/dual-write behavior with mixed-version tests.
--
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]
