somu-imply commented on code in PR #14237:
URL: https://github.com/apache/druid/pull/14237#discussion_r1190582006


##########
processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java:
##########
@@ -144,9 +144,15 @@ static JoinClauseToFilterConversion convertJoinToFilter(
       final Set<String> rightPrefixes
   )
   {
+    // This optimization kicks in when there is exactly 1 equijoin
+    // The reason being that getNonNullColumnValues uses a TreeSet for 
handling duplicates
+    // and does not return values in order.
+    // In case TreeSet if replaced by a LinkedHashSet, duplicates are not 
handled correctly
+    // and resulting ordering is improper.
+    // Considering these joins are converted to filters only when there is 1 
equijoin
     if (clause.getJoinType() == JoinType.INNER
         && clause.getCondition().getNonEquiConditions().isEmpty()
-        && clause.getCondition().getEquiConditions().size() > 0) {
+        && clause.getCondition().getEquiConditions().size() == 1) {
       final List<Filter> filters = new ArrayList<>();

Review Comment:
   Yup, no need for the for loop anymore. Made things simpler



-- 
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]


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

Reply via email to