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

["imply-cheddar (via GitHub)" <gi...@apache.org>]

imply-cheddar commented on code in PR #13922:
URL: https://github.com/apache/druid/pull/13922#discussion_r1134828705


##########
processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java:
##########
@@ -138,13 +135,16 @@ public Sequence<Cursor> makeCursors(
                 retVal.getColumnSelectorFactory(),
                 unnestColumn,
                 outputColumnName,
-                allowSet
+                filterPair.rhs
             );
           }
+          // This is needed at this moment for nested queries
+          // Future developer would want to move the virtual columns
+          // inside the UnnestCursor and wrap the columnSelectorFactory
           return PostJoinCursor.wrap(
               retVal,
               virtualColumns,
-              filterPair.rhs
+              null

Review Comment:
   The implementation that we had talked about would be push the filter down all the way to the `StorageAdapter` without any figuring out of which filters to maybe push down (Calcite has already figured that out for us by separating the filters on the two sides of the LogicalCorrelate).
   
   The intent of the implementation (maybe it wasn't done that way though) was that, for applying the filter on the read, it would just be a ValueMatcher happening at read-time which can reuse other code like the `PostJoinCursor`.
   
   The point of attaching the Filter on the UnnestColumn is to make ordering of things more explicit, with the intention that native queries are supposed to be explicit "you told me to do X, so I do X" things.



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