Support limit pushdown in Loki connector. #25876
Conversation
| @@ -51,7 +51,7 @@ | |||
| <dependency> | |||
| <groupId>io.github.jeschkies</groupId> | |||
| <artifactId>loki-client</artifactId> | |||
| <version>0.0.4</version> | |||
| <version>0.0.5</version> | |||
There was a problem hiding this comment.
How does this bump relate to LIMIT pushdown?
There was a problem hiding this comment.
The client did not support passing the limit as URL parameter. We need 0.0.5 to pass it.
| { | ||
| LokiTableHandle lokiTableHandle = (LokiTableHandle) tableHandle; | ||
|
|
||
| // TODO: return Optional.empty() if query is a metric query |
There was a problem hiding this comment.
When do you handle this TODO comment? Follow-up or this PR?
| )) | ||
| LIMIT 1 | ||
| """, | ||
| SELECT to_iso8601(timestamp), value FROM |
| @@ -81,6 +82,42 @@ void testLogsQuery() | |||
| "VALUES ('line 1')"); | |||
| } | |||
|
|
|||
| @Test | |||
| void testLimitedLogsQuery() | |||
There was a problem hiding this comment.
Please verify LIMIT pushdown is exactly happened.
There was a problem hiding this comment.
I'm not sure this is possible other than inferring that it was passed down. We'd have to grab the Loki logs or mock the client. How do you verify it in other database connectors?
There was a problem hiding this comment.
You can refer to tests using SUPPORTS_LIMIT_PUSHDOWN.
ebyhr
added
the
needs-docs
https://trino.io/development/process.html#pull-request-and-commit-guidelines-
|
| @@ -49,7 +53,8 @@ public LokiRecordSet(LokiClient lokiClient, LokiSplit split, List<LokiColumnHand | |||
|
|
|||
| // Execute the query | |||
| try { | |||
| this.result = lokiClient.rangeQuery(split.query(), split.start(), split.end(), split.step()); | |||
| log.info("querying %s with limit %d", split.query(), split.limit()); | |||
There was a problem hiding this comment.
Change to debug level and move before try.
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
Description
This bumps the Loki client version to 0.0.5 and adds support for limit pushdown in the Loki connector for log queries.
Additional context and related issues
Closes #25875
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: