adarshsanjeev commented on code in PR #16804: URL: https://github.com/apache/druid/pull/16804#discussion_r1696404767
########## extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryKit.java: ########## @@ -132,17 +132,22 @@ public QueryDefinition makeQueryDefinition( // Update partition by of next window final RowSignature signatureSoFar = signatureBuilder.build(); boolean addShuffle = true; - if (originalQuery.getContext().containsKey(MultiStageQueryContext.NEXT_WINDOW_SHUFFLE_COL)) { - final ClusterBy windowClusterBy = (ClusterBy) originalQuery.getContext() - .get(MultiStageQueryContext.NEXT_WINDOW_SHUFFLE_COL); - for (KeyColumn c : windowClusterBy.getColumns()) { - if (!signatureSoFar.contains(c.columnName())) { - addShuffle = false; - break; + boolean windowHasEmptyOver = false; + if (originalQuery.getContext().containsKey(MultiStageQueryContext.NEXT_WINDOW_SHUFFLE_SPEC)) { + if (originalQuery.getContext().get(MultiStageQueryContext.NEXT_WINDOW_SHUFFLE_SPEC) instanceof MixShuffleSpec) { Review Comment: Ignoring the `resultShuffleSpecFactory` seems to be an issue. I am not familiar with the WindowFunction query kit, but would a window stage never be the second last stage in an ingestion query or an async query? If it is guaranteed to never be the case, ideally we would need way to validate that or have a defensive check. If it is not, then hardcoding the mix shuffle spec seems incorrect. -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org