Allow indexing on nested fields #718
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks @Moocar , I have been offline for a while but I hope to look this over soon. |
|
Thanks @obeliskos for all your great work on Loki! Excited to have this in Gatsby soon. |
|
Great, thanks @Moocar AND @KyleAMathews! Since this is performance rather than functional related, I will leave option of creating a unit test now or in the future up to you. |
|
@obeliskos thanks! Funny you should mention tests, I just found a bug where nested indexed fields aren't handling the |
pieh
pushed a commit
to gatsbyjs/gatsby
that referenced
this pull request
Nov 29, 2018
This PR adds a feature flag `GATSBY_DB_NODES` that can be used to change the storage engine for the gatsby data layer (`nodes`). - `redux` (default) which uses the existing redux implementation, and `sift` for querying. Or, you can use - `loki` which uses the loki in-memory database to store and query. This PR re-implements functionality in #9338, but with all tests passing and addressing previous feedback. It should also be easier to review since it builds on several refactorings. Some things to note: 1. I [submitted a PR to lokijs](techfort/LokiJS#718) which still hasn't been merged, though the author says he'll start working on it soon. Therefore, in in interim, I've published [@moocar/lokijs](https://www.npmjs.com/package/@moocar/lokijs). 1. I haven't implemented auto indexing of query fields yet. I'll attack that next. 1. I suggest we ask the community to try out the feature flag once it's merged to get feedback. All the tests pass, but this is such a big change that we'll want to test it gradually 1. While loki uses the same mongo query language as sift, they do have different behavior. Most of my time on this PR was spent ensuring that loki behaves **exactly** like sift. See [db/loki/nodes-query.js](https://github.com/gatsbyjs/gatsby/blob/cddbe893a4ce638babb1cbe5e5da4c13b6f5e57d/packages/gatsby/src/db/loki/nodes-query.js). But there's a chance a few edge cases have slipped through the cracks. 1. the feature flag works with the tests too `GATSBY_DB_NODES=loki yarn test`. We should perhaps look into running this on all PRs
gpetrioli
pushed a commit
to gpetrioli/gatsby
that referenced
this pull request
Jan 22, 2019
This PR adds a feature flag `GATSBY_DB_NODES` that can be used to change the storage engine for the gatsby data layer (`nodes`). - `redux` (default) which uses the existing redux implementation, and `sift` for querying. Or, you can use - `loki` which uses the loki in-memory database to store and query. This PR re-implements functionality in gatsbyjs#9338, but with all tests passing and addressing previous feedback. It should also be easier to review since it builds on several refactorings. Some things to note: 1. I [submitted a PR to lokijs](techfort/LokiJS#718) which still hasn't been merged, though the author says he'll start working on it soon. Therefore, in in interim, I've published [@moocar/lokijs](https://www.npmjs.com/package/@moocar/lokijs). 1. I haven't implemented auto indexing of query fields yet. I'll attack that next. 1. I suggest we ask the community to try out the feature flag once it's merged to get feedback. All the tests pass, but this is such a big change that we'll want to test it gradually 1. While loki uses the same mongo query language as sift, they do have different behavior. Most of my time on this PR was spent ensuring that loki behaves **exactly** like sift. See [db/loki/nodes-query.js](https://github.com/gatsbyjs/gatsby/blob/cddbe893a4ce638babb1cbe5e5da4c13b6f5e57d/packages/gatsby/src/db/loki/nodes-query.js). But there's a chance a few edge cases have slipped through the cracks. 1. the feature flag works with the tests too `GATSBY_DB_NODES=loki yarn test`. We should perhaps look into running this on all PRs
|
Is the use of this documented (preferably a walk-through)? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Loki doesn't support indexing on nested fields (period-delimited). This PR adds support for it.
The main change is the addition of the
Utils.getInfunction which performs the deep lookup functionality. Initially, I replaced allobject[path]lookups with this function, but that resulted in a 2x performance penalty due to thestring.split('.')that was required for each lookup. So instead, I added a flagusingDotNotationthat can be set so that the function knows whether it needs to split the path at all.I've been testing it on https://www.gatsbyjs.org/ as a replacement for the in-memory DB and it's working great so far.