Skip to content

KAFKA-19833: Reduce code duplication in nullable protocol types#21019

Merged
junrao merged 9 commits intoapache:trunkfrom
DL1231:KAFKA-19833
Feb 10, 2026
Merged

KAFKA-19833: Reduce code duplication in nullable protocol types#21019
junrao merged 9 commits intoapache:trunkfrom
DL1231:KAFKA-19833

Conversation

@DL1231
Copy link
Copy Markdown
Collaborator

@DL1231 DL1231 commented Nov 29, 2025

Nullable types (NULLABLE_STRING, NULLABLE_BYTES, NULLABLE_RECORDS,
etc.) in Type.java duplicate most of the read/write/sizeOf logic from
their non-nullable counterparts.

This PR reduces the duplication by:

  • Extracting shared stringRead() and bytesRead() helper methods
    for common read logic
  • Delegating nullable type write()/read()/sizeOf() to the
    corresponding non-nullable type after null checks

@github-actions github-actions Bot added triage PRs from the community connect generator RPC and Record code generator transactions Transactions and EOS clients labels Nov 29, 2025
@DL1231 DL1231 removed connect transactions Transactions and EOS labels Nov 29, 2025
@DL1231 DL1231 requested review from chia7712 and junrao November 29, 2025 07:51
@github-actions github-actions Bot added connect transactions Transactions and EOS labels Dec 1, 2025
@DL1231
Copy link
Copy Markdown
Collaborator Author

DL1231 commented Dec 1, 2025

Hi @junrao @chia7712, could you please review this when you have a moment? Thank you!

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 7, 2025

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@DL1231 : Thanks for the PR. Left a comment.

Comment thread clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java Outdated
@github-actions github-actions Bot removed triage PRs from the community needs-attention labels Dec 13, 2025
@DL1231 DL1231 removed connect transactions Transactions and EOS labels Dec 17, 2025
@DL1231 DL1231 requested a review from junrao December 25, 2025 07:23
Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@DL1231 : Thanks for the updated PR. Sorry for the late reply. Added a few more comments.

* Represents a type for an array of a particular type
*/
public class ArrayOf extends DocumentedType {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we kept the BYTES implementation, we already have different patterns for implementing nullable types. So, instead of unifying the pattern, the goal should probably be to avoid code duplication. Since there is no code duplication in the existing ArrayOf, we can just keep it as it is. Ditto for CompactArraryOf.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Comment thread clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java Outdated
Comment thread clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java Outdated
# Conflicts:
#	clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
#	transaction-coordinator/src/test/java/org/apache/kafka/coordinator/transaction/TransactionLogTest.java
@github-actions github-actions Bot added the transactions Transactions and EOS label Feb 10, 2026
@DL1231 DL1231 changed the title KAFKA-19833: Refactor Nullable Types to Use a Unified Pattern KAFKA-19833: Reduce code duplication in nullable protocol types Feb 10, 2026
@DL1231 DL1231 removed the transactions Transactions and EOS label Feb 10, 2026
@DL1231 DL1231 requested a review from junrao February 10, 2026 06:36
Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@DL1231 : Thanks for the updated PR. LGTM

@junrao junrao merged commit 9a9e497 into apache:trunk Feb 10, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clients generator RPC and Record code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants