timshannon / bolthold

BoltHold is an embeddable NoSQL store for Go types built on BoltDB
MIT License
648 stars 46 forks source link

seekCursor bugfix: criterion value might not be in database #66

Closed ivard closed 5 years ago

ivard commented 5 years ago

We found a bug when using the following query on our bolthold database and the value was not in the list of keys: bolthold.Where(bolthold.Key).Gt(value).Limit(max)

When this is the case and > or the >= operator is used, the current code of seekCursus will seek the element in the database. I think this is an search optimization, since all keys are sorted. However, when the particular value is not in the database, no value is going to be found (logically of course). This results in the situation that no results are returned. However, there can still be items in the database that are greater than the given value. Now these results are not returned too.

My solution is to now always fall back on the First() function when the element cannot be found. This is of course a bit less efficient, but it solves the problem. If you wish to solve the problem more efficiently, we'd also be very happy. Then this pull request can be seen as an issue report.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.02%) to 86.974% when pulling 406f142f232b5687f9af5b18162cfa3022f03ad0 on ivard:master into b73eaf0ecf37450c47a16d16e036945901ecd9fc on timshannon:master.

ivard commented 5 years ago

I'm sorry, as I now notice this approach still has some bugs. I'm going to deep into the problem further or use a work-around.

timshannon commented 5 years ago

Can you write up a quick test to replicate the issue, then I can look at it as well.

Thanks,

ivard commented 5 years ago

Of course, I made a little example for you. In my view the result should be [{Test3} {Test2}], however the result is []. I have not the idea that I do something really strange actually. It looks like it is related to the negative numbers.

package main

import (
    "fmt"
    "github.com/timshannon/bolthold"
    "go.etcd.io/bbolt"
    "encoding/json"
    "time"
)

type Test struct {
    TestString string
}

func main() {
    db, err := bolthold.Open("/tmp/database", 0600, &bolthold.Options{
        Options: &bbolt.Options{Timeout: 1 * time.Second},
        Encoder: json.Marshal,
        Decoder: json.Unmarshal,
    })

    if err != nil {
        return
    }

    db.Upsert(-5, Test{TestString: "Test"})
    db.Upsert(-3, Test{TestString: "Test2"})
    db.Upsert(-1, Test{TestString: "Test3"})

    var result []Test
    db.Find(&result,
        bolthold.Where(bolthold.Key).Gt(-4).Limit(3),
    )

    fmt.Println(result)
}
timshannon commented 5 years ago

Thanks for the reproducible test. The SeekCursor func was definitely the issue. It basically assumed the values would be positive. I simply removed the func in pr #70