Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix nested field issue #277

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Conversation

kaituo
Copy link
Member

@kaituo kaituo commented Oct 19, 2020

Issue #, if available:

Description of changes:

Previously, our validation logic cannot parse nested fields from the getFieldMapping response because nested fields don't have the full name in the response. For example, the field nested_host.host only has "host" in the type mapping instead of "nested_host.host". This PR fixes that by not depending on field name when parsing the name type mapping.

Testing done:

  1. manual testing passes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Previously, our validation logic cannot parse nested fields from the getFieldMapping response because nested fields don't have the full name in the response.  For example, the field nested_host.host only has "host" in the type mapping instead of "nested_host.host".  This PR fixes that by not depending on field name when parsing the name type mapping.
Testing done:
1. manual testing passes.
@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #277 into master will decrease coverage by 0.03%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #277      +/-   ##
============================================
- Coverage     73.29%   73.25%   -0.04%     
- Complexity     1751     1752       +1     
============================================
  Files           180      180              
  Lines          8119     8122       +3     
  Branches        670      672       +2     
============================================
- Hits           5951     5950       -1     
- Misses         1859     1863       +4     
  Partials        309      309              
Flag Coverage Δ Complexity Δ
#cli 79.27% <ø> (ø) 0.00 <ø> (ø)
#plugin 72.67% <80.00%> (-0.05%) 1752.00 <0.00> (+1.00) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...est/handler/IndexAnomalyDetectorActionHandler.java 51.38% <80.00%> (+0.21%) 27.00 <0.00> (+1.00)
...ransport/DeleteAnomalyDetectorTransportAction.java 54.21% <0.00%> (-4.82%) 15.00% <0.00%> (-1.00%)
...sticsearch/ad/indices/AnomalyDetectionIndices.java 73.93% <0.00%> (+0.60%) 31.00% <0.00%> (+1.00%)

@ohltyler
Copy link
Contributor

Have manually confirmed this works for both flat and nested category fields.

Copy link
Contributor

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@yizheliu-amazon yizheliu-amazon added the bug Something isn't working label Oct 19, 2020
Copy link
Contributor

@yizheliu-amazon yizheliu-amazon left a comment

Choose a reason for hiding this comment

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

thanks for the quick fix

@kaituo kaituo merged commit 9a8d1f2 into opendistro-for-elasticsearch:master Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants