Conversation
garrensmith
left a comment
There was a problem hiding this comment.
This will crash collection creation. If a user creates a schema with no search fields. Then in tenant.go where we create the search collection it will throw an error.
The best way to test this is to create a collection with no search fields and then update the collection with a field that needs to search indexed.
|
Yes, this is tested with a collection with no search index and then updating the collection with a search index. This shouldn't crash or error anything. Note that we are not removing the fields from the search but rather just not indexing them when we don't need them. This is mainly optimizing indexing. Do you see any errors or crashes? |
You can try it out like this, and then, |
| @@ -579,9 +556,9 @@ func (s *ImplicitSearchIndex) GetSearchDeltaFields(existingFields []*QueryableFi | |||
| tsFields = append(tsFields, tsApi.Field{ | |||
There was a problem hiding this comment.
We adding to the tsFields here but all the values would be false if we have no search fields. What would then happen for typesense?
I was testing this on the background search branch and what I did was not add the field to the tsFields if there was no search field But this caused the crashes in collection creation
There was a problem hiding this comment.
All the fields false and optional true mean storing in search but not indexing. This is needed so we don't need to read from the database when we get search results. You should not change that because that will break the search as then search results won't have full data.
garrensmith
left a comment
There was a problem hiding this comment.
Looks good. It might be worth checking if we have an integration test with no search fields just to make sure this doesn't throw errors
Yes, many of the collection tests are not adding a search index. |
Describe your changes
We only index fields in the indexing store that are tagged in the model
How best to test these changes
Issue ticket number and link