Skip to content

Commit

Permalink
Make sure to return keyed JSON field types in FieldTypeLookup#iterato…
Browse files Browse the repository at this point in the history
…r. (#41319)

The index warmer iterates through all field types when determining the fields
for which global ordinals should be loaded. Previously, keyed JSON field types
were not returned from FieldTypeLookup#iterator, so their eager_global_ordinals
setting would be ignored. This PR fixes the issue by including keyed JSON fields
in FieldTypeLookup#iterator.
  • Loading branch information
jtibshirani committed May 29, 2019
1 parent 9126b04 commit 2b71ed8
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.index.mapper;

import org.elasticsearch.common.collect.CopyOnWriteHashMap;
import org.elasticsearch.common.collect.Iterators;
import org.elasticsearch.common.regex.Regex;

import java.util.Collection;
Expand Down Expand Up @@ -204,7 +205,16 @@ public Collection<String> simpleMatchToFullName(String pattern) {

@Override
public Iterator<MappedFieldType> iterator() {
return fullNameToFieldType.values().iterator();
Iterator<MappedFieldType> concreteFieldTypes = fullNameToFieldType.values().iterator();

if (fullNameToJsonMapper.isEmpty()) {
return concreteFieldTypes;
} else {
Iterator<MappedFieldType> keyedJsonFieldTypes = fullNameToJsonMapper.values().stream()
.<MappedFieldType>map(mapper -> mapper.keyedFieldType(""))
.iterator();
return Iterators.concat(concreteFieldTypes, keyedJsonFieldTypes);
}
}

// Visible for testing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@

import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import static java.util.Collections.emptyList;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.instanceOf;

public class FieldTypeLookupTests extends ESTestCase {
Expand Down Expand Up @@ -245,6 +248,22 @@ public void testSimpleMatchToFullName() {
assertTrue(names.contains("barometer"));
}

public void testFieldTypeIterator() {
MockFieldMapper mapper = new MockFieldMapper("foo");
JsonFieldMapper jsonMapper = createJsonMapper("object1.object2.field");

FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", newList(mapper, jsonMapper), emptyList());

Set<String> fieldNames = new HashSet<>();
for (MappedFieldType fieldType : lookup) {
fieldNames.add(fieldType.name());
}

assertThat(fieldNames, containsInAnyOrder(
mapper.name(), jsonMapper.name(), jsonMapper.keyedFieldName()));
}

public void testIteratorImmutable() {
MockFieldMapper f1 = new MockFieldMapper("foo");
FieldTypeLookup lookup = new FieldTypeLookup();
Expand Down

0 comments on commit 2b71ed8

Please sign in to comment.