gianm commented on a change in pull request #10235:
URL: https://github.com/apache/druid/pull/10235#discussion_r465960675
##########
File path: docs/querying/limitspec.md
##########
@@ -35,11 +35,23 @@ The default limit spec takes a limit and the list of
columns to do an orderBy op
```json
{
"type" : "default",
- "limit" : <integer_value>,
- "columns" : [list of OrderByColumnSpec],
+ "limit" : <optional integer>,
+ "offset" : <optional integer>,
+ "columns" : [<optional list of OrderByColumnSpec>],
}
```
+The "limit" parameter is the maximum number of rows to return.
+
+The "offset" parameter tells Druid to skip this many rows when returning
results. If both "limit" and "offset" are
+provided, then "offset" will be applied first, followed by "limit". For
example, a spec with limit 100 and offset 10
+will return 100 rows starting from row number 10. Skipped rows will still need
to be generated internally and then
+discarded, meaning that raising offsets to high values can cause queries to
use additional resources.
Review comment:
It would need to be based on the limit, I suppose. I'll add this
language about how to think about this:
> Internally, the query is executed by extending the limit by the offset and
then discarding a number of rows equal to the offset. This means that raising
the offset will increase resource usage by an amount similar to increasing the
limit.
A full picture would require some docs about how to think of the impact of
limits on resource usage, which would be awesome but I would consider outside
the scope of this patch.
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java
##########
@@ -928,6 +928,7 @@ public GroupByQuery toGroupByQuery()
sorting != null
? new DefaultLimitSpec(
sorting.getOrderBys(),
+ 0,
Review comment:
Yes.
##########
File path:
processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java
##########
@@ -3700,7 +3702,7 @@ public void testGroupByWithOrderOnHyperUnique()
query,
"1970-01-01T00:00:00.000Z",
"market",
- "upfront",
+ "total_market",
Review comment:
Yes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
