Nightly upstream merge - #84
Conversation
Push down ORDER BY __time ... LIMIT to Druid. Druid only supports ordering by __time for non-aggregating queries, so any other sort key is still handled by Trino. __time is never null in Druid, so the null ordering requested by Trino can be ignored and the pushdown is guaranteed. Pushdown is only applied on a plain table scan, not on a handle that already carries a limit or sort order, so the ORDER BY never ends up on a subquery where Druid does not honor it.
bill-ph
left a comment
bill-ph
left a comment
There was a problem hiding this comment.
Reviewed the PR description, diff, existing discussion, JDBC Top-N application and SQL construction, Druid connector tests, and Druid's scan ordering limits. No overengineering or scope creep concerns.
P1
- Time-ordered scan pushdown can make large Top-N queries fail on Druid. Druid permits scan time ordering only when the requested limit is below
druid.query.scan.maxRowsQueuedForOrdering(default 100,000) or the scanned segments stay belowdruid.query.scan.maxSegmentPartitionsOrderedInMemory(default 50). A query such asORDER BY __time DESC LIMIT 100001over enough partitions is now sent to Druid and fails, whereas Trino previously performed the sort. See Druid scan query documentation.
— Robo Bill v2 (gpt-6-sol, high reasoning)
| } | ||
|
|
||
| @Override | ||
| public boolean supportsTopN(ConnectorSession session, JdbcTableHandle handle, List<JdbcSortItem> sortOrder) |
There was a problem hiding this comment.
P1: This accepts any Top-N limit, but Druid's time-ordered scan has resource limits: the requested limit must be below maxRowsQueuedForOrdering (default 100,000) or the scan must involve fewer than maxSegmentPartitionsOrderedInMemory partitions (default 50). With a larger limit on a sufficiently partitioned datasource, a query that Trino previously sorted locally now fails in Druid. Please gate this pushdown on a safe limit or provide a fallback for those scans. See Druid scan ordering.
3 participants
Merge the latest
trinodb/trinomaster into the PostHog fork.Upstream changes:
__timeto the Druid connector.Local validation:
JAVA_HOME=/usr/lib/jvm/jdk-25.0.4.1+1 ./mvnw install -pl plugin/trino-ducklake -am -DskipTests -Dair.check.skip-all=true