timshannon / bolthold

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

Query Builder approach #108

Closed nrocco closed 4 years ago

nrocco commented 4 years ago

Hi there, awesome package, first of all :)

I am giving bolthold a try for my next project and I have a question when it comes to query building based on user input.

How does one create a base (empty) query that matches everything that can be used as a starting point for adding where clauses?

I would like a user to be able to get all Items, or to filter them by one or more criteria.

Take the following example:

type Item struct {
  ID       uint64 `boltholdKey:"ID"`
  Archived bool
  Category string
}

query := &bolthold.Query{} // <<< this line here

if userInput.Archived {
  query.And("Archived").Eq(true)
}

if userInput.Category {
  query.And("Category").Eq(userInput.Category)
}

// ... et cetera

query.Limit(userInput.Limit)
query.Skip(userInput.Offset)

bh.Find(&items, query)

The above code does not work and panics with an error assignment to entry in nil map. Which I understand is expected. As a work around I replaced the first line with something that I know will match everything:

// errors with gob: decoding into local type *int, received remote type uint
query := bolthold.Where(bolthold.Key).Gt(0)

// errors with gob: decoding into local type *string, received remote type uint
query := bolthold.Where(bolthold.Key).Ne("")

// panics with reflect: New(nil)
query := bolthold.Where(bolthold.Key).Not().IsNil()

// works
query := bolthold.Where(bolthold.Key).Gt(uint(0))

How would one implement such a pattern?

timshannon commented 4 years ago

A nil query will match everything, but I can see how that makes query building on the fly, like this difficult.

I'll use this issue to make a nil query behave the same as an empty query.

Thanks for opening the issue.

timshannon commented 4 years ago

I am unable to replicate a panic using an already initialized, empty query. Your other panics mentioned make sense though. For example:

// can't compare ints and uints in Go
query := bolthold.Where(bolthold.Key).Gt(0)

// can't compare strings and uints
query := bolthold.Where(bolthold.Key).Ne("")

// uints cannot be nil
query := bolthold.Where(bolthold.Key).Not().IsNil()

Here's my code I used to try to replicate your query builder code, but it all works fine for me:

func Test108QueryBuilding(t *testing.T) {
    type Item struct {
        ID       uint64 `boltholdKey:"ID"`
        Archived bool
        Category string
    }

    testWrap(t, func(store *bolthold.Store, t *testing.T) {
        query := &bolthold.Query{}

        query.Limit(10)
        query.Skip(0)

        var items []Item

        ok(t, store.Find(&items, query))
    })
}

Bolthold already supports doing this, so I'm guessing the panic is elsewhere in your code. Could you give me a real example of the code you're trying to run?

nrocco commented 4 years ago

Try adding a filter to your example to get the panic:

package main

import (
    "log"

    "github.com/timshannon/bolthold"
)

func main() {
    store, err := bolthold.Open("test.db", 0664, nil)

    if err != nil {
        log.Fatal(err)
    }

    type Item struct {
        ID       uint64 `boltholdKey:"ID"`
        Archived bool
        Category string
    }

    query := &bolthold.Query{}

    query.And("Archived").Eq(false)

    query.Limit(10)
    query.Skip(0)

    var items []Item

    store.Find(&items, query)

    log.Printf("Found %d items\n", len(items))
}

This will give you:

$ go run main.go
panic: assignment to entry in nil map

goroutine 1 [running]:
github.com/timshannon/bolthold.(*Criterion).op(...)
    /usr/local/go/pkg/mod/github.com/timshannon/bolthold@v0.0.0-20200420150217-0e8c0be6fd3c/criteria.go:318
github.com/timshannon/bolthold.(*Criterion).Eq(...)
    /usr/local/go/pkg/mod/github.com/timshannon/bolthold@v0.0.0-20200420150217-0e8c0be6fd3c/criteria.go:325
main.main()
    ~/fafa/main.go:24 +0x190
exit status 2
timshannon commented 4 years ago

Thanks. I'm getting the panic now.

timshannon commented 4 years ago

Fixed in #109 Go get the latest and you should be good.

nrocco commented 4 years ago

Pulled the latest version and I can confirm it works now:

$ go run main.go
2020/04/27 22:28:18 Found 0 items

Thanks a lot!