fix: Add a check to ensure limit is not 0 when evaluating query limit…

… and offset (sourcenetwork#706) Relevant issue(s) Resolves sourcenetwork#705 Description This PR solves a situation where setting an offset to the query without setting a limit would yield a single result.
shahzadlone · Aug 2, 2022 · 5ddcb0e · 5ddcb0e
1 parent 0bffb47
commit 5ddcb0e
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 8 deletions.
diff --git a/query/graphql/planner/limit.go b/query/graphql/planner/limit.go
@@ -59,7 +59,7 @@ func (n *hardLimitNode) Value() core.Doc        { return n.plan.Value() }
 
 func (n *hardLimitNode) Next() (bool, error) {
 	// check if we're passed the limit
-	if n.rowIndex-n.offset >= n.limit {
+	if n.limit != 0 && n.rowIndex-n.offset >= n.limit {
 		return false, nil
 	}
 
@@ -82,10 +82,16 @@ func (n *hardLimitNode) Next() (bool, error) {
 func (n *hardLimitNode) Source() planNode { return n.plan }
 
 func (n *hardLimitNode) Explain() (map[string]interface{}, error) {
-	return map[string]interface{}{
+	exp := map[string]interface{}{
 		limitLabel:  n.limit,
 		offsetLabel: n.offset,
-	}, nil
+	}
+
+	if n.limit == 0 {
+		exp[limitLabel] = nil
+	}
+
+	return exp, nil
 }
 
 // limit the results, flagging any records outside the bounds of limit/offset with
@@ -139,7 +145,7 @@ func (n *renderLimitNode) Next() (bool, error) {
 	n.currentValue = n.plan.Value()
 
 	n.rowIndex++
-	if n.rowIndex-n.offset > n.limit || n.rowIndex <= n.offset {
+	if (n.limit != 0 && n.rowIndex-n.offset > n.limit) || n.rowIndex <= n.offset {
 		n.currentValue.Hidden = true
 	}
 	return true, nil
@@ -148,8 +154,14 @@ func (n *renderLimitNode) Next() (bool, error) {
 func (n *renderLimitNode) Source() planNode { return n.plan }
 
 func (n *renderLimitNode) Explain() (map[string]interface{}, error) {
-	return map[string]interface{}{
+	exp := map[string]interface{}{
 		limitLabel:  n.limit,
 		offsetLabel: n.offset,
-	}, nil
+	}
+
+	if n.limit == 0 {
+		exp[limitLabel] = nil
+	}
+
+	return exp, nil
 }
diff --git a/tests/integration/query/explain/with_hard_limit_test.go b/tests/integration/query/explain/with_hard_limit_test.go
@@ -138,7 +138,7 @@ func TestExplainQueryWithOnlyOffsetSpecified(t *testing.T) {
 				"explain": dataMap{
 					"selectTopNode": dataMap{
 						"hardLimitNode": dataMap{
-							"limit":  int64(0),
+							"limit":  nil,
 							"offset": int64(2),
 							"selectNode": dataMap{
 								"filter": nil,
@@ -528,7 +528,7 @@ func TestExplainQueryWithOnlyOffsetOnChild(t *testing.T) {
 								"subType": dataMap{
 									"selectTopNode": dataMap{
 										"hardLimitNode": dataMap{
-											"limit":  int64(0),
+											"limit":  nil,
 											"offset": int64(2),
 											"selectNode": dataMap{
 												"filter": nil,

diff --git a/tests/integration/query/simple/with_limit_offset_test.go b/tests/integration/query/simple/with_limit_offset_test.go
@@ -165,3 +165,86 @@ func TestQuerySimpleWithLimitAndOffset(t *testing.T) {
 		executeTestCase(t, test)
 	}
 }
+
+func TestQuerySimpleWithOffset(t *testing.T) {
+	tests := []testUtils.QueryTestCase{
+		{
+			Description: "Simple query with offset only",
+			Query: `query {
+						users(offset: 1) {
+							Name
+							Age
+						}
+					}`,
+			Docs: map[int][]string{
+				0: {
+					`{
+						"Name": "John",
+						"Age": 21
+					}`,
+					`{
+						"Name": "Bob",
+						"Age": 32
+					}`,
+				},
+			},
+			Results: []map[string]interface{}{
+				{
+					"Name": "John",
+					"Age":  uint64(21),
+				},
+			},
+		},
+		{
+			Description: "Simple query with offset only, more rows",
+			Query: `query {
+						users(offset: 2) {
+							Name
+							Age
+						}
+					}`,
+			Docs: map[int][]string{
+				0: {
+					`{
+						"Name": "John",
+						"Age": 21
+					}`,
+					`{
+						"Name": "Bob",
+						"Age": 32
+					}`,
+					`{
+						"Name": "Carlo",
+						"Age": 55
+					}`,
+					`{
+						"Name": "Alice",
+						"Age": 19
+					}`,
+					`{
+						"Name": "Melynda",
+						"Age": 30
+					}`,
+				},
+			},
+			Results: []map[string]interface{}{
+				{
+					"Name": "Alice",
+					"Age":  uint64(19),
+				},
+				{
+					"Name": "John",
+					"Age":  uint64(21),
+				},
+				{
+					"Name": "Carlo",
+					"Age":  uint64(55),
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		executeTestCase(t, test)
+	}
+}