[GitHub] [druid] paul-rogers commented on a diff in pull request #13922: Refactoring and bug fixes on top of unnest. The filter now is passed …

["paul-rogers (via GitHub)" <gi...@apache.org>]

paul-rogers commented on code in PR #13922:
URL: https://github.com/apache/druid/pull/13922#discussion_r1132886605


##########
processing/src/main/java/org/apache/druid/segment/UnnestDimensionCursor.java:
##########
@@ -330,22 +327,19 @@ public void reset()
   @Nullable
   private void initialize()
   {
-    IdLookup idLookup = dimSelector.idLookup();
-    this.indexIntsForRow = new SingleIndexInts();
-    if (allowSet != null && !allowSet.isEmpty() && idLookup != null) {
-      for (String s : allowSet) {
-        if (idLookup.lookupId(s) >= 0) {
-          allowedBitSet.set(idLookup.lookupId(s));
-        }
-      }
+    index = 0;
+    if (allowFilter != null) {

Review Comment:
   Nit: reverse priority



##########
processing/src/main/java/org/apache/druid/segment/UnnestDimensionCursor.java:
##########
@@ -290,9 +283,13 @@ public void advance()
   @Override
   public void advanceUninterruptibly()
   {
-    do {
+    while (true) {
       advanceAndUpdate();
-    } while (matchAndProceed());
+      boolean match = valueMatcher.matches();
+      if (match || baseCursor.isDone()) {
+        return;
+      }
+    }

Review Comment:
   Nit:
   
   ```java
         if (valueMatcher.matches() || baseCursor.isDone()) {
   ```



##########
processing/src/main/java/org/apache/druid/segment/UnnestColumnValueSelectorCursor.java:
##########
@@ -310,12 +314,15 @@ private void getNextRow()
   private void initialize()
   {
     getNextRow();
-    if (allowSet != null) {
-      if (!allowSet.isEmpty()) {
-        if (!allowSet.contains((String) unnestListForCurrentRow.get(index))) {
-          advance();
-        }
-      }
+    if (allowFilter != null) {

Review Comment:
   Nit: reverse polarity.
   
   ```java
       if (allowFilter = null) {
         this.valueMatcher = BooleanValueMatcher.of(true);
       } else {
         this.valueMatcher = allowFilter.makeMatcher(getColumnSelectorFactory());
       }
   ```



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org