Skip to content

Commit

Permalink
Remove getMatchingFieldTypes method (#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 authored Jun 3, 2021
1 parent e036378 commit 05ca9cf
Showing 12 changed files with 70 additions and 227 deletions.
Original file line number Diff line number Diff line change
@@ -67,9 +67,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;

@@ -213,7 +213,7 @@ public MappedFieldType getFieldType(String path) {
}

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

Original file line number Diff line number Diff line change
@@ -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
@@ -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<>();

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

}
Original file line number Diff line number Diff line change
@@ -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;
@@ -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
@@ -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());

@@ -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"));
@@ -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"));
Original file line number Diff line number Diff line change
@@ -10,12 +10,10 @@

import org.elasticsearch.common.regex.Regex;

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;
@@ -147,49 +145,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
*
Original file line number Diff line number Diff line change
@@ -370,12 +370,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());
}

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

this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, mapping.getRoot().runtimeFields());
this.indexTimeLookup = indexTimeScriptMappers.isEmpty()
? null
: new FieldTypeLookup(mappers, aliasMappers, Collections.emptyList());
this.indexTimeLookup = new FieldTypeLookup(mappers, aliasMappers, Collections.emptyList());
this.fieldMappers = Collections.unmodifiableMap(fieldMappers);
this.objectMappers = Collections.unmodifiableMap(objects);
}
@@ -300,26 +298,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.
*/
Original file line number Diff line number Diff line change
@@ -58,7 +58,6 @@
import org.elasticsearch.transport.RemoteClusterAware;

import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -330,49 +329,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.
Original file line number Diff line number Diff line change
@@ -42,9 +42,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;
@@ -113,9 +113,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
@@ -360,8 +361,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
Loading

0 comments on commit 05ca9cf

Please sign in to comment.