appchemist commented on code in PR #18800:
URL: https://github.com/apache/kafka/pull/18800#discussion_r2173641799
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java:
##########
@@ -1307,6 +1295,7 @@ public <KOut, VOut> KStream<KOut, VOut> process(
name,
new ProcessorParameters<>(processorSupplier, name),
stateStoreNames);
+ processNode.requireRepartitionAlways();
Review Comment:
I initially thought of using `processNode.setKeyChangingOperation(true)` as
well.
However, I realized that using `processNode.setKeyChangingOperation(true)`
can lead to changes in the topology compared to the original behavior.
In fact, if we use `processNode.setKeyChangingOperation(true)`, the test
`StreamsGraphTest.shouldNotThrowNPEWithMergeNodes()` fails, and the resulting
topology differs as well.
For now, I decided to preserve the existing behavior from a refactoring
standpoint.
That’s the reasoning behind introducing the Repartition enum in GraphNode.
If changing the topology is not a concern, or if using
`processNode.setKeyChangingOperation(true)` is considered the correct approach,
then `Partition.REQUIRED` in `GraphNode` wouldn’t be necessary — we could just
use `processNode.setKeyChangingOperation(true)` instead.
--
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
