[GitHub] [kafka] vvcephei commented on a diff in pull request #12535: KAFKA-13769 Fix version check in SubscriptionStoreReceiveProcessorSupplier

[GitBox <gi...@apache.org>]

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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org