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

feat: add support for query operators #2

Merged
merged 2 commits into from
Apr 9, 2020

Conversation

vasa-develop
Copy link
Member

@vasa-develop vasa-develop commented Apr 9, 2020

Supported Conditional Operators

  • $lt
  • $gt
  • $lte
  • $gte

Supported Logical Operators

  • $and
  • $or

Observations

  1. The benchmark seems to have taken a big hit.
461 queries per second, 17695 queries in 40 seconds (Oplog: 5000)
488 queries per second, 18183 queries in 41 seconds (Oplog: 5000)
562 queries per second, 18745 queries in 42 seconds (Oplog: 5000)
652 queries per second, 19397 queries in 43 seconds (Oplog: 5000)
578 queries per second, 19975 queries in 44 seconds (Oplog: 5000)
611 queries per second, 20586 queries in 45 seconds (Oplog: 5000)
541 queries per second, 21127 queries in 46 seconds (Oplog: 5000)
457 queries per second, 21584 queries in 47 seconds (Oplog: 5000)
544 queries per second, 22128 queries in 48 seconds (Oplog: 5000)
561 queries per second, 22689 queries in 49 seconds (Oplog: 5000)
--> Average of 535 q/s in the last 10 seconds
  1. The benchmarks have the same results, irrespective of how complex is the query.

License: MIT
Signed-off-by: Vaibhav Saini vasa@dappkit.io

License: MIT
Signed-off-by: Vaibhav Saini <vasa@dappkit.io>
@vasa-develop
Copy link
Member Author

vasa-develop commented Apr 9, 2020

@vaultec81 I will try to optimize the query algorithm in the future releases. Let me know if you can suggest some optimizations in QueryOperators.js

Also, the implementation is a bit messy. I will clean it up going forward.

Comment on lines +13 to +14
let res = parseAndFind(query, this._index, false)
return res
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible issue here. How does this affect when we want to use an on disk index or other optimized index?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of that. How should this be written? Should I add another param for supplying a custom index?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure at the moment. We don't have any formal API for index/store. I think for now its alright.

@vasa-develop
Copy link
Member Author

vasa-develop commented Apr 9, 2020

I got a few ideas to make this efficient: Short-circuit evaluation

UPDATE: Applied all optimizations that I could think of. The performance improved for some specific conditions.
But for worst cases (in which it has to loop through all the query conditions), we can't do much better.

use short-circuit evaluation

License: MIT
Signed-off-by: Vaibhav Saini <vasa@dappkit.io>
@vasa-develop vasa-develop merged commit f915b8b into aviondb:master Apr 9, 2020
@vaultec81 vaultec81 mentioned this pull request Apr 11, 2020
10 tasks
@vaultec81 vaultec81 linked an issue Apr 11, 2020 that may be closed by this pull request
10 tasks
@vaultec81 vaultec81 removed a link to an issue Apr 11, 2020
10 tasks
vaultec81 added a commit that referenced this pull request Jun 2, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants