Skip to content

Commit

Permalink
Remove getMatchingFieldTypes method (elastic#73655)
Browse files Browse the repository at this point in the history
FieldTypeLookup and MappingLookup expose the getMatchingFieldTypes method to look up matching field type by a string pattern. We have migrated ExistsQueryBuilder to instead rely on getMatchingFieldNames, hence we can go ahead and remove the remaining usages and the method itself.

The remaining usages are to find specific field types from the mappings, specifically to eagerly load global ordinals and for the join field type. These are operations that are performed only once when loading the mappings, and may be refactored to work differently in the future. For now, we remove getMatchingFieldTypes and rather call for the two mentioned scenarios getMatchingFieldNames(*) and then getFieldType for each of the returned field name. This is a bit wasteful but performance can be sacrificed for these scenarios in favour of less code to maintain.
  • Loading branch information
javanna committed Jun 3, 2021
1 parent e1c87ae commit a3c548b
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 226 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

Expand Down Expand Up @@ -222,7 +222,7 @@ public MappedFieldType getFieldType(String path) {
}

@Override
public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
public Set<String> getMatchingFieldNames(String pattern) {
throw new UnsupportedOperationException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
import org.elasticsearch.join.mapper.ParentJoinFieldMapper.JoinFieldType;
import org.elasticsearch.search.aggregations.support.AggregationContext;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Stream;

/**
* Utility class to help build join queries and aggregations, based on a join_field
Expand All @@ -36,24 +37,23 @@ public final class Joiner {
* Get the Joiner for this context, or {@code null} if none is configured
*/
public static Joiner getJoiner(SearchExecutionContext context) {
return getJoiner(context.getAllFieldTypes());
return getJoiner(context.getMatchingFieldNames("*").stream().map(context::getFieldType));
}

/**
* Get the Joiner for this context, or {@code null} if none is configured
*/
public static Joiner getJoiner(AggregationContext context) {
return getJoiner(context.getMatchingFieldTypes("*"));
return getJoiner(context.getMatchingFieldNames("*").stream().map(context::getFieldType));
}

/**
* Get the Joiner for this context, or {@code null} if none is configured
*/
static Joiner getJoiner(Collection<MappedFieldType> fieldTypes) {
JoinFieldType ft = ParentJoinFieldMapper.getJoinFieldType(fieldTypes);
return ft != null ? ft.getJoiner() : null;
static Joiner getJoiner(Stream<MappedFieldType> fieldTypes) {
Optional<JoinFieldType> joinType = fieldTypes.filter(ft -> ft instanceof JoinFieldType).map(ft -> (JoinFieldType) ft).findFirst();
return joinType.map(JoinFieldType::getJoiner).orElse(null);
}

private final Map<String, Set<String>> parentsToChildren = new HashMap<>();
private final Map<String, String> childrenToParents = new HashMap<>();

Expand Down Expand Up @@ -178,5 +178,4 @@ boolean canMerge(Joiner other, Consumer<String> conflicts) {
}
return conflicted == false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
Expand Down Expand Up @@ -291,31 +290,11 @@ public FieldMapper.Builder getMergeBuilder() {
}

@Override
protected void doValidate(MappingLookup mappers) {
List<String> joinFields = getJoinFieldTypes(mappers.getAllFieldTypes()).stream()
.map(JoinFieldType::name)
.collect(Collectors.toList());
protected void doValidate(MappingLookup mappingLookup) {
List<String> joinFields = mappingLookup.getMatchingFieldNames("*").stream().map(mappingLookup::getFieldType)
.filter(ft -> ft instanceof JoinFieldType).map(MappedFieldType::name).collect(Collectors.toList());
if (joinFields.size() > 1) {
throw new IllegalArgumentException("Only one [parent-join] field can be defined per index, got " + joinFields);
}
}

static JoinFieldType getJoinFieldType(Collection<MappedFieldType> fieldTypes) {
for (MappedFieldType ft : fieldTypes) {
if (ft instanceof JoinFieldType) {
return (JoinFieldType) ft;
}
}
return null;
}

private List<JoinFieldType> getJoinFieldTypes(Collection<MappedFieldType> fieldTypes) {
final List<JoinFieldType> joinFields = new ArrayList<>();
for (MappedFieldType ft : fieldTypes) {
if (ft instanceof JoinFieldType) {
joinFields.add((JoinFieldType) ft);
}
}
return joinFields;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public void testSingleLevel() throws Exception {
b.endObject();
}));
DocumentMapper docMapper = mapperService.documentMapper();

Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getMatchingFieldNames("*").stream()
.map(mapperService.mappingLookup()::getFieldType));
assertNotNull(joiner);
assertEquals("join_field", joiner.getJoinField());

Expand Down Expand Up @@ -233,7 +233,8 @@ public void testUpdateRelations() throws Exception {
b.endObject();
}));

Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getMatchingFieldNames("*").stream()
.map(mapperService.mappingLookup()::getFieldType));
assertNotNull(joiner);
assertEquals("join_field", joiner.getJoinField());
assertTrue(joiner.childTypeExists("child2"));
Expand All @@ -259,7 +260,8 @@ public void testUpdateRelations() throws Exception {
}
b.endObject();
}));
joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
joiner = Joiner.getJoiner(mapperService.mappingLookup().getMatchingFieldNames("*").stream()
.map(mapperService.mappingLookup()::getFieldType));
assertNotNull(joiner);
assertEquals("join_field", joiner.getJoinField());
assertTrue(joiner.childTypeExists("child2"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.mapper.TypeFieldMapper.TypeFieldType;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -154,49 +152,6 @@ private MappedFieldType getDynamicField(String field) {
}
}

/**
* Returns all the mapped field types that match a pattern
*
* Note that if a field is aliased and both its actual name and its alias
* match the pattern, the returned collection will contain the field type
* twice.
*/
Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
if (Regex.isMatchAllPattern(pattern)) {
if (dynamicFieldTypes.isEmpty()) {
return fullNameToFieldType.values();
}
List<MappedFieldType> fieldTypes = new ArrayList<>(fullNameToFieldType.values());
for (DynamicFieldType dynamicFieldType : dynamicFieldTypes.values()) {
for (String subfield : dynamicFieldType.getKnownSubfields()) {
fieldTypes.add(dynamicFieldType.getChildFieldType(subfield));
}
}
return fieldTypes;
}
if (Regex.isSimpleMatchPattern(pattern) == false) {
// no wildcards
MappedFieldType ft = get(pattern);
return ft == null ? Collections.emptySet() : Collections.singleton(ft);
}
List<MappedFieldType> matchingFields = new ArrayList<>();
for (String field : fullNameToFieldType.keySet()) {
if (Regex.simpleMatch(pattern, field)) {
matchingFields.add(fullNameToFieldType.get(field));
}
}
for (Map.Entry<String, DynamicFieldType> dynamicFieldTypeEntry : dynamicFieldTypes.entrySet()) {
String parentName = dynamicFieldTypeEntry.getKey();
DynamicFieldType dynamicFieldType = dynamicFieldTypeEntry.getValue();
for (String subfield : dynamicFieldType.getKnownSubfields()) {
if (Regex.simpleMatch(pattern, parentName + "." + subfield)) {
matchingFields.add(dynamicFieldType.getChildFieldType(subfield));
}
}
}
return matchingFields;
}

/**
* Returns a set of field names that match a regex-like pattern
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,12 +608,13 @@ public MappingLookup mappingLookup() {
* Returns all mapped field types.
*/
public Iterable<MappedFieldType> getEagerGlobalOrdinalsFields() {
if (this.mapper == null) {
DocumentMapper mapper = this.mapper;
if (mapper == null) {
return Collections.emptySet();
}
return this.mapper.mappers().getAllFieldTypes().stream()
.filter(MappedFieldType::eagerGlobalOrdinals)
.collect(Collectors.toList());
MappingLookup mappingLookup = mapper.mappers();
return mappingLookup.getMatchingFieldNames("*").stream().map(mappingLookup::getFieldType)
.filter(MappedFieldType::eagerGlobalOrdinals).collect(Collectors.toList());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,7 @@ private MappingLookup(Mapping mapping,
}

this.fieldTypeLookup = new FieldTypeLookup(mapping.type(), mappers, aliasMappers, mapping.getRoot().runtimeFields());
this.indexTimeLookup = indexTimeScriptMappers.isEmpty()
? null
: new FieldTypeLookup(mapping.type(), mappers, aliasMappers, emptyList());
this.indexTimeLookup = new FieldTypeLookup(mapping.type(), mappers, aliasMappers, emptyList());
this.fieldMappers = Collections.unmodifiableMap(fieldMappers);
this.objectMappers = Collections.unmodifiableMap(objects);
}
Expand Down Expand Up @@ -302,26 +300,6 @@ public Set<String> getMatchingFieldNames(String pattern) {
return fieldTypeLookup.getMatchingFieldNames(pattern);
}

/**
* Returns all the mapped field types that match a pattern
*
* Note that if a field is aliased and both its actual name and its alias
* match the pattern, the returned collection will contain the field type
* twice.
*
* @param pattern the pattern to match field names against
*/
public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
return fieldTypeLookup.getMatchingFieldTypes(pattern);
}

/**
* @return all mapped field types
*/
public Collection<MappedFieldType> getAllFieldTypes() {
return fieldTypeLookup.getMatchingFieldTypes("*");
}

/**
* Returns the mapped field type for the given field name.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,49 +346,6 @@ public Set<String> getMatchingFieldNames(String pattern) {
return matches;
}

/**
* @return all mapped field types, including runtime fields defined in the request
*/
public Collection<MappedFieldType> getAllFieldTypes() {
return getMatchingFieldTypes("*");
}

/**
* Returns all mapped field types that match a given pattern
*
* Includes any runtime fields that have been defined in the request. Note
* that a runtime field with the same name as a mapped field will override
* the mapped field.
*
* @param pattern the field name pattern
*/
public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
Collection<MappedFieldType> mappedFieldTypes = mappingLookup.getMatchingFieldTypes(pattern);
if (runtimeMappings.isEmpty()) {
return mappedFieldTypes;
}

Map<String, MappedFieldType> mappedByName = new HashMap<>();
mappedFieldTypes.forEach(ft -> mappedByName.put(ft.name(), ft));

if ("*".equals(pattern)) {
mappedByName.putAll(runtimeMappings);
} else if (Regex.isSimpleMatchPattern(pattern) == false) {
// no wildcard
if (runtimeMappings.containsKey(pattern) == false) {
return mappedFieldTypes;
}
mappedByName.put(pattern, runtimeMappings.get(pattern));
} else {
for (String name : runtimeMappings.keySet()) {
if (Regex.simpleMatch(pattern, name)) {
mappedByName.put(name, runtimeMappings.get(name));
}
}
}
return mappedByName.values();
}

/**
* Returns the {@link MappedFieldType} for the provided field name.
* If the field is not mapped, the behaviour depends on the index.query.parse.allow_unmapped_fields setting, which defaults to true.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.function.Supplier;
Expand Down Expand Up @@ -114,9 +114,10 @@ public final FieldContext buildFieldContext(MappedFieldType ft) {
public abstract MappedFieldType getFieldType(String path);

/**
* Returns the registered mapped field types.
* Returns a set of field names that match a regex-like pattern
* All field names in the returned set are guaranteed to resolve to a field
*/
public abstract Collection<MappedFieldType> getMatchingFieldTypes(String pattern);
public abstract Set<String> getMatchingFieldNames(String pattern);

/**
* Returns true if the field identified by the provided name is mapped, false otherwise
Expand Down Expand Up @@ -363,8 +364,8 @@ public MappedFieldType getFieldType(String path) {
}

@Override
public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
return context.getMatchingFieldTypes(pattern);
public Set<String> getMatchingFieldNames(String pattern) {
return context.getMatchingFieldNames(pattern);
}

@Override
Expand Down
Loading

0 comments on commit a3c548b

Please sign in to comment.