Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

cadonna merged PR #15601: URL: https://github.com/apache/kafka/pull/15601 -- 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.

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2135147503 > @raminqaf Could you please rebase this PR on current trunk to get the new build setup? We need to restart the builds because one was red. Done! -- This is an automated messag

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

cadonna commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2135043687 @raminqaf Could you please rebase this PR on current trunk to get the new build setup? We need to restart the builds because one was red. -- This is an automated message from the Apach

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1607786468 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -187,17 +157,25 @@ public void process(final Record record) {

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

cadonna commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1605020116 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java: ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

cadonna commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1604937635 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -187,17 +157,25 @@ public void process(final Record record) {

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600269458 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsCo

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600077088 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsConfi

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsConfi

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsConfi

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600088096 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsConfi

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1600076976 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java: ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

cadonna commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r166724 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -43,7 +42,7 @@ import static org.apache.kafka.streams.StreamsConfig

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1593500243 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -221,41 +199,28 @@ private void emitNonJoinedOuterRecords(

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

ableegoldman commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2099136409 Thanks Greg. If no one's had time to look at this by next week, we'll assign a reviewer during the next Kafka Streams hangout -- This is an automated message from the Apache Git Se

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1592924655 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -187,17 +157,25 @@ public void process(final Record record) {

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2098817942 @gharris1727 Thanks for the feedback! I reverted all the changes you requested and reverted a couple of other indentation problems that caused a diff. I can even go further and revert &

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1591293880 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -184,20 +146,34 @@ public void process(final Record record) {

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2084636567 @gharris1727 Thanks for the positive feedback! I am happy that you like the changes! I addressed all your nit reviews and refactored the logic of the non-final boolean variables (i.e., `

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1583698116 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -240,27 +185,33 @@ private void emitNonJoinedOuterRecords(

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1563938474 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## Review Comment: No more un safe type casting to be found in this c

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1563929782 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## @@ -63,21 +63,6 @@ public static LeftOrRightValue makeRightValue(final V2

Re: [PR] KAFKA-16362: Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1554016667 ## streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyAndJoinSide.java: ## @@ -33,28 +34,36 @@ public class TimestampedKeyAndJoinSide {

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1545788943 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamRightJoin.java: ## @@ -0,0 +1,221 @@ +/* + * Licensed to the Apache Software Foundati

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1545727392 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -16,276 +16,98 @@ */ package org.apache.kafka.streams.kstream.int

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

mcmmining commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1545728831 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamLeftJoin.java: ## @@ -0,0 +1,201 @@ +/* + * Licensed to the Apache Software Foundati

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1545727392 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoin.java: ## @@ -16,276 +16,98 @@ */ package org.apache.kafka.streams.kstream.int

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2028756399 @gharris1727 I have broken down the KStreamKstreamJoin class into two classes. For now, I just moved the code (+the fix in #15510) to see if all the tests pass and if I am going in the c

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on PR #15601: URL: https://github.com/apache/kafka/pull/15601#issuecomment-2027729205 This PR looks like it'll have merge conflicts with #15510. Since that is fixing a bug it should probably have higher priority than this refactor, can you rebase on top of their change

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1544779363 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImplJoin.java: ## @@ -41,6 +41,7 @@ import java.util.Optional; import java.util.Set; i

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1544787082 ## streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java: ## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749545 ## streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java: ## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749545 ## streams/src/main/java/org/apache/kafka/streams/state/internals/JoinSide.java: ## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1544749273 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## Review Comment: @gharris1727 Thanks for the fast review! I have addres

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1540138631 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## @@ -39,45 +39,6 @@ private LeftOrRightValue(final V1 leftValue, final V2

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

gharris1727 commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1540136195 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## Review Comment: I assumed that this would be necessary. I think eit

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## Review Comment: Retroperspectivly, we can refactor this class and brea

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## Review Comment: Retroperspectivly, we can refactor this class and brea

Re: [PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf commented on code in PR #15601: URL: https://github.com/apache/kafka/pull/15601#discussion_r1539078322 ## streams/src/main/java/org/apache/kafka/streams/state/internals/LeftOrRightValue.java: ## Review Comment: Retroperspectivly, we can refactor this class and brea

[PR] KAFKA-16362 Fix type-unsafety in KStreamKStreamJoin caused by isLeftSide [kafka]

raminqaf opened a new pull request, #15601: URL: https://github.com/apache/kafka/pull/15601 Description The introduced changes provide a cleaner definition of the join side in KStreamKStreamJoin. Before, this was done by using a Boolean flag, which led to returning a raw LeftOrRight