clintropolis commented on a change in pull request #11081:
URL: https://github.com/apache/druid/pull/11081#discussion_r610305869
##########
File path:
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
##########
@@ -398,8 +399,10 @@ public static ColumnSelectorFactory
createResultRowBasedColumnSelectorFactory(
);
final SettableSupplier<ResultRow> rowSupplier = new SettableSupplier<>();
- final ColumnSelectorFactory columnSelectorFactory =
-
RowBasedGrouperHelper.createResultRowBasedColumnSelectorFactory(subquery,
rowSupplier);
+ final ColumnSelectorFactory columnSelectorFactory = new
VirtualizedColumnSelectorFactory(
+
RowBasedGrouperHelper.createResultRowBasedColumnSelectorFactory(subquery,
rowSupplier),
+ query.getVirtualColumns()
+ );
Review comment:
```suggestion
final ColumnSelectorFactory columnSelectorFactory =
query.getVirtualColumns().wrap(
RowBasedGrouperHelper.createResultRowBasedColumnSelectorFactory(subquery,
rowSupplier)
);
```
would be more consistent with how other methods such as
`createGrouperAccumulatorPair` are doing this
Since `VirtualColumns.wrap` is currently the only 'production' method that
is calling `VirtualizedColumnSelectorFactory` constructor right now, so I think
we should probably change this to use `wrap`
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
