LakshSingla commented on code in PR #16854:
URL: https://github.com/apache/druid/pull/16854#discussion_r1714837684
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessor.java:
##########
@@ -119,6 +126,21 @@ public WindowOperatorQueryFrameProcessor(
for (int i = 0; i < frameReader.signature().size(); i++) {
typeStrategies[i] =
frameReader.signature().getColumnType(i).get().getNullableStrategy();
}
+
+ // Get virtual columns to be added to the frame writer.
+ this.partitionBoostVirtualColumn = new
SettableLongVirtualColumn(QueryKitUtils.PARTITION_BOOST_COLUMN);
+ final List<VirtualColumn> frameWriterVirtualColumns = new ArrayList<>();
+ if (addVirtualColumns) {
+ frameWriterVirtualColumns.add(partitionBoostVirtualColumn);
+
+ final VirtualColumn segmentGranularityVirtualColumn =
+ QueryKitUtils.makeSegmentGranularityVirtualColumn(jsonMapper, query);
+
+ if (segmentGranularityVirtualColumn != null) {
+ frameWriterVirtualColumns.add(segmentGranularityVirtualColumn);
Review Comment:
Likewise, this needn't be gated behind `addVirtualColumns` flag. Last window
would have set the signature correctly (which is why it is getting used), and
the other stages wouldn't have used it anyways (otherwise you'd have seen the
error earlier).
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessor.java:
##########
@@ -99,7 +105,8 @@ public WindowOperatorQueryFrameProcessor(
final List<OperatorFactory> operatorFactoryList,
final RowSignature rowSignature,
final int maxRowsMaterializedInWindow,
- final List<String> partitionColumnNames
+ final List<String> partitionColumnNames,
+ final boolean addVirtualColumns
Review Comment:
Can revert this change. LMK if you face any error - correctness or otherwise
while running the query after making the suggested changes.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessorFactory.java:
##########
Review Comment:
We can revert the changes in this file since we don't wanna rely on a flag.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessor.java:
##########
@@ -119,6 +126,21 @@ public WindowOperatorQueryFrameProcessor(
for (int i = 0; i < frameReader.signature().size(); i++) {
typeStrategies[i] =
frameReader.signature().getColumnType(i).get().getNullableStrategy();
}
+
+ // Get virtual columns to be added to the frame writer.
+ this.partitionBoostVirtualColumn = new
SettableLongVirtualColumn(QueryKitUtils.PARTITION_BOOST_COLUMN);
+ final List<VirtualColumn> frameWriterVirtualColumns = new ArrayList<>();
+ if (addVirtualColumns) {
+ frameWriterVirtualColumns.add(partitionBoostVirtualColumn);
Review Comment:
Boosting isn't done since the query kit isn't setting the boost column. The
boost column should be explicitly added in the window query kit for this line
to take effect. We can take it up in a separate PR.
--
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]
