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

fixing ElasticsearchDocumentStore initialisation #415

Merged
merged 2 commits into from
Sep 22, 2020
Merged

fixing ElasticsearchDocumentStore initialisation #415

merged 2 commits into from
Sep 22, 2020

Conversation

guillim
Copy link
Contributor

@guillim guillim commented Sep 21, 2020

By default, Elasticsearch creates an index called document. That's the biggest problem when initializing ElasticsearchDocumentStore through Haystack. Because when calling _create_document_index, you may end up doing nothing since document index is already created. This is not a problem for non-vector Retriever.

But Embeddings needs special type "dense_vector" into Elasticsearch mapping definition. That's why it is important to update the default 'document' index in this case.

By default, Elasticsearch creates an index called document. That's the biggest problem when initializing ElasticsearchDocumentStore through Haystack. Because when calling `_create_document_index`, you may end up doing nothing since `document` index is already created. This is not a problem for non-vector Retriever. 

But Embeddings needs special type "dense_vector" into Elasticsearch mapping definition. That's why it is important to update the default 'document' index in this case.
@tanaysoni
Copy link
Contributor

Hi @guillim, thank you for the PR! I just added a check to ensure we don't overwrite an existing field.

@tanaysoni tanaysoni merged commit 29cbd1e into deepset-ai:master Sep 22, 2020
@guillim guillim deleted the patch-2 branch September 22, 2020 10:13
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