guozhangwang commented on code in PR #12555:
URL: https://github.com/apache/kafka/pull/12555#discussion_r958991975


##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/InternalStreamsBuilder.java:
##########
@@ -270,16 +272,20 @@ private void maybeAddNodeForOptimizationMetadata(final 
GraphNode node) {
 
     // use this method for testing only
     public void buildAndOptimizeTopology() {
-        buildAndOptimizeTopology(false);
+        buildAndOptimizeTopology(false, false);
     }
 
-    public void buildAndOptimizeTopology(final boolean optimizeTopology) {
+    public void buildAndOptimizeTopology(
+        final boolean optimizeTopology, final boolean optimizeSelfJoin) {
 
         mergeDuplicateSourceNodes();
         if (optimizeTopology) {
             LOG.debug("Optimizing the Kafka Streams graph for repartition 
nodes");
             optimizeKTableSourceTopics();
             maybeOptimizeRepartitionOperations();
+            if (optimizeSelfJoin) {

Review Comment:
   The original idea for not doing "two-pass" mechanism is that some of the 
optimizations, like renaming the internal topics, are only doable with the 
configs in place which was not the case before. But after thinking about your 
arguments I agree that optimizations such as self-joins would better be done 
after the original logical plan is built, so we should keep the second pass 
still for this and future optimization rules.
   
   Following that, my next thought would be, is there an easier condition to 
check if this rule can be applied. The current condition as you stated in the 
comments need to check three conditions, and I'm wondering if we can just check 
condition #2 i.e. the StreamStreamJoinNode has only one parent. Could you 
elaborate a bit why we need the other two conditions as well?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to