search

timshannon / badgerhold

BadgerHold is an embeddable NoSQL store for querying Go types built on Badger
MIT License
514 stars 52 forks source link

Optimise index queries with `eq` and `in` operators #78

Closed b-tarczynski closed 2 years ago

b-tarczynski commented 2 years ago

Partially resolves https://github.com/timshannon/badgerhold/issues/77 for queries with eq and in operators. It queries badger database directly instead of using iterator.

timshannon commented 2 years ago

Looks great. If this is to fix #77 then write some tests or benchmarks to show that it fixes it.

b-tarczynski commented 2 years ago

Looks great. If this is to fix #77 then write some tests or benchmarks to show that it fixes it.

Yea, I can totally do that. I have question regarding that topic. Why have you decided to write tests as different package name? I am thinking about creating new PR after this, that will change badgerhold_test package to badgerhold as production code. It would make testing private functions easier.

timshannon commented 2 years ago

Because we want to specifically test the exported interface. If there's an private function that isn't getting hit by an external interface, then why does it exist?

Treating the testing package as a client of the library ensures that everything the client does is tested in the way a client would use it. If you test internally you can easily call library functions in ways clients can't and potentially miss client behaviors and effects.

b-tarczynski commented 2 years ago

Fixed BenchmarkFindIndexed and added benchmark that shows performance improvements. For me (MacBook Pro (13-inch, M1, 2020)) benchmarks results looks as follow:

goos: darwin
goarch: arm64

BenchmarkFindIndexedWithManyIndexValues-8           1941        558365 ns/op (master)
BenchmarkFindIndexedWithManyIndexValues-8          18756         61348 ns/op (this branch)
timshannon commented 2 years ago

I really appreciate the PR, and the benchmarks.

Thanks,

b-tarczynski commented 2 years ago

Thanks for merging. @timshannon Is it possible to create new Github release with these changes? According to existing tags it would be v4.0.2.

timshannon commented 2 years ago

Creating a release shouldn't be necessary, I have yet to create any releases for this project, unless I'm misunderstanding what you need one for.

Go should just need the tags, which I can add for you if you like.

b-tarczynski commented 2 years ago

Yeah, I mean just tag. Sorry.

timshannon commented 2 years ago

All good, I just wanted to make sure I understood the request.

The new tag should be out there now.

Thanks,