imply-cheddar commented on code in PR #16800:
URL: https://github.com/apache/druid/pull/16800#discussion_r1715658955
##########
processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChest.java:
##########
@@ -116,6 +128,36 @@ public Sequence<Object[]> resultsAsArrays(
return (Sequence) resultSequence;
}
+ @Override
+ public Optional<Sequence<FrameSignaturePair>> resultsAsFrames(
+ WindowOperatorQuery query,
+ Sequence<RowsAndColumns> resultSequence,
+ MemoryAllocatorFactory memoryAllocatorFactory,
+ boolean useNestedForUnknownTypes
+ )
+ {
Review Comment:
Okay, the overall problem here is the lifecycle of the `resultsAsFrames`
method, it's disjointed from the overall lifecycle of the query itself and
there is no way for the query to control it.
The method would have been better done by making it actually wrap the
`QueryRunner` instead of be a transformation of a Sequence to a Sequence. In
that case, the QueryRunner would be able to know that it already returns
frames, include the context parameter to skip the processing and move on.
The other option which requires a lot less code change for now would be to
add a context parameter that is like `"subQuery": true` indicating that the
query is being executed as a subQuery instead of as the full query. I think
that changing the method to wrap the QueryRunner is the "better" resolution,
but I think that the addition of the "subQuery" context parameter is also a
viable and meaningful solution for this.
Generally speaking, all of the logic that is implemented in the
QuerySegmentWalker that's dealing with data ends up problematic as we flesh out
more query capabilities. We generally need to move more and more of that into
Query implementations instead of in the Walker.
--
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]
