Skip to content

KAFKA-9763: Correct InsertField to not skip keys of tombstone records#8351

Closed
rhauch wants to merge 1 commit intoapache:trunkfrom
rhauch:kafka-9763
Closed

KAFKA-9763: Correct InsertField to not skip keys of tombstone records#8351
rhauch wants to merge 1 commit intoapache:trunkfrom
rhauch:kafka-9763

Conversation

@rhauch
Copy link
Copy Markdown
Contributor

@rhauch rhauch commented Mar 25, 2020

The fix for KAFKA-8523 attempted to skip (immediately returns) tombstone records when InsertField$Value is used. However, it also inadvertently also skips tombstone records when InsertField$Key is used, despite the possibility that the tombstone record’s key is non null.

This commit corrects the behavior so that the InsertField$Value continues to skip tombstone records, but InsertField$Key only skips when the record’s key is null.

Added two unit tests, and verified that these tests fail without the proposed fix and pass with the proposed fix. The other unit tests were added with KAFKA-8523 continue to pass.

This should be backported all the way back to the 1.0 branch, since that's how far back KAFKA-8523 was backported.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

The fix for KAFKA-8523 attempted to skip (immediately returns) tombstone records when InsertField$Value is used. However, it also inadvertently also skips tombstone records when InsertField$Key is used, despite the possibility that the tombstone record’s key is non null.

This commit corrects the behavior so that the InsertField$Value continues to skip tombstone records, but InsertField$Key only skips when the record’s *key* is null.

Added two unit tests, and verified that these tests fail on the previous code and pass with the proposed fix.
@rhauch rhauch requested a review from kkonstantine March 25, 2020 19:12
@rhauch
Copy link
Copy Markdown
Contributor Author

rhauch commented Mar 25, 2020

Closed in favor of #8280

@rhauch rhauch closed this Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant