Repository: textileio/go-threads (go)

Issue:
limit and skip operation is applied before conditions
Given a set of 20 instances with a schema of :

```json
{
	"properties": {
		"_id": {
			"type": "string"
		},
                "index": {
                       "type": "number"
                }
	},
	"type": "object"
}
```
with index property values that range from 1 - 20.

If I do a JSON query on the result set with a query:

```json
{
     "ands": [
         {
              "fieldPath": "index",
              "operation": 0, //Eq
              "value": {
                  "float": 15
              }
         }
     ],
     "limit": 10,
     "skip": 0,
}
```

I expect to get a list of a single result of the instance with a property of `index = 15`.

I actually get an empty list.

The limit and skip operation is applied before the conditions so if my item is not within the limit and skip operation it won't find it.
I would expect that it would search through the whole result set for the item and then apply the limit and skip operation to the result set.

Pull request description:
Track real counts for limit and skip
Applies Limit and Skip _after_ other conditions _and_ read filter have been applied. This leads to potentially slightly less efficient queries (because the child query engine can't handle the limits and skips directly), but provides more consistent and accurate/expected results. A test has been added that captures the issue presented in #458, plus a "regression" test was added to ensure we don't over-compensate.

Fixes #458.

Technically this is a bug fix, but it also changes how results are returned when applying limit and skip, so this might need to come with an appropriate version bump?

Patch:
diff --git a/db/index.go b/db/index.go
index 3c23cab..6adb8e8 100644
--- a/db/index.go
+++ b/db/index.go
@@ -290,8 +290,10 @@ func newIterator(txn dse.TxnExt, baseKey ds.Key, q *Query) *iterator {
 	dsq := dse.QueryExt{
 		Query: query.Query{
 			Prefix: prefix.String(),
-			Limit:  q.Limit,
-			Offset: q.Skip,
+			// Pull out Skip and Limit here because we won't know ahead of time
+			// (due to readFilters) how many to skip/limit
+			// Limit:  q.Limit,
+			// Offset: q.Skip,
 		},
 	}
 	if q.Sort.FieldPath == idFieldName {
diff --git a/db/query.go b/db/query.go
index 28f030e..edb03c5 100644
--- a/db/query.go
+++ b/db/query.go
@@ -314,6 +314,9 @@ func (t *Txn) Find(q *Query) ([][]byte, error) {
 		return nil, err
 	}
 	var values []MarshaledResult
+	// Use count to track real count of returned values taking into account
+	// read filter and any indexes etc in the query
+	var count = 0
 	for {
 		res, ok := iter.NextSync()
 		if !ok {
@@ -323,9 +326,13 @@ func (t *Txn) Find(q *Query) ([][]byte, error) {
 		if err != nil {
 			return nil, err
 		}
-		if res.Value != nil {
+		count++
+		if res.Value != nil && count > q.Skip {
 			values = append(values, res)
 		}
+		if count == q.Limit {
+			break
+		}
 	}
 
 	if q.Sort.FieldPath != "" && q.Sort.FieldPath != idFieldName {


Test patch:
diff --git a/db/query_more_test.go b/db/query_more_test.go
index c72f18a..77e2d91 100644
--- a/db/query_more_test.go
+++ b/db/query_more_test.go
@@ -89,6 +89,9 @@ var (
 
 		{name: "SortAllAscFloat", query: OrderBy("Meta.Rating"), resIdx: []int{0, 1, 2, 3, 4}, ordered: true},
 		{name: "SortAllDescFloat", query: OrderByDesc("Meta.Rating"), resIdx: []int{4, 3, 2, 1, 0}, ordered: true},
+
+		{name: "LimitTotalReadsOutside", query: Where("Meta.TotalReads").Gt(float64(100)).LimitTo(2), resIdx: []int{3, 4}},
+		{name: "LimitTotalReadsInside", query: Where("Meta.TotalReads").Lt(float64(100)).LimitTo(2), resIdx: []int{0, 1}},
 	}
 )