[GitHub] [kafka] mjsax commented on a change in pull request #10462: KAFKA-10847: Fix spurious results on left/outer stream-stream joins

[GitBox]

mjsax commented on a change in pull request #10462:
URL: https://github.com/apache/kafka/pull/10462#discussion_r616264850



##########
File path: 
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImplJoin.java
##########
@@ -118,20 +142,47 @@
         final ProcessorGraphNode<K1, V2> otherWindowedStreamsNode = new 
ProcessorGraphNode<>(otherWindowStreamProcessorName, 
otherWindowStreamProcessorParams);
         builder.addGraphNode(otherGraphNode, otherWindowedStreamsNode);
 
+        Optional<StoreBuilder<WindowStore<KeyAndJoinSide<K1>, 
LeftOrRightValue<V1, V2>>>> outerJoinWindowStore = Optional.empty();
+        if (leftOuter || rightOuter) {
+            final String outerJoinSuffix = leftOuter ? 
"-shared-left-outer-join" : "-shared-outer-join";

Review comment:
       That was the intent of my comment, but if you look into the newly added 
tests in `TopologyTest.java` it might not matter too much, as we also have some 
"weird" naming in existing code -- and to stay backward compatible, we cannot 
really change the naming:
   
   ```
   inner-join: (store names)
    - KSTREAM-JOINTHIS-0000000004-store
    - KSTREAM-JOINOTHER-0000000005-store
   
   left-join: (store names)
    - KSTREAM-JOINTHIS-0000000004-store
    - KSTREAM-OUTEROTHER-0000000005-store
   
   (Ideally we should have named both KSTREAM-LEFTTHIS-0000000004-store and 
KSTREAM-LEFTOTHER-0000000005-store...)
   
   outer-join: (store names)
    - KSTREAM-OUTERTHIS-0000000004-store
    - KSTREAM-OUTEROTHER-0000000005-store
    ```




-- 
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.

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