You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "appchemist (via GitHub)" <gi...@apache.org> on 2026/02/22 11:57:12 UTC
[PR] MINOR: fix null overwrite in recursive parent-node matching [kafka]
appchemist opened a new pull request, #21540:
URL: https://github.com/apache/kafka/pull/21540
Description:
This change fixes a bug in recursive parent-node matching where a previously found ancestor match could be overwritten by a later branch returning null.
The issue occurs when traversing multiple parents.
if an earlier parent path finds a matching ancestor but a later parent path does not, the previous implementation could return null instead of preserving the first valid match.
This can lead to incorrect ancestor detection during topology optimization for merged streams and may prevent expected repartition-topic sharing behavior.
The fix returns immediately when a recursive parent traversal finds a non-null match, and returns null only after all parent branches have been checked with no match.
Testing strategy:
- Added a unit test that reproduces the multi-parent traversal case and verifies the matching ancestor is preserved.
- Added topology optimization tests for merged streams with key-changing operations on both left and right branches, verifying repartition-topic sharing still produces a single repartition node.
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4199940163
@appchemist Thanks for the PR! Appologies for the delay, I'll make a pass over this today. Since this has been open for a bit can you rebase this?
--
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
Re: [PR] MINOR: fix overwrite in recursive parent-node matching [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-3981860287
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4202506667
Thanks @appchemist - still needs `./gradlew spotlessApply` to fix some import formatting issues
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4210442522
Merged #21540 into trunk
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4210477121
cherry-picked to 4.3
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on code in PR #21540:
URL: https://github.com/apache/kafka/pull/21540#discussion_r3054623251
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/InternalStreamsBuilder.java:
##########
@@ -688,20 +688,25 @@ private boolean isVersionedOrVersionedUpstream(final GraphNode startSeekingNode)
return isVersionedUpstream(startSeekingNode);
}
- private GraphNode findParentNodeMatching(final GraphNode startSeekingNode,
+ // Visible for testing.
+ GraphNode findParentNodeMatching(final GraphNode startSeekingNode,
Review Comment:
nit: align parameters
--
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
Re: [PR] MINOR: fix null overwrite in recursive parent-node matching [kafka]
Posted by "appchemist (via GitHub)" <gi...@apache.org>.
appchemist commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-3940796360
Hello, @mjsax
While working on [KAFKA-18692](https://github.com/apache/kafka/pull/18800), I found an issue in the existing code.
Since this code is likely to remain unchanged until [KAFKA-18692](https://github.com/apache/kafka/pull/18800) is merged, I am submitting a separate PR that fixes only this issue.
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "appchemist (via GitHub)" <gi...@apache.org>.
appchemist commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4042703056
@Nikita-Shupletsov kindly ping
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "appchemist (via GitHub)" <gi...@apache.org>.
appchemist commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4202451999
@bbejeck hello
sure, I rebased it now
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "Nikita-Shupletsov (via GitHub)" <gi...@apache.org>.
Nikita-Shupletsov commented on code in PR #21540:
URL: https://github.com/apache/kafka/pull/21540#discussion_r2913018233
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/InternalStreamsBuilderTest.java:
##########
@@ -1235,6 +1239,84 @@ public void shouldSetUseVersionedSemanticsOnTableForeignSelfJoin() {
verifyVersionedSemantics((ForeignTableJoinNode<?, ?>) joinOther, true);
}
+ @Test
+ public void shouldFindMatchingAncestorWhenLaterParentHasNoMatch() throws Exception {
+ // Given:
+ final GraphNode matchingNode = newTestGraphNode("matchingNode");
+ matchingNode.setKeyChangingOperation(true);
+
+ final GraphNode p1 = newTestGraphNode("p1");
+ final GraphNode p2 = newTestGraphNode("p2");
+ final GraphNode startSeekNode = newTestGraphNode("startSeekNode");
+ matchingNode.addChild(p1);
+ p1.addChild(startSeekNode);
+ p2.addChild(startSeekNode);
+
+ final Method method = InternalStreamsBuilder.class
Review Comment:
nit: may be easier to make the method package-private, as the test and the class are in the same package. and leave a comment `// Visible for testing`.
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4201195857
@appchemist the build failure is related, can you address 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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck merged PR #21540:
URL: https://github.com/apache/kafka/pull/21540
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "appchemist (via GitHub)" <gi...@apache.org>.
appchemist commented on code in PR #21540:
URL: https://github.com/apache/kafka/pull/21540#discussion_r2914999470
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/InternalStreamsBuilderTest.java:
##########
@@ -1235,6 +1239,84 @@ public void shouldSetUseVersionedSemanticsOnTableForeignSelfJoin() {
verifyVersionedSemantics((ForeignTableJoinNode<?, ?>) joinOther, true);
}
+ @Test
+ public void shouldFindMatchingAncestorWhenLaterParentHasNoMatch() throws Exception {
+ // Given:
+ final GraphNode matchingNode = newTestGraphNode("matchingNode");
+ matchingNode.setKeyChangingOperation(true);
+
+ final GraphNode p1 = newTestGraphNode("p1");
+ final GraphNode p2 = newTestGraphNode("p2");
+ final GraphNode startSeekNode = newTestGraphNode("startSeekNode");
+ matchingNode.addChild(p1);
+ p1.addChild(startSeekNode);
+ p2.addChild(startSeekNode);
+
+ final Method method = InternalStreamsBuilder.class
Review Comment:
thank you.
fixed 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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4015445532
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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
Re: [PR] KAFKA-20264: fix overwrite in recursive parent-node matching [kafka]
Posted by "appchemist (via GitHub)" <gi...@apache.org>.
appchemist commented on PR #21540:
URL: https://github.com/apache/kafka/pull/21540#issuecomment-4202544121
Thanks @bbejeck , fixed some import formatting issues with `./gradlew spotlessApply`
--
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