vvcephei commented on code in PR #12535:
URL: https://github.com/apache/kafka/pull/12535#discussion_r949388743
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/foreignkeyjoin/SubscriptionStoreReceiveProcessorSupplier.java:
##########
@@ -91,7 +91,7 @@ public void process(final Record<KO, SubscriptionWrapper<K>>
record) {
droppedRecordsSensor.record();
return;
}
- if (record.value().getVersion() !=
SubscriptionWrapper.CURRENT_VERSION) {
+ if (record.value().getVersion() >
SubscriptionWrapper.CURRENT_VERSION) {
Review Comment:
I assume that you've now searched the code base for every usage of
`getVersion` and `CURRENT_VERSION` to make sure we don't have any more sneaky
bugs, right?
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/foreignkeyjoin/CombinedKey.java:
##########
@@ -38,8 +38,22 @@ public KP getPrimaryKey() {
return primaryKey;
}
- public boolean equals(final KF foreignKey, final KP primaryKey) {
- return this.foreignKey.equals(foreignKey) &&
this.primaryKey.equals(primaryKey);
+ @Override
+ public int hashCode() {
+ return Objects.hash(foreignKey, primaryKey);
+ }
Review Comment:
Do we need to implement hashCode? It's not free of side effects. If we don't
actually need it, but also want to have a correct hashCode/equals contract, you
can throw an UnsupportedOperationException from the hashCode method,
effectively preventing anyone from using these objects in hash collections.
Why would you want to do that? Mostly if you can't guarantee that all fields
are immutable. OTOH, if all fields are immutable, then it's safe to just
implement it.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
