Skip to content
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

Refactors TermsQueryBuilder and Parser #12042

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 @@ -43,12 +43,15 @@
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.internal.AllFieldMapper;
import org.elasticsearch.index.search.termslookup.TermsLookupFetchService;
import org.elasticsearch.index.settings.IndexSettings;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.indices.cache.query.terms.TermsLookup;
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.script.ScriptService;

import java.io.IOException;
import java.util.List;

public class IndexQueryParserService extends AbstractIndexComponent {

Expand Down Expand Up @@ -89,6 +92,8 @@ protected QueryShardContext initialValue() {
private final ParseFieldMatcher parseFieldMatcher;
private final boolean defaultAllowUnmappedFields;

private TermsLookupFetchService termsLookupFetchService;
Copy link
Member

Choose a reason for hiding this comment

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

I think I prefer the Client directly here. This service makes you think that there is no client needed, but it just hides that client away :) I think I prefer to have the client here but keep it contained and add the fetch method directly to IndexQueryParsreService, at least for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Geo and MLT all use a fetch service, so this is being consistent with these queries.


@Inject
public IndexQueryParserService(Index index, @IndexSettings Settings indexSettings,
IndicesQueriesRegistry indicesQueriesRegistry,
Expand All @@ -115,6 +120,11 @@ public IndexQueryParserService(Index index, @IndexSettings Settings indexSetting
this.indicesQueriesRegistry = indicesQueriesRegistry;
}

@Inject(optional=true)
public void setTermsLookupFetchService(@Nullable TermsLookupFetchService termsLookupFetchService) {
this.termsLookupFetchService = termsLookupFetchService;
}

public void close() {
cache.close();
}
Expand Down Expand Up @@ -339,4 +349,8 @@ public boolean matchesIndices(String... indices) {
}
return false;
}

public List<Object> handleTermsLookup(TermsLookup termsLookup) {
return this.termsLookupFetchService.fetch(termsLookup);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ public static TypeQueryBuilder typeQuery(String type) {
* A terms query that can extract the terms from another doc in an index.
*/
public static TermsQueryBuilder termsLookupQuery(String name) {
return new TermsQueryBuilder(name, (Object[]) null);
return new TermsQueryBuilder(name);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,5 +327,4 @@ public QueryParseContext parseContext() {
public boolean matchesIndices(String... indices) {
return this.indexQueryParserService().matchesIndices(indices);
}

}
Loading