Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SliceIndex query ContainsAll not working #137

Closed
mmorrison57 opened this issue Nov 28, 2023 · 6 comments
Closed

SliceIndex query ContainsAll not working #137

mmorrison57 opened this issue Nov 28, 2023 · 6 comments
Assignees

Comments

@mmorrison57
Copy link

mmorrison57 commented Nov 28, 2023

Hello,

Recently I found an issue with querying using the ContainsAll criteria.

Using the unit tests provided in index_test.go I was able to confirm this by adjusting theTestSliceIndexWithPointers to the following.

I updated the tests in 2 ways, first to not use Categories []*string but instead to Categories []string instead. Secondly, I updated the store.Find ContainsAll call to include both {cat1, cat2} (multiple items to search for, not just one). The test looks as follows and fails when it should succeed:

func TestSliceIndexWithPointers(t *testing.T) {

	type Event struct {
		Id         uint64
		Type       string    boltholdIndex:"Type"
		Categories []string boltholdSliceIndex:"Categories"
	}

	testWrap(t, func(store *bh.Store, t *testing.T) {
		cat1 := "Cat 1"
		cat2 := "Cat 2"
		cat3 := "Cat 3"

		e1 := &Event{Id: 1, Type: "Type1", Categories: []string{cat1, cat2}}
		e2 := &Event{Id: 2, Type: "Type1", Categories: []string{cat3}}

		ok(t, store.Insert(e1.Id, e1))
		ok(t, store.Insert(e2.Id, e2))

		var es []*Event
		query := bh.Where("Categories").ContainsAll(cat1, cat2).Index("Categories")
		ok(t, store.Find(&es, query))
		equals(t, 1,len(es))
	})
}

The test expects the variable in e1 to be returned, but it is not found when run with an index.

Remove the .Index("Categories") qualifier and it succeeds.

func TestSliceIndexWithPointers(t *testing.T) {

	type Event struct {
		Id         uint64
		Type       string    boltholdIndex:"Type"
		Categories []string boltholdSliceIndex:"Categories"
	}

	testWrap(t, func(store *bh.Store, t *testing.T) {
		cat1 := "Cat 1"
		cat2 := "Cat 2"
		cat3 := "Cat 3"

		e1 := &Event{Id: 1, Type: "Type1", Categories: []string{cat1, cat2}}
		e2 := &Event{Id: 2, Type: "Type1", Categories: []string{cat3}}

		ok(t, store.Insert(e1.Id, e1))
		ok(t, store.Insert(e2.Id, e2))

		var es []*Event
		query := bh.Where("Categories").ContainsAll(cat1, cat2)
		ok(t, store.Find(&es, query))
		equals(t, 1,len(es))
	})

}
@timshannon timshannon self-assigned this Nov 28, 2023
@timshannon
Copy link
Owner

Thanks for the detailed issue and test repro. I'll take a look.

@mmorrison57
Copy link
Author

Absolutely. Sorry about the formatting issues! You may need to add back in the ` backticks in the event structs.

@timshannon
Copy link
Owner

I edited your comment to fix the formatting, FYI.

@timshannon
Copy link
Owner

Ok, there is definitely a logical inconsistency here with ContainsAll.

The slice indexes store the slice values individually, but ContainsAll needs to compare them as a group.

I just need to figure out the best way to handle it.

The easiest option is to simply ignore the Index request in the query, like I do for "MatchFunc"queries where any value (like those not in the index) can be matched against, but that may not be the most performant.

@mmorrison57
Copy link
Author

Thanks for confirming there is an issue there. I came to the same conclusion, ignore the index request and all is still working. Thank you

timshannon added a commit that referenced this issue Nov 29, 2023
Indexes can't be used efficiently with ContainsAll queries because all
values in the index need to be checked.  Instead we just short circuit
the index lookup like we do for MatchFunc
timshannon added a commit that referenced this issue Nov 29, 2023
@timshannon
Copy link
Owner

Master branch should work for you now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants