Skip to content

Commit

Permalink
view: Fix panic if offset is longer than result set
Browse files Browse the repository at this point in the history
Specifically in the case that a query

- Has ORDER BY, LIMIT, and OFFSET
- Has aggregates with a group by
- *doesn't* filter by a primary or unique column

If the OFFSET is longer than the length of the result set, we'd panic
due to a bounds checking bug in `Vec::drain` inside the ResultIterator.
This fixes that by just clearing the result set entirely if the offset
is >= the length of the result set.

Fixes: ENG-2970
Change-Id: Iadadbe4ee986f0a991745814a927813a946dd9ac
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/4805
Tested-by: Buildkite CI
Reviewed-by: Fran Noriega <fran@readyset.io>
  • Loading branch information
glittershark committed May 3, 2023
1 parent ec8ccf0 commit 5d9c5ce
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
13 changes: 13 additions & 0 deletions logictests/pagination.test
Expand Up @@ -4,6 +4,7 @@ create table t1 (grp int, val int);
statement ok
insert into t1 (grp, val) values (1, 1), (1, 2), (1, 3), (1, 4), (1, 5), (1, 6);

onlyif readyset
statement ok
create cache from
select val from t1 where grp = ? order by val asc limit 3 offset ?;
Expand Down Expand Up @@ -43,6 +44,7 @@ select val from t1 where grp = ? order by val asc limit ? offset ?;
statement ok
create table t2 (grp2 int, val int);

onlyif readyset
statement ok
create cache from
select t1.val
Expand Down Expand Up @@ -89,3 +91,14 @@ offset ?
1
2
3

query II nosort
select max(val), grp
from t1
where grp >= ?
group by grp
order by grp asc
limit 10
offset 1234
? = 1
----
6 changes: 5 additions & 1 deletion readyset-client/src/view/results.rs
Expand Up @@ -262,7 +262,11 @@ impl ResultIterator {
});

if let Some(offset) = offset {
results.drain(offset..);
if offset >= results.len() {
results.clear();
} else {
results.drain(offset..);
}
}

return ResultIterator::owned(vec![Results {
Expand Down

0 comments on commit 5d9c5ce

Please sign in to comment.