Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.kafka.common.protocol.types;

import org.apache.kafka.common.protocol.types.Type.DocumentedType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few things that we need to fix in clients/src/main/resources/common/message/README.md.

  1. We need to add type struct in Field Types.
  2. In Nullable Fields, it says uuid is nullable. This is incorrect (the generator throws an exception if a uuid filed is nullable) and needs to be removed.
  3. We need to add struct as a nullable type.


import java.nio.ByteBuffer;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -24,7 +26,10 @@
/**
* The schema for a compound record definition
*/
public final class Schema extends Type {
public final class Schema extends DocumentedType {

private static final String STRUCT_TYPE_NAME = "NULLABLE_STRUCT";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current Schema class is written for a struct that's not nullable since the write() method just writes all fields without the null indicator. We should name this STRUCT and create a new NullableStruct version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without the null indicator

The documentation for other nullable types also needs to be updated to mention the null indicator.


private static final Object[] NO_VALUES = new Object[0];

private final BoundField[] fields;
Expand Down Expand Up @@ -206,6 +211,19 @@ public Struct validate(Object item) {
}
}

@Override
public String typeName() {
return STRUCT_TYPE_NAME;
}

@Override
public String documentation() {
return "Represents a composite object or null. " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should first say sth like "A struct is named by a string with a capitalized first letter and consists of one or more fields".

"For non-null values, the first byte is an INT8 with value 1, " +
"followed by the serialization of each field in the order they are defined. " +
"A null value is encoded as an INT8 with value -1 and there are no following bytes.";
}

public void walk(Visitor visitor) {
Objects.requireNonNull(visitor, "visitor must be non-null");
handleNode(this, visitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,8 @@ private static String toHtml() {
UINT16, UNSIGNED_INT32, VARINT, VARLONG, UUID, FLOAT64,
STRING, COMPACT_STRING, NULLABLE_STRING, COMPACT_NULLABLE_STRING,
BYTES, COMPACT_BYTES, NULLABLE_BYTES, COMPACT_NULLABLE_BYTES,
RECORDS, COMPACT_RECORDS, new ArrayOf(STRING), new CompactArrayOf(COMPACT_STRING)};
RECORDS, COMPACT_RECORDS, new ArrayOf(STRING), new CompactArrayOf(COMPACT_STRING),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple of existing issues with the generated schemas in https://kafka.apache.org/protocol#protocol_api_keys.

  1. The current RECORDS class is written as NULLABLE_RECORDS. So, we should rename it NULLABLE_RECORDS. We also need to create a new RECORDS class that's not nullable. In SchemaGenerator, we should distinguish between RECORDS and NULLABLE_RECORDS. Currently, the generated schema only has RECORDS for both nullable and non-nullable fields, which is misleading. Ditto for COMPACT_RECORDS.

  2. For ArrayOf, it takes nullable as the input, but its name is always ARRAY. We need to add the nullable name. Ditto for CompactArrayOf. Currently, the generator schema only uses [] to represent an array. We need a way to denote whether it's nullable and compact.

  3. When generating the schema for a struct, we just fold in the fields in the struct like the following without indicating whether that struct is nullable or not. We need a way to make that clear.

  assignment => [topic_partitions] _tagged_fields 
    topic_partitions => topic_id [partitions] _tagged_fields 
      topic_id => UUID
      partitions => INT32

new Schema()};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you generate the website with this change?

final StringBuilder b = new StringBuilder();
b.append("<table class=\"data-table\"><tbody>\n");
b.append("<tr>");
Expand Down