[GitHub] [incubator-druid] himanshug commented on a change in pull request #7618: Virtual column updates for exploiting base column internal structure

[GitBox <gi...@apache.org>]

himanshug commented on a change in pull request #7618: Virtual column updates for exploiting base column internal structure
URL: https://github.com/apache/incubator-druid/pull/7618#discussion_r284474011
 
 

 ##########
 File path: processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
 ##########
 @@ -159,12 +162,14 @@
     final boolean includeTimestamp = GroupByStrategyV2.getUniversalTimestamp(query) == null;
 
     final ThreadLocal<Row> columnSelectorRow = new ThreadLocal<>();
-    final ColumnSelectorFactory columnSelectorFactory = query.getVirtualColumns().wrap(
-        RowBasedColumnSelectorFactory.create(
-            columnSelectorRow,
-            rawInputRowSignature
-        )
+
+    ColumnSelectorFactory columnSelectorFactory = RowBasedColumnSelectorFactory.create(
+        columnSelectorRow,
+        rawInputRowSignature
     );
+    if (useVirtualizedColumnSelectorFactory) {
 
 Review comment:
   Things would work without this change but not in most optimal way.
   
   `VirtualizedColumnSelectorFactory` always works by the "row" based methods (that existed before) , however point of this PR is to use "column" based version of those methods wherever possible (e.g. "column" is not available when processing query on IncrementalIndex or processing outer query of a groupBy and outer query has virtual columns).
   Without this change, all groupBy query processing would blindly use row based methods in VirtualColumn and discard more efficient column based methods (even if they were implemented) , this change is making sure that we use row based methods only when necessary during groupBy query processing.
   there are unit tests introduced in this PR which would fail without this change, to ensure that things stay this way.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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