From b3d767343006e92cac8f37604ddf598fb7efa9bb Mon Sep 17 00:00:00 2001 From: Mathias Fussenegger Date: Fri, 13 Mar 2015 23:08:30 +0100 Subject: [PATCH] make LuceneQueryBuilder a singleton --- .../action/sql/query/CrateSearchService.java | 8 +- .../io/crate/lucene/LuceneQueryBuilder.java | 141 ++++++++++-------- .../operation/collect/LuceneDocCollector.java | 6 +- .../collect/ShardCollectService.java | 5 + .../crate/lucene/LuceneQueryBuilderTest.java | 10 +- 5 files changed, 99 insertions(+), 71 deletions(-) diff --git a/sql/src/main/java/io/crate/action/sql/query/CrateSearchService.java b/sql/src/main/java/io/crate/action/sql/query/CrateSearchService.java index 9c05cb656c12..5ce5848371c2 100644 --- a/sql/src/main/java/io/crate/action/sql/query/CrateSearchService.java +++ b/sql/src/main/java/io/crate/action/sql/query/CrateSearchService.java @@ -88,6 +88,7 @@ public class CrateSearchService extends InternalSearchService { private final SortSymbolVisitor sortSymbolVisitor; private final Functions functions; + private LuceneQueryBuilder luceneQueryBuilder; @Inject public CrateSearchService(Settings settings, @@ -104,7 +105,8 @@ public CrateSearchService(Settings settings, QueryPhase queryPhase, FetchPhase fetchPhase, Functions functions, - IndicesQueryCache indicesQueryCache) { + IndicesQueryCache indicesQueryCache, + LuceneQueryBuilder luceneQueryBuilder) { super(settings, clusterService, indicesService, indicesLifecycle, indicesWarmer, threadPool, @@ -113,6 +115,7 @@ public CrateSearchService(Settings settings, pageCacheRecycler, bigArrays, dfsPhase, queryPhase, fetchPhase, indicesQueryCache); this.functions = functions; + this.luceneQueryBuilder = luceneQueryBuilder; CollectInputSymbolVisitor> inputSymbolVisitor = new CollectInputSymbolVisitor<>(functions, LuceneDocLevelReferenceResolver.INSTANCE); sortSymbolVisitor = new SortSymbolVisitor(inputSymbolVisitor); @@ -235,8 +238,7 @@ private SearchContext createContext(QueryShardRequest request, @Nullable Engine. SearchContext.setCurrent(context); try { - LuceneQueryBuilder builder = new LuceneQueryBuilder(functions, context, indexService.cache()); - LuceneQueryBuilder.Context ctx = builder.convert(request.whereClause()); + LuceneQueryBuilder.Context ctx = luceneQueryBuilder.convert(request.whereClause(), context, indexService.cache()); context.parsedQuery(new ParsedQuery(ctx.query(), ImmutableMap.of())); Float minScore = ctx.minScore(); if (minScore != null) { diff --git a/sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java b/sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java index 7e31721220bc..dce8d313f98b 100644 --- a/sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java +++ b/sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java @@ -67,6 +67,8 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.geo.GeoDistance; import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.inject.Singleton; import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.lucene.docset.MatchDocIdSet; import org.elasticsearch.common.lucene.search.NotFilter; @@ -88,25 +90,25 @@ import static com.google.common.base.Preconditions.checkArgument; import static io.crate.operation.scalar.regex.RegexMatcher.isPcrePattern; +@Singleton public class LuceneQueryBuilder { - private final Visitor visitor; + private final static Visitor VISITOR = new Visitor(); + private final CollectInputSymbolVisitor> inputSymbolVisitor; - public LuceneQueryBuilder(Functions functions, SearchContext searchContext, IndexCache indexCache) { - CollectInputSymbolVisitor> inputSymbolVisitor = - new CollectInputSymbolVisitor<>(functions, LuceneDocLevelReferenceResolver.INSTANCE); - visitor = new Visitor(inputSymbolVisitor, indexCache); - visitor.searchContext = searchContext; + @Inject + public LuceneQueryBuilder(Functions functions) { + inputSymbolVisitor = new CollectInputSymbolVisitor<>(functions, LuceneDocLevelReferenceResolver.INSTANCE); } - public Context convert(WhereClause whereClause) throws UnsupportedFeatureException { - Context ctx = new Context(); + public Context convert(WhereClause whereClause, SearchContext searchContext, IndexCache indexCache) throws UnsupportedFeatureException { + Context ctx = new Context(inputSymbolVisitor, searchContext, indexCache); if (whereClause.noMatch()) { ctx.query = Queries.newMatchNoDocsQuery(); } else if (!whereClause.hasQuery()) { ctx.query = Queries.newMatchAllQuery(); } else { - ctx.query = visitor.process(whereClause.query(), ctx); + ctx.query = VISITOR.process(whereClause.query(), ctx); } return ctx; } @@ -116,6 +118,18 @@ public static class Context { final Map filteredFieldValues = new HashMap<>(); + final SearchContext searchContext; + final CollectInputSymbolVisitor> inputSymbolVisitor; + final IndexCache indexCache; + + Context(CollectInputSymbolVisitor> inputSymbolVisitor, + SearchContext searchContext, + IndexCache indexCache) { + this.inputSymbolVisitor = inputSymbolVisitor; + this.searchContext = searchContext; + this.indexCache = indexCache; + } + public Query query() { return this.query; } @@ -182,23 +196,13 @@ public static String convertWildcard(String wildcardString) { static class Visitor extends SymbolVisitor { - private SearchContext searchContext; - private final CollectInputSymbolVisitor> inputSymbolVisitor; - private final IndexCache indexCache; - - public Visitor(CollectInputSymbolVisitor> inputSymbolVisitor, - IndexCache indexCache) { - this.inputSymbolVisitor = inputSymbolVisitor; - this.indexCache = indexCache; - } - interface FunctionToQuery { @Nullable public Query apply (Function input, Context context) throws IOException; } - abstract class CmpQuery implements FunctionToQuery { + static abstract class CmpQuery implements FunctionToQuery { @Nullable protected Tuple prepare(Function input) { @@ -219,7 +223,7 @@ protected Tuple prepare(Function input) { /** * 1 != any ( col ) --> gt 1 or lt 1 */ - class AnyNeqQuery extends CmpQuery { + static class AnyNeqQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -246,7 +250,7 @@ public Query apply(Function input, Context context) { } } - class AnyNotLikeQuery extends CmpQuery { + static class AnyNotLikeQuery extends CmpQuery { private String negateWildcard(String wildCard) { return String.format("~(%s)", wildCard); @@ -268,7 +272,8 @@ public Query apply(Function input, Context context) { ); } } - class LikeQuery extends CmpQuery { + + static class LikeQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -282,7 +287,7 @@ public Query apply(Function input, Context context) { } } - class InQuery extends CmpQuery { + static class InQuery extends CmpQuery { @Override public Query apply(Function input, Context context) throws IOException { @@ -318,7 +323,7 @@ public Query apply(Function input, Context context) { } } - class IsNullQuery implements FunctionToQuery { + static class IsNullQuery implements FunctionToQuery { @Override public Query apply(Function input, Context context) { @@ -338,7 +343,8 @@ public Query apply(Function input, Context context) { } } - class EqQuery extends CmpQuery { + static class EqQuery extends CmpQuery { + @Override public Query apply(Function input, Context context) { Tuple tuple = super.prepare(input); @@ -362,7 +368,7 @@ public Query apply(Function input, Context context) { if (boolTermsFilter.clauses().isEmpty()) { // all values are null... - return genericFunctionQuery(input); + return genericFunctionQuery(input, context.inputSymbolVisitor, context.searchContext); } // wrap boolTermsFilter and genericFunction filter in an additional BooleanFilter to control the ordering of the filters @@ -370,7 +376,9 @@ public Query apply(Function input, Context context) { // afterwards the more expensive genericFunctionFilter BooleanFilter filterClauses = new BooleanFilter(); filterClauses.add(boolTermsFilter, BooleanClause.Occur.MUST); - filterClauses.add(genericFunctionFilter(input), BooleanClause.Occur.MUST); + filterClauses.add( + genericFunctionFilter(input, context.inputSymbolVisitor, context.searchContext), + BooleanClause.Occur.MUST); return new FilteredQuery(Queries.newMatchAllQuery(), filterClauses); } QueryBuilderHelper builder = QueryBuilderHelper.forType(tuple.v1().valueType()); @@ -420,7 +428,7 @@ public Query apply(Function input, Context context) { } } - class LtQuery extends CmpQuery { + static class LtQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -435,7 +443,7 @@ public Query apply(Function input, Context context) { } } - class LteQuery extends CmpQuery { + static class LteQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -450,7 +458,7 @@ public Query apply(Function input, Context context) { } } - class GtQuery extends CmpQuery { + static class GtQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -465,7 +473,7 @@ public Query apply(Function input, Context context) { } } - class GteQuery extends CmpQuery { + static class GteQuery extends CmpQuery { @Override public Query apply(Function input, Context context) { @@ -480,7 +488,7 @@ public Query apply(Function input, Context context) { } } - class ToMatchQuery implements FunctionToQuery { + static class ToMatchQuery implements FunctionToQuery { @Override public Query apply(Function input, Context context) throws IOException { @@ -501,15 +509,15 @@ public Query apply(Function input, Context context) throws IOException { MatchQueryBuilder queryBuilder; if (fields.size() == 1) { - queryBuilder = new MatchQueryBuilder(searchContext, indexCache, matchType, options); + queryBuilder = new MatchQueryBuilder(context.searchContext, context.indexCache, matchType, options); } else { - queryBuilder = new MultiMatchQueryBuilder(searchContext, indexCache, matchType, options); + queryBuilder = new MultiMatchQueryBuilder(context.searchContext, context.indexCache, matchType, options); } return queryBuilder.query(fields, queryString); } } - class RegexpMatchQuery extends CmpQuery { + static class RegexpMatchQuery extends CmpQuery { @Override public Query apply(Function input, Context context) throws IOException { @@ -539,7 +547,7 @@ public Query apply(Function input, Context context) throws IOException { } } - class RegexMatchQueryCaseInsensitive extends CmpQuery { + static class RegexMatchQueryCaseInsensitive extends CmpQuery { @Override public Query apply(Function input, Context context) throws IOException { @@ -595,7 +603,7 @@ interface InnerFunctionToQuery { /** * for where within(shape1, shape2) = [ true | false ] */ - class WithinQuery implements FunctionToQuery, InnerFunctionToQuery { + static class WithinQuery implements FunctionToQuery, InnerFunctionToQuery { @Override public Query apply(Function parent, Function inner, Context context) throws IOException { @@ -603,7 +611,7 @@ public Query apply(Function parent, Function inner, Context context) throws IOEx if (!outerPair.isValid()) { return null; } - Query query = getQuery(inner); + Query query = getQuery(inner, context); if (query == null) return null; Boolean negate = !(Boolean) outerPair.input().value(); if (negate) { @@ -615,15 +623,18 @@ public Query apply(Function parent, Function inner, Context context) throws IOEx } } - private Query getQuery(Function inner) { + private Query getQuery(Function inner, Context context) { RefLiteralPair innerPair = new RefLiteralPair(inner); if (!innerPair.isValid()) { return null; } - GeoPointFieldMapper mapper = getGeoPointFieldMapper(innerPair.reference().info().ident().columnIdent().fqn()); + GeoPointFieldMapper mapper = getGeoPointFieldMapper( + innerPair.reference().info().ident().columnIdent().fqn(), + context.searchContext + ); Shape shape = (Shape) innerPair.input().value(); Geometry geometry = JtsSpatialContext.GEO.getGeometryFrom(shape); - IndexGeoPointFieldData fieldData = searchContext.fieldData().getForField(mapper); + IndexGeoPointFieldData fieldData = context.searchContext.fieldData().getForField(mapper); Filter filter; if (geometry.isRectangle()) { Rectangle boundingBox = shape.getBoundingBox(); @@ -641,12 +652,12 @@ private Query getQuery(Function inner) { } filter = new GeoPolygonFilter(fieldData, points); } - return new FilteredQuery(Queries.newMatchAllQuery(), indexCache.filter().cache(filter)); + return new FilteredQuery(Queries.newMatchAllQuery(), context.indexCache.filter().cache(filter)); } @Override public Query apply(Function input, Context context) throws IOException { - return getQuery(input); + return getQuery(input, context); } } @@ -680,9 +691,9 @@ public Query apply(Function parent, Function inner, Context context) { Double distance = DataTypes.DOUBLE.value(functionLiteralPair.input().value()); String fieldName = distanceRefLiteral.reference().info().ident().columnIdent().fqn(); - FieldMapper mapper = getGeoPointFieldMapper(fieldName); + FieldMapper mapper = getGeoPointFieldMapper(fieldName, context.searchContext); GeoPointFieldMapper geoMapper = ((GeoPointFieldMapper) mapper); - IndexGeoPointFieldData fieldData = searchContext.fieldData().getForField(mapper); + IndexGeoPointFieldData fieldData = context.searchContext.fieldData().getForField(mapper); Input geoPointInput = distanceRefLiteral.input(); Double[] pointValue = (Double[]) geoPointInput.value(); @@ -733,11 +744,11 @@ public Query apply(Function parent, Function inner, Context context) { fieldData, optimizeBox ); - return new FilteredQuery(Queries.newMatchAllQuery(), indexCache.filter().cache(filter)); + return new FilteredQuery(Queries.newMatchAllQuery(), context.indexCache.filter().cache(filter)); } } - private GeoPointFieldMapper getGeoPointFieldMapper(String fieldName) { + private static GeoPointFieldMapper getGeoPointFieldMapper(String fieldName, SearchContext searchContext) { MapperService.SmartNameFieldMappers smartMappers = searchContext.smartFieldMappers(fieldName); if (smartMappers == null || !smartMappers.hasMapper()) { throw new IllegalArgumentException(String.format("column \"%s\" doesn't exist", fieldName)); @@ -749,13 +760,13 @@ private GeoPointFieldMapper getGeoPointFieldMapper(String fieldName) { return (GeoPointFieldMapper) mapper; } - private final EqQuery eqQuery = new EqQuery(); - private final LtQuery ltQuery = new LtQuery(); - private final LteQuery lteQuery = new LteQuery(); - private final GtQuery gtQuery = new GtQuery(); - private final GteQuery gteQuery = new GteQuery(); - private final LikeQuery likeQuery = new LikeQuery(); - private final WithinQuery withinQuery = new WithinQuery(); + private static final EqQuery eqQuery = new EqQuery(); + private static final LtQuery ltQuery = new LtQuery(); + private static final LteQuery lteQuery = new LteQuery(); + private static final GtQuery gtQuery = new GtQuery(); + private static final GteQuery gteQuery = new GteQuery(); + private static final LikeQuery likeQuery = new LikeQuery(); + private static final WithinQuery withinQuery = new WithinQuery(); private final ImmutableMap functions = ImmutableMap.builder() .put(WithinFunction.NAME, withinQuery) @@ -799,7 +810,7 @@ public Query visitFunction(Function function, Context context) { FunctionToQuery toQuery = functions.get(function.info().ident().name()); if (toQuery == null) { - return genericFunctionQuery(function); + return genericFunctionQuery(function, context.inputSymbolVisitor, context.searchContext); } Query query; @@ -808,12 +819,12 @@ public Query visitFunction(Function function, Context context) { } catch (IOException e) { throw ExceptionsHelper.convertToRuntime(e); } catch (UnsupportedOperationException e) { - return genericFunctionQuery(function); + return genericFunctionQuery(function, context.inputSymbolVisitor, context.searchContext); } if (query == null) { query = queryFromInnerFunction(function, context); if (query == null) { - return genericFunctionQuery(function); + return genericFunctionQuery(function, context.inputSymbolVisitor, context.searchContext); } } return query; @@ -877,7 +888,9 @@ private String validateNoUnsupportedFields(Function function, Context context){ return null; } - private Filter genericFunctionFilter(Function function) { + private static Filter genericFunctionFilter(Function function, + CollectInputSymbolVisitor> inputSymbolVisitor, + SearchContext searchContext) { if (function.valueType() != DataTypes.BOOLEAN) { raiseUnsupported(function); } @@ -921,8 +934,12 @@ public DocIdSet getDocIdSet(AtomicReaderContext context, Bits acceptDocs) throws }; } - private Query genericFunctionQuery(Function function) { - return new FilteredQuery(Queries.newMatchAllQuery(), genericFunctionFilter(function)); + private static Query genericFunctionQuery(Function function, + CollectInputSymbolVisitor> inputSymbolVisitor, + SearchContext searchContext) { + return new FilteredQuery( + Queries.newMatchAllQuery(), + genericFunctionFilter(function, inputSymbolVisitor, searchContext)); } static class FunctionDocSet extends MatchDocIdSet { @@ -969,7 +986,7 @@ protected boolean matchDoc(int doc) { } } - private Query raiseUnsupported(Function function) { + private static Query raiseUnsupported(Function function) { throw new UnsupportedOperationException( SymbolFormatter.format("Cannot convert function %s into a query", function)); } diff --git a/sql/src/main/java/io/crate/operation/collect/LuceneDocCollector.java b/sql/src/main/java/io/crate/operation/collect/LuceneDocCollector.java index 3dbbcdfd3a5e..bddaba3a677e 100644 --- a/sql/src/main/java/io/crate/operation/collect/LuceneDocCollector.java +++ b/sql/src/main/java/io/crate/operation/collect/LuceneDocCollector.java @@ -66,6 +66,7 @@ public class LuceneDocCollector extends Collector implements CrateCollector, Row private boolean visitorEnabled = false; private AtomicReader currentReader; private RamAccountingContext ramAccountingContext; + private LuceneQueryBuilder luceneQueryBuilder; public static class CollectorFieldsVisitor extends FieldsVisitor { @@ -104,6 +105,7 @@ public void required(boolean required) { public LuceneDocCollector(ThreadPool threadPool, ClusterService clusterService, + LuceneQueryBuilder luceneQueryBuilder, ShardId shardId, IndexService indexService, ScriptService scriptService, @@ -115,6 +117,7 @@ public LuceneDocCollector(ThreadPool threadPool, Functions functions, WhereClause whereClause, RowDownstream downStreamProjector) throws Exception { + this.luceneQueryBuilder = luceneQueryBuilder; this.downstream = downStreamProjector.registerUpstream(this); SearchShardTarget searchShardTarget = new SearchShardTarget( clusterService.localNode().id(), shardId.getIndex(), shardId.id()); @@ -138,9 +141,8 @@ public LuceneDocCollector(ThreadPool threadPool, bigArrays, threadPool.estimatedTimeInMillisCounter() ); - LuceneQueryBuilder builder = new LuceneQueryBuilder(functions, searchContext, indexService.cache()); try { - LuceneQueryBuilder.Context ctx = builder.convert(whereClause); + LuceneQueryBuilder.Context ctx = luceneQueryBuilder.convert(whereClause, searchContext, indexService.cache()); searchContext.parsedQuery(new ParsedQuery(ctx.query(), ImmutableMap.of())); Float minScore = ctx.minScore(); if (minScore != null) { diff --git a/sql/src/main/java/io/crate/operation/collect/ShardCollectService.java b/sql/src/main/java/io/crate/operation/collect/ShardCollectService.java index ce613b6d6241..5ef3922dc89b 100644 --- a/sql/src/main/java/io/crate/operation/collect/ShardCollectService.java +++ b/sql/src/main/java/io/crate/operation/collect/ShardCollectService.java @@ -26,6 +26,7 @@ import io.crate.breaker.CrateCircuitBreakerService; import io.crate.exceptions.UnhandledServerException; import io.crate.executor.transport.TransportActionProvider; +import io.crate.lucene.LuceneQueryBuilder; import io.crate.metadata.Functions; import io.crate.metadata.shard.ShardReferenceResolver; import io.crate.metadata.shard.blob.BlobShardReferenceResolver; @@ -57,6 +58,7 @@ public class ShardCollectService { private final CollectInputSymbolVisitor docInputSymbolVisitor; private final ThreadPool threadPool; private final ClusterService clusterService; + private LuceneQueryBuilder luceneQueryBuilder; private final ShardId shardId; private final IndexService indexService; private final ScriptService scriptService; @@ -73,6 +75,7 @@ public class ShardCollectService { @Inject public ShardCollectService(ThreadPool threadPool, ClusterService clusterService, + LuceneQueryBuilder luceneQueryBuilder, Settings settings, TransportActionProvider transportActionProvider, ShardId shardId, @@ -88,6 +91,7 @@ public ShardCollectService(ThreadPool threadPool, CrateCircuitBreakerService breakerService) { this.threadPool = threadPool; this.clusterService = clusterService; + this.luceneQueryBuilder = luceneQueryBuilder; this.shardId = shardId; this.indexService = indexService; @@ -177,6 +181,7 @@ private CrateCollector getLuceneIndexCollector(CollectNode collectNode, RowDowns return new LuceneDocCollector( threadPool, clusterService, + luceneQueryBuilder, shardId, indexService, scriptService, diff --git a/sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java b/sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java index 0f0c59495292..d01f4e64d36f 100644 --- a/sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java +++ b/sql/src/test/java/io/crate/lucene/LuceneQueryBuilderTest.java @@ -60,14 +60,16 @@ public class LuceneQueryBuilderTest extends CrateUnitTest { private LuceneQueryBuilder builder; + private SearchContext searchContext; + private IndexCache indexCache; @Before public void prepare() throws Exception { Functions functions = new ModulesBuilder() .add(new OperatorModule()).createInjector().getInstance(Functions.class); - builder = new LuceneQueryBuilder(functions, - mock(SearchContext.class, Answers.RETURNS_MOCKS.get()), - mock(IndexCache.class, Answers.RETURNS_MOCKS.get())); + builder = new LuceneQueryBuilder(functions); + searchContext = mock(SearchContext.class, Answers.RETURNS_MOCKS.get()); + indexCache = mock(IndexCache.class, Answers.RETURNS_MOCKS.get()); } @Test @@ -185,7 +187,7 @@ public void testRegexQueryPcre() throws Exception { private Query convert(WhereClause clause) { - return builder.convert(clause).query; + return builder.convert(clause, searchContext, indexCache).query; } private WhereClause whereClause(String opname, Symbol left, Symbol right) {