Skip to content

Commit 81145f2

Browse files
authored
fix(stdlib): error if using limit with negative offset (#5544)
A negative offset does not make sense with a limit function. Detect if a negative offset is being used and produce an error, rather than waiting for arrow to panic.
1 parent 0c5580e commit 81145f2

File tree

2 files changed

+97
-0
lines changed

2 files changed

+97
-0
lines changed

stdlib/universe/limit.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ func createLimitOpSpec(args flux.Arguments, a *flux.Administration) (flux.Operat
4949
} else if ok {
5050
spec.Offset = offset
5151
}
52+
if spec.Offset < 0 {
53+
return nil, errors.Newf(codes.Invalid, "limit offset cannot be negative (%d)", spec.Offset)
54+
}
5255

5356
return spec, nil
5457
}

stdlib/universe/limit_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"sort"
66
"testing"
7+
"time"
78

89
"github.com/google/go-cmp/cmp"
910
"github.com/influxdata/flux"
@@ -13,11 +14,104 @@ import (
1314
"github.com/influxdata/flux/execute/executetest"
1415
"github.com/influxdata/flux/execute/table"
1516
"github.com/influxdata/flux/internal/gen"
17+
"github.com/influxdata/flux/internal/operation"
1618
"github.com/influxdata/flux/memory"
19+
"github.com/influxdata/flux/querytest"
20+
"github.com/influxdata/flux/stdlib/influxdata/influxdb"
1721
"github.com/influxdata/flux/stdlib/universe"
1822
"github.com/influxdata/flux/values"
1923
)
2024

25+
func TestLimit_NewQuery(t *testing.T) {
26+
tests := []querytest.NewQueryTestCase{
27+
{
28+
Name: "no offset",
29+
Raw: `
30+
from(bucket:"db") |> range(start:-1h) |> limit(n: 1)`,
31+
Want: &operation.Spec{
32+
Operations: []*operation.Node{
33+
{
34+
ID: "from0",
35+
Spec: &influxdb.FromOpSpec{Bucket: influxdb.NameOrID{Name: "db"}},
36+
},
37+
{
38+
ID: "range1",
39+
Spec: &universe.RangeOpSpec{
40+
Start: flux.Time{
41+
Relative: -1 * time.Hour,
42+
IsRelative: true,
43+
},
44+
Stop: flux.Time{
45+
IsRelative: true,
46+
},
47+
TimeColumn: "_time",
48+
StartColumn: "_start",
49+
StopColumn: "_stop",
50+
},
51+
},
52+
{
53+
ID: "limit2",
54+
Spec: &universe.LimitOpSpec{N: 1},
55+
},
56+
},
57+
Edges: []operation.Edge{
58+
{Parent: "from0", Child: "range1"},
59+
{Parent: "range1", Child: "limit2"},
60+
},
61+
},
62+
},
63+
{
64+
Name: "positive offset",
65+
Raw: `
66+
from(bucket:"db") |> range(start:-1h) |> limit(n: 1, offset: 2)`,
67+
Want: &operation.Spec{
68+
Operations: []*operation.Node{
69+
{
70+
ID: "from0",
71+
Spec: &influxdb.FromOpSpec{Bucket: influxdb.NameOrID{Name: "db"}},
72+
},
73+
{
74+
ID: "range1",
75+
Spec: &universe.RangeOpSpec{
76+
Start: flux.Time{
77+
Relative: -1 * time.Hour,
78+
IsRelative: true,
79+
},
80+
Stop: flux.Time{
81+
IsRelative: true,
82+
},
83+
TimeColumn: "_time",
84+
StartColumn: "_start",
85+
StopColumn: "_stop",
86+
},
87+
},
88+
{
89+
ID: "limit2",
90+
Spec: &universe.LimitOpSpec{N: 1, Offset: 2},
91+
},
92+
},
93+
Edges: []operation.Edge{
94+
{Parent: "from0", Child: "range1"},
95+
{Parent: "range1", Child: "limit2"},
96+
},
97+
},
98+
},
99+
{
100+
Name: "negative offset",
101+
Raw: `
102+
from(bucket:"db") |> range(start:-1h) |> limit(n: 1, offset: -1)`,
103+
WantErr: true,
104+
},
105+
}
106+
for _, tc := range tests {
107+
tc := tc
108+
t.Run(tc.Name, func(t *testing.T) {
109+
t.Parallel()
110+
querytest.NewQueryTestHelper(t, tc)
111+
})
112+
}
113+
}
114+
21115
func TestLimit_Process(t *testing.T) {
22116
testCases := []struct {
23117
name string

0 commit comments

Comments
 (0)