gianm commented on code in PR #17168:
URL: https://github.com/apache/druid/pull/17168#discussion_r1777451722
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java:
##########
@@ -305,7 +313,12 @@ protected ReturnOrAwait<Unit> runWithInputChannel(
}
final CursorHolder nextCursorHolder =
-
cursorFactory.makeCursorHolder(ScanQueryEngine.makeCursorBuildSpec(query,
null));
+ cursorFactory.makeCursorHolder(
+ ScanQueryEngine.makeCursorBuildSpec(
+ query.withQuerySegmentSpec(new
MultipleIntervalSegmentSpec(Intervals.ONLY_ETERNITY)),
Review Comment:
The old code did this too, but, it seems like it'd be incorrect if the
`query` had an `intervals` filter on the `__time` column. I am not sure how (or
if) this can actually happen, but I wonder if it could be made to happen by
doing a query like this:
```sql
SELECT * FROM (SELECT * FROM "wikipedia" ORDER BY countryName LIMIT 100)
WHERE TIME_IN_INTERVAL(__time, '2020/P1D') OR TIME_IN_INTERVAL(__time,
'2021/P1D')
```
I tried doing that just now, and got a `QueryNotSupported` error, which
seems like a different problem. But what if it was supported?
IMO, here we should add a validation here to check that `query` has an
eternity interval, rather than overriding it to be eternity. If it was anything
other than eternity, this wouldn't be correct.
--
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]
