Skip to content

KAFKA-9921: explicit handling of null values with retainDuplicates#8626

Merged
mjsax merged 5 commits intoapache:trunkfrom
ableegoldman:9921-add-missing-test-and-docs
May 11, 2020
Merged

KAFKA-9921: explicit handling of null values with retainDuplicates#8626
mjsax merged 5 commits intoapache:trunkfrom
ableegoldman:9921-add-missing-test-and-docs

Conversation

@ableegoldman
Copy link
Copy Markdown
Member

In general the behavior of window stores with retainDuplicates is not well documented or enforced, so we should attempt to clarify things better in the javadocs and in the code itself. This explicitly skips the put/delete when the value is null and duplicates are allowed, and specifies this behavior in the docs.

Also adds in a test I left out in the earlier PR #8564

@mjsax
Copy link
Copy Markdown
Member

mjsax commented May 8, 2020

Retest this please

Comment thread streams/src/main/java/org/apache/kafka/streams/state/WindowStore.java Outdated
ableegoldman and others added 2 commits May 8, 2020 13:46
Co-authored-by: GeorgiPetkov <gp000077@gmail.com>
@mjsax
Copy link
Copy Markdown
Member

mjsax commented May 8, 2020

Retest this please

Copy link
Copy Markdown
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

LGTM (assuming Jenkins passes).

@mjsax
Copy link
Copy Markdown
Member

mjsax commented May 8, 2020

Retest this please.

@ableegoldman
Copy link
Copy Markdown
Member Author

Java 14 passed, Java 8 and Java 11 has one failure each but results cleaned up already.

@mjsax mjsax merged commit 72d72a9 into apache:trunk May 11, 2020
mjsax pushed a commit that referenced this pull request May 11, 2020
@mjsax
Copy link
Copy Markdown
Member

mjsax commented May 11, 2020

Merged to trunk and cherry-picked to 2.5.

@ableegoldman ableegoldman deleted the 9921-add-missing-test-and-docs branch May 12, 2020 22:59
jwijgerd pushed a commit to buxapp/kafka that referenced this pull request May 14, 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.

3 participants