timshannon / bolthold

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

Get vs FindOne #126

Closed voroskoi closed 3 years ago

voroskoi commented 3 years ago

Hi,

I would like to thank You for bolthold, it is really amazing.

However I have got an issue, but I can not find out what am I doing wrong. I have a piece of code where I use the following line:

store.FindOne(&res, bolthold.Where(bolthold.Key).Eq(SLEID))

which works fine. I have thought that replacing it with:

store.Get(SLEID, &res)

would also work, but surprisingly it does not ("No data found for this key").

I have also tried that if res.SLEID == SLEID gives true with FindOne, so SLEID does exists in the DB.

The data structure is this:

type ShiftLogEntry struct {
    SLEID   uuid.UUID `boltholdKey:"SLEID"`
    Parent  uint64    `boltholdIndex:"Parent"`
    Area    string
    Body    string
    Ticket  []uint64
    Comment []uuid.UUID
}

What am I doing wrong here?

timshannon commented 3 years ago

That is interesting, and it looks like a bug. What UUID package are you using, and what encoding? My guess is that with Get it's doing a strict byte comparison, where as Find attempts to make a comparison and may end up doing a UUID.String() comparison.

voroskoi commented 3 years ago

Hi,

The UUID package is "github.com/google/uuid". I will try adding a Comparer to see if that helps.

Thanks,

timshannon commented 3 years ago

Yeah that would work as a workaround, but I think I'm going to change the behavior to ensure a byte compare happens when using the bolthold.Key field.

timshannon commented 3 years ago

Sorry it's taken so long, but I finally found some time to look at this, and I'm having trouble replicating the issue.

Here's my attempt a repro which passes fine. I expected it to fail

func Test126FindOneVSGetOne(t *testing.T) {
        testWrap(t, func(store *bolthold.Store, t *testing.T) {
                type Test struct {
                        UUID uuid.UUID
                }

                id := uuid.New()

                store.Upsert(id, Test{UUID: id})

                result := Test{}
                ok(t, store.FindOne(&result, bolthold.Where(bolthold.Key).Eq(id)))
                equals(t, result.UUID, id)

                result = Test{}
                ok(t, store.Get(id, &result))
                equals(t, result.UUID, id)

        })
}

Could you show me how you are inserting the data and generating the UUID?

voroskoi commented 3 years ago

Hi,

Thanks for Your time! I went on with using string instead of uuid.UUID as a workaround, but I am still curious what is going on here.

Your test works indeed, I have also tried to write my own reproducer so I can avoid the walk of shame comes with showing my real code, but I guess I can not help it.

These functions are for adding and getting a ShiftLogEntry: https://git.sr.ht/~voroskoi/nRG/tree/ce0c37e08ab5f2ba6b62aa4c16cada480762e546/item/internal/db/sle.go

And these are the handler functions: https://git.sr.ht/~voroskoi/nRG/tree/ce0c37e08ab5f2ba6b62aa4c16cada480762e546/item/internal/ui/sle.go

The error occurs on line 71. If I use the commented out line in SLEGet function it works, when I try using Get directly it does not.

Funny thing that after adding new entries getting them works for a while, but I have items that used to work, but now they do not. I have spent couple of hours to find out what happens, but I can not even reproduce it :-(

timshannon commented 3 years ago

So I may be wrong, as I've only done quick look through of your code, but I believe the issue is that you are defining your Key as a string here: https://git.sr.ht/~voroskoi/nRG/tree/master/item/models/shiftlog.go#L17

but trying to get it with a UUID type here: https://git.sr.ht/~voroskoi/nRG/tree/ce0c37e08ab5f2ba6b62aa4c16cada480762e546/item/internal/db/sle.go#L30

So the Find query just happens to work in this scenario because it's changing the UUID type to string because it can't find any other way to compare these two different types, and it works because you are actually storing the UUID as a string.

voroskoi commented 3 years ago

No, it is not that. On Mar 18 I was at this state: https://git.sr.ht/~voroskoi/nRG/tree/ce0c37e08ab5f2ba6b62aa4c16cada480762e546/item/models/shiftlog.go#L25

So You have mixed git HEAD with ce0c37e08ab5f2ba6b62aa4c16cada480762e546, I have switched to string with the next commit as I was unable to figure out what is going on.

timshannon commented 3 years ago

So once again, I can't replicate this issue myself. If you can get me a stand alone piece of code to replicate this, then I'll have something to fix, otherwise I don't see any issues here.

voroskoi commented 3 years ago

I have written a minimal version of my program with inserting to the db and retrieving values via a http.Hander, but I was unable to reproduce this problem.

I think there are some other issue with the original code, which I can not reproduce and probably not related to bolthold.