kfaraz commented on code in PR #13297:
URL: https://github.com/apache/druid/pull/13297#discussion_r1012445658


##########
processing/src/main/java/org/apache/druid/segment/nested/NestedFieldLiteralColumnIndexSupplier.java:
##########
@@ -223,16 +214,41 @@ private <T> IntIntPair getLocalRangeFromDictionary(
       }
     }
     globalEndIndex = Math.max(globalStartIndex, globalEndIndex);
-    // end index is not inclusive, so we find the last value in the local 
dictionary that falls within the range
+
+    // with global dictionary id range settled, now lets map that onto a local 
dictionary id range
+    int localFound = localDictionary.indexOf(globalStartIndex);
+    if (localFound < 0) {
+      // the first valid global index is not within the local dictionary, so 
the insertion point is where we begin
+      localStartIndex = -(localFound + 1);
+      // if the computed local start index violates the global range, shift up 
by 1
+      int actualGlobalStartIndex = localDictionary.get(localStartIndex);
+      if (actualGlobalStartIndex < globalStartIndex) {

Review Comment:
   Thanks! Simpler to follow now.
   
   I also wonder if the last value returned from `FixedIndex.indexOf()` should 
always be `-(minIndex + 1)`.
   My concern is that if we haven't found the value until now, then the 
insertion point could either be before or after the `currIndex` that we are 
evaluating. I think we should incorporate that fact into the returned value.
   
   Say at the last step, we found that our `value` is bigger than the 
`currValue`, so `minIndex` becomes `currIndex + 1`, then we break out of the 
loop and then we return `-(minIndex + 1) = -(currIndex + 2)`.
   
   Doesn't this mean that we have actually skipped the insertion point? Or 
maybe I am missing something.
   (Although, I guess this should be okay because the `Indexed` interface 
doesn't make any promises about the returned negative value.)



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