Akshat-Jain commented on code in PR #16854:
URL: https://github.com/apache/druid/pull/16854#discussion_r1715548427
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryKit.java:
##########
Review Comment:
@LakshSingla
1. Fair enough. I added them for debuggability purposes. Since it's not
anything excessive, I was thinking we can tone these down once we've had a bit
of soak time for these flows (as major changes are going into these code flows
currently).
2. Fair enough. There's a pending refactoring work item as well. I wanted to
add exhaustive tests along with that. Can also add exhaustive documentation
with that. Would certainly want to take it up separate from this PR though, if
that works.
3. Fair enough. Similar as point (2).
4. Fair enough. Similar as point (2).
1. Yes we can have multiple sort/partition operators to the input. Their
ordering is done in `Windowing.java` file, and MSQ receives them properly
ordered. We can potentially improve in that regards (wrt further
optimisations), but that's outside the scope of the MSQ specific files. Hope
that answers.
2. Yeah it's correct. If we don't have any partitioning requirements,
then we don't need to do any shuffling, and hence any sorting if needed can
happen via NaiveSortOperatorFactory itself during processing.
3. "What if a partition isn't present in the sort columns." --- can you
please elaborate what you mean by that?
--
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]
