-
Notifications
You must be signed in to change notification settings - Fork 123
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
Mappings update trigger a reindex #2333
Conversation
Codecov Report
@@ Coverage Diff @@
## 2-dev #2333 +/- ##
==========================================
- Coverage 91.44% 91.43% -0.01%
==========================================
Files 118 118
Lines 8033 8037 +4
==========================================
+ Hits 7346 7349 +3
- Misses 687 688 +1
Continue to review full report at Codecov.
|
…isible to search + conflicts param
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@@ -38,6 +38,29 @@ Feature: Collection Controller | |||
Then I should receive a "hits" array of objects matching: | |||
| _id | _source | | |||
| "document-1" | { "age": 2 } | | |||
|
|||
Scenario: Update a collection and search document (dyanmic false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scenario: Update a collection and search document (dyanmic false) | |
Scenario: Update a collection and search document (dynamic false) |
@@ -1387,6 +1387,20 @@ class ElasticSearch extends Service { | |||
|
|||
await this.updateMapping(index, collection, mappings); | |||
|
|||
// update documents with new mappings | |||
if (previousMappings.dynamic === 'false' && (await this._client.search(esRequest)).body.hits.hits.length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If think it's better to call count
than search
if you just want to know if there are documents to update
Side note:
Avoid putting long calls with chained property access like this in condition for clarity await this._client.search(esRequest)).body.hits.hits.length
prefer storing the result in a const variable.
Conditions that are too long and uses &&
or ||
should be splitted on multiple lines like so
const documentsCount = await this._client.search(esRequest)).body.hits.hits.length;
if ( previousMappings.dynamic === 'false'
&& documentsCount > 0
) {
When I "update" the collection "nyc-open-data":"green-taxi" with: | ||
| mappings | { "properties": { "age": { "type": "long" } } } | | ||
When I search documents with the following query: | ||
""" | ||
{ | ||
"match": { | ||
"age": 2 | ||
} | ||
} | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good but you also need to verify that the search does not match the field age
before updating the mapping and triggering the reindex
Closed due to inactivity |
What does this PR do ?
reindex documents with the new mappings (
collection:update
)How should this be manually tested?
Add a few documents in it.
Then add mappings to describe the fields in the added documents.
Expected: running a document:search in the collection should return the searched documents.
or run the functional tests