gianm commented on a change in pull request #10697:
URL: https://github.com/apache/druid/pull/10697#discussion_r587927920



##########
File path: processing/src/main/java/org/apache/druid/query/JoinDataSource.java
##########
@@ -108,10 +122,26 @@ public static JoinDataSource create(
       final DataSource right,
       final String rightPrefix,
       final JoinConditionAnalysis conditionAnalysis,
-      final JoinType joinType
+      final JoinType joinType,
+      final DimFilter leftFilter
+  )
+  {
+    return new JoinDataSource(left, right, rightPrefix, conditionAnalysis, 
joinType, leftFilter);
+  }
+
+  /**
+   * Create a join dataSource from an existing {@link JoinConditionAnalysis}.

Review comment:
       The javadoc doesn't seem right, it doesn't accept a 
JoinConditionAnalysis.
   
   Fwiw, also, I don't really like adding "everything minus one" constructors 
when a new parameter is added. It minimizes the amount of code to update, but 
it leaves things messier, and it makes it more likely that someone will forget 
to use the new parameter when it is actually important to use. IMO it is better 
to remove this constructor and update all the call sites to use the new one.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to