Skip to content

Replace some usages of QueryShardContext#getMapperService #63239

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,6 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
final List<ParsedDocument> docs = new ArrayList<>();
final DocumentMapper docMapper;
final MapperService mapperService = context.getMapperService();
String type = mapperService.documentMapper().type();
docMapper = mapperService.documentMapper();
for (BytesReference document : documents) {
docs.add(docMapper.parse(new SourceToParse(context.index().getName(), "_temp_id", document, documentXContentType)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapping;
import org.elasticsearch.index.mapper.ParseContext.Document;
import org.elasticsearch.index.mapper.ParsedDocument;
Expand Down Expand Up @@ -705,7 +705,7 @@ public enum SearcherScope {
* Creates a new history snapshot from Lucene for reading operations whose seqno in the requesting seqno range (both inclusive).
* This feature requires soft-deletes enabled. If soft-deletes are disabled, this method will throw an {@link IllegalStateException}.
*/
public abstract Translog.Snapshot newChangesSnapshot(String source, MapperService mapperService,
public abstract Translog.Snapshot newChangesSnapshot(String source, Function<String, MappedFieldType> fieldTypeLookup,
long fromSeqNo, long toSeqNo, boolean requiredFullRange) throws IOException;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.fieldvisitor.IdOnlyFieldVisitor;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
Expand Down Expand Up @@ -113,6 +113,7 @@
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.LongConsumer;
import java.util.function.LongSupplier;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -2457,14 +2458,14 @@ long getNumDocUpdates() {
}

@Override
public Translog.Snapshot newChangesSnapshot(String source, MapperService mapperService,
public Translog.Snapshot newChangesSnapshot(String source, Function<String, MappedFieldType> fieldTypeLookup,
long fromSeqNo, long toSeqNo, boolean requiredFullRange) throws IOException {
ensureOpen();
refreshIfNeeded(source, toSeqNo);
Searcher searcher = acquireSearcher(source, SearcherScope.INTERNAL);
try {
LuceneChangesSnapshot snapshot = new LuceneChangesSnapshot(
searcher, mapperService, LuceneChangesSnapshot.DEFAULT_BATCH_SIZE, fromSeqNo, toSeqNo, requiredFullRange);
searcher, fieldTypeLookup, LuceneChangesSnapshot.DEFAULT_BATCH_SIZE, fromSeqNo, toSeqNo, requiredFullRange);
searcher = null;
return snapshot;
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.fieldvisitor.FieldsVisitor;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.index.translog.Translog;
Expand All @@ -47,6 +47,7 @@
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;

/**
* A {@link Translog.Snapshot} from changes in a Lucene index
Expand All @@ -61,7 +62,7 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
private final boolean requiredFullRange;

private final IndexSearcher indexSearcher;
private final MapperService mapperService;
private final Function<String, MappedFieldType> fieldTypeLookup;
private int docIndex = 0;
private final int totalHits;
private ScoreDoc[] scoreDocs;
Expand All @@ -72,13 +73,13 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
* Creates a new "translog" snapshot from Lucene for reading operations whose seq# in the specified range.
*
* @param engineSearcher the internal engine searcher which will be taken over if the snapshot is opened successfully
* @param mapperService the mapper service which will be mainly used to resolve the document's type and uid
* @param fieldTypeLookup the lookup function used to resolve field types by name
* @param searchBatchSize the number of documents should be returned by each search
* @param fromSeqNo the min requesting seq# - inclusive
* @param toSeqNo the maximum requesting seq# - inclusive
* @param requiredFullRange if true, the snapshot will strictly check for the existence of operations between fromSeqNo and toSeqNo
*/
LuceneChangesSnapshot(Engine.Searcher engineSearcher, MapperService mapperService, int searchBatchSize,
LuceneChangesSnapshot(Engine.Searcher engineSearcher, Function<String, MappedFieldType> fieldTypeLookup, int searchBatchSize,
long fromSeqNo, long toSeqNo, boolean requiredFullRange) throws IOException {
if (fromSeqNo < 0 || toSeqNo < 0 || fromSeqNo > toSeqNo) {
throw new IllegalArgumentException("Invalid range; from_seqno [" + fromSeqNo + "], to_seqno [" + toSeqNo + "]");
Expand All @@ -92,7 +93,7 @@ final class LuceneChangesSnapshot implements Translog.Snapshot {
IOUtils.close(engineSearcher);
}
};
this.mapperService = mapperService;
this.fieldTypeLookup = fieldTypeLookup;
final long requestingSize = (toSeqNo - fromSeqNo) == Long.MAX_VALUE ? Long.MAX_VALUE : (toSeqNo - fromSeqNo + 1L);
this.searchBatchSize = requestingSize < searchBatchSize ? Math.toIntExact(requestingSize) : searchBatchSize;
this.fromSeqNo = fromSeqNo;
Expand Down Expand Up @@ -234,7 +235,7 @@ private Translog.Operation readDocAsOp(int docIndex) throws IOException {
SourceFieldMapper.NAME;
final FieldsVisitor fields = new FieldsVisitor(true, sourceField);
leaf.reader().document(segmentDocID, fields);
fields.postProcess(mapperService);
fields.postProcess(fieldTypeLookup);

final Translog.Operation op;
final boolean isTombstone = parallelArray.isTombStone[docIndex];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.seqno.SeqNoStats;
import org.elasticsearch.index.seqno.SequenceNumbers;
import org.elasticsearch.index.store.Store;
Expand Down Expand Up @@ -291,8 +291,8 @@ public Closeable acquireHistoryRetentionLock() {
}

@Override
public Translog.Snapshot newChangesSnapshot(String source, MapperService mapperService, long fromSeqNo, long toSeqNo,
boolean requiredFullRange) {
public Translog.Snapshot newChangesSnapshot(String source, Function<String, MappedFieldType> fieldTypeLookup, long fromSeqNo,
long toSeqNo, boolean requiredFullRange) {
return newEmptySnapshot();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.IgnoredFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.RoutingFieldMapper;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.index.mapper.Uid;
Expand All @@ -38,6 +37,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableSet;
Expand Down Expand Up @@ -87,9 +87,9 @@ public Status needsField(FieldInfo fieldInfo) {
: Status.NO;
}

public void postProcess(MapperService mapperService) {
public final void postProcess(Function<String, MappedFieldType> fieldTypeLookup) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do wonder if we can remove this step entirely, or replace it with the ValueFetcher functionality somehow - for a followup though.

Copy link
Contributor

Choose a reason for hiding this comment

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

So this doesn't do anything to the _id, _source or _routing fields, so I think we can remove the call to it entirely from LuceneChangesSnapshot and possibly a few other places as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok I will look at it, possibly make this change in a separate PR then.

Copy link
Contributor

Choose a reason for hiding this comment

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

++

for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
MappedFieldType fieldType = mapperService.fieldType(entry.getKey());
MappedFieldType fieldType = fieldTypeLookup.apply(entry.getKey());
if (fieldType == null) {
throw new IllegalStateException("Field [" + entry.getKey()
+ "] exists in the index but not in mappings");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ private GetResult innerGetLoadFromStoredFields(String id, String[] storedFields,

// put stored fields into result objects
if (!fieldVisitor.fields().isEmpty()) {
fieldVisitor.postProcess(mapperService);
fieldVisitor.postProcess(mapperService::fieldType);
documentFields = new HashMap<>();
metadataFields = new HashMap<>();
for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public static Query newFilter(QueryShardContext context, String fieldPattern, bo
}

private static Query newFieldExistsQuery(QueryShardContext context, String field) {
MappedFieldType fieldType = context.getMapperService().fieldType(field);
MappedFieldType fieldType = context.fieldMapper(field);
if (fieldType == null) {
// The field does not exist as a leaf but could be an object so
// check for an object mapper
Expand All @@ -182,7 +182,7 @@ private static Query newObjectFieldExistsQuery(QueryShardContext context, String
BooleanQuery.Builder booleanQuery = new BooleanQuery.Builder();
Collection<String> fields = context.simpleMatchToIndexNames(objField + ".*");
for (String field : fields) {
Query existsQuery = context.getMapperService().fieldType(field).existsQuery(context);
Query existsQuery = context.fieldMapper(field).existsQuery(context);
booleanQuery.add(existsQuery, Occur.SHOULD);
}
return new ConstantScoreQuery(booleanQuery.build());
Expand All @@ -194,7 +194,7 @@ private static Query newObjectFieldExistsQuery(QueryShardContext context, String
*/
private static Collection<String> getMappedField(QueryShardContext context, String fieldPattern) {
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType) context
.getMapperService().fieldType(FieldNamesFieldMapper.NAME);
.fieldMapper(FieldNamesFieldMapper.NAME);

if (fieldNamesFieldType == null) {
// can only happen when no types exist, so no docs exist either
Expand All @@ -212,7 +212,7 @@ private static Collection<String> getMappedField(QueryShardContext context, Stri

if (fields.size() == 1) {
String field = fields.iterator().next();
MappedFieldType fieldType = context.getMapperService().fieldType(field);
MappedFieldType fieldType = context.fieldMapper(field);
if (fieldType == null) {
// The field does not exist as a leaf but could be an object so
// check for an object mapper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,11 @@ protected void setupInnerHitsContext(QueryShardContext queryShardContext,
innerHitsContext.storedFieldsContext(innerHitBuilder.getStoredFieldsContext());
}
if (innerHitBuilder.getDocValueFields() != null) {
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(
queryShardContext.getMapperService(), innerHitBuilder.getDocValueFields());
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(queryShardContext::simpleMatchToIndexNames,
queryShardContext.getIndexSettings().getMaxDocvalueFields(), innerHitBuilder.getDocValueFields());
innerHitsContext.docValuesContext(docValuesContext);
}
if (innerHitBuilder.getFetchFields() != null) {
String indexName = queryShardContext.index().getName();
FetchFieldsContext fieldsContext = new FetchFieldsContext(innerHitBuilder.getFetchFields());
innerHitsContext.fetchFieldsContext(fieldsContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ protected Query doToQuery(QueryShardContext context) throws IOException {

// ToParentBlockJoinQuery requires that the inner query only matches documents
// in its child space
if (new NestedHelper(context.getMapperService()).mightMatchNonNestedDocs(innerQuery, path)) {
NestedHelper nestedHelper = new NestedHelper(context.getMapperService()::getObjectMapper, context.getMapperService()::fieldType);
if (nestedHelper.mightMatchNonNestedDocs(innerQuery, path)) {
innerQuery = Queries.filtered(innerQuery, nestedObjectMapper.nestedTypeFilter());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public Analyzer getSearchAnalyzer(MappedFieldType fieldType) {
if (fieldType.getTextSearchInfo().getSearchAnalyzer() != null) {
return fieldType.getTextSearchInfo().getSearchAnalyzer();
}
return getMapperService().searchAnalyzer();
return mapperService.searchAnalyzer();
}

/**
Expand All @@ -254,7 +254,7 @@ public Analyzer getSearchQuoteAnalyzer(MappedFieldType fieldType) {
if (fieldType.getTextSearchInfo().getSearchQuoteAnalyzer() != null) {
return fieldType.getTextSearchInfo().getSearchQuoteAnalyzer();
}
return getMapperService().searchQuoteAnalyzer();
return mapperService.searchQuoteAnalyzer();
}

public ValuesSourceRegistry getValuesSourceRegistry() {
Expand Down Expand Up @@ -288,7 +288,7 @@ MappedFieldType failIfFieldMappingNotFound(String name, MappedFieldType fieldMap
public SearchLookup lookup() {
if (this.lookup == null) {
this.lookup = new SearchLookup(
getMapperService(),
mapperService,
(fieldType, searchLookup) -> indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), searchLookup)
);
}
Expand All @@ -304,7 +304,7 @@ public SearchLookup newFetchLookup() {
* Real customization coming soon, I promise!
*/
return new SearchLookup(
getMapperService(),
mapperService,
(fieldType, searchLookup) -> indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), searchLookup)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
* if the {@link FieldNamesFieldMapper} is enabled.
*/
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType =
(FieldNamesFieldMapper.FieldNamesFieldType) context.getMapperService().fieldType(FieldNamesFieldMapper.NAME);
(FieldNamesFieldMapper.FieldNamesFieldType) context.fieldMapper(FieldNamesFieldMapper.NAME);
if (fieldNamesFieldType == null) {
return new MatchNoDocsQuery("No mappings yet");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ protected int doHashCode() {

@Override
protected ScoreFunction doToFunction(QueryShardContext context) {
MappedFieldType fieldType = context.getMapperService().fieldType(field);
MappedFieldType fieldType = context.fieldMapper(field);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should rename this fieldType rather than fieldMapper?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes indeed, I planned to do it as a follow-up

IndexNumericFieldData fieldData = null;
if (fieldType == null) {
if(missing == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ protected ScoreFunction doToFunction(QueryShardContext context) {
} else {
final MappedFieldType fieldType;
if (field != null) {
fieldType = context.getMapperService().fieldType(field);
fieldType = context.fieldMapper(field);
} else {
deprecationLogger.deprecate("seed_requires_field",
"As of version 7.0 Elasticsearch will require that a [field] parameter is provided when a [seed] is set");
fieldType = context.getMapperService().fieldType(IdFieldMapper.NAME);
fieldType = context.fieldMapper(IdFieldMapper.NAME);
}
if (fieldType == null) {
if (context.getMapperService().documentMapper() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.apache.lucene.index.PrefixCodedTerms;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.ConstantScoreQuery;
Expand All @@ -31,18 +32,21 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermInSetQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.BooleanClause.Occur;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.ObjectMapper;

import java.util.function.Function;

/** Utility class to filter parent and children clauses when building nested
* queries. */
public final class NestedHelper {

private final MapperService mapperService;
private final Function<String, ObjectMapper> objectMapperLookup;
private final Function<String, MappedFieldType> fieldTypeLookup;

public NestedHelper(MapperService mapperService) {
this.mapperService = mapperService;
public NestedHelper(Function<String, ObjectMapper> objectMapperLookup, Function<String, MappedFieldType> fieldTypeLookup) {
this.objectMapperLookup = objectMapperLookup;
this.fieldTypeLookup = fieldTypeLookup;
}

/** Returns true if the given query might match nested documents. */
Expand Down Expand Up @@ -103,12 +107,12 @@ boolean mightMatchNestedDocs(String field) {
// we might add a nested filter when it is nor required.
return true;
}
if (mapperService.fieldType(field) == null) {
if (fieldTypeLookup.apply(field) == null) {
// field does not exist
return false;
}
for (String parent = parentObject(field); parent != null; parent = parentObject(parent)) {
ObjectMapper mapper = mapperService.getObjectMapper(parent);
ObjectMapper mapper = objectMapperLookup.apply(parent);
if (mapper != null && mapper.nested().isNested()) {
return true;
}
Expand Down Expand Up @@ -172,11 +176,11 @@ boolean mightMatchNonNestedDocs(String field, String nestedPath) {
// we might add a nested filter when it is nor required.
return true;
}
if (mapperService.fieldType(field) == null) {
if (fieldTypeLookup.apply(field) == null) {
return false;
}
for (String parent = parentObject(field); parent != null; parent = parentObject(parent)) {
ObjectMapper mapper = mapperService.getObjectMapper(parent);
ObjectMapper mapper = objectMapperLookup.apply(parent);
if (mapper!= null && mapper.nested().isNested()) {
if (mapper.fullPath().equals(nestedPath)) {
// If the mapper does not include in its parent or in the root object then
Expand Down
Loading