junrao commented on code in PR #20614:
URL: https://github.com/apache/kafka/pull/20614#discussion_r2539989419
##########
generator/src/main/java/org/apache/kafka/message/SchemaGenerator.java:
##########
@@ -86,16 +86,16 @@ void generateSchemas(MessageSpec message) {
// available when we generate the inline structures
for (Iterator<StructSpec> iter = structRegistry.commonStructs();
iter.hasNext(); ) {
StructSpec struct = iter.next();
- generateSchemas(struct.name(), struct,
message.struct().versions());
+ generateSchemas(struct.name(), struct,
message.struct().versions(), Versions.NONE);
Review Comment:
> If Y is nullable, then declare that field as new NullableSchema(X);
> if Z is non-nullable, then reference X directly.
> X, by default, should be declared as new Schema().
@DL1231 : Thinking a bit more. I feel the above solution that you proposed
probably works the best. We will need to change the constructor of
NullableSchema to take a Schema. We will use this approach for both shared and
non-shared schema when generating the classes.
##########
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java:
##########
@@ -66,6 +66,8 @@ private static void schemaToBnfHtml(Schema schema,
StringBuilder b, int indentSi
b.append(indentStr);
b.append(entry.getKey());
b.append(" => ");
+ b.append(((Schema) entry.getValue()).typeName());
+ b.append(" ");
Review Comment:
The generated HTML looks like this:
```
ConsumerGroupHeartbeat Response (Version: 0) => STRUCT throttle_time_ms
error_code error_message member_id member_epoch heartbeat_interval_ms
assignment
throttle_time_ms => INT32
error_code => INT16
error_message => COMPACT_NULLABLE_STRING
member_id => COMPACT_NULLABLE_STRING
member_epoch => INT32
heartbeat_interval_ms => INT32
assignment => NULLABLE_STRUCT (topic_partitions)
topic_partitions => STRUCT topic_id (partitions)
topic_id => UUID
partitions => INT32
```
Would it be better to use {} and ?{} to represent struct and nullable struct
like the following? We will also want to change [T]? and (T)? to ?[T] and ?(T)
to be consistent.
```
ConsumerGroupHeartbeat Response (Version: 0) => {throttle_time_ms error_code
error_message member_id member_epoch heartbeat_interval_ms assignment}
throttle_time_ms => INT32
error_code => INT16
error_message => COMPACT_NULLABLE_STRING
member_id => COMPACT_NULLABLE_STRING
member_epoch => INT32
heartbeat_interval_ms => INT32
assignment => ?{ (topic_partitions) }
topic_partitions => STRUCT topic_id (partitions)
topic_id => UUID
partitions => INT32
```
--
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]
