Skip to content

Commit 784e0ff

Browse files
authored
Make searcher available through DocLookup (#924)
1 parent bee362e commit 784e0ff

File tree

5 files changed

+97
-14
lines changed

5 files changed

+97
-14
lines changed

src/main/java/com/yelp/nrtsearch/server/doc/DocLookup.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Collection;
2020
import java.util.function.Function;
2121
import java.util.function.Supplier;
22+
import org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager;
2223
import org.apache.lucene.index.LeafReaderContext;
2324

2425
/**
@@ -28,6 +29,7 @@
2829
public class DocLookup {
2930
private final Function<String, FieldDef> fieldDefLookup;
3031
private final Supplier<Collection<String>> allFieldNamesSupplier;
32+
private final SearcherTaxonomyManager.SearcherAndTaxonomy searcherAndTaxonomy;
3133

3234
/**
3335
* Constructor.
@@ -37,9 +39,11 @@ public class DocLookup {
3739
*/
3840
public DocLookup(
3941
Function<String, FieldDef> fieldDefLookup,
40-
Supplier<Collection<String>> allFieldNamesSupplier) {
42+
Supplier<Collection<String>> allFieldNamesSupplier,
43+
SearcherTaxonomyManager.SearcherAndTaxonomy searcherAndTaxonomy) {
4144
this.fieldDefLookup = fieldDefLookup;
4245
this.allFieldNamesSupplier = allFieldNamesSupplier;
46+
this.searcherAndTaxonomy = searcherAndTaxonomy;
4347
}
4448

4549
/**
@@ -82,4 +86,12 @@ public FieldDef getFieldDefOrThrow(String fieldName) {
8286
public Collection<String> getAllFieldNames() {
8387
return allFieldNamesSupplier.get();
8488
}
89+
90+
/**
91+
* Get the searcher for this request, or null if unavailable. Currently, this is only available
92+
* for query supplied virtual or runtime fields.
93+
*/
94+
public SearcherTaxonomyManager.SearcherAndTaxonomy getSearcherAndTaxonomy() {
95+
return searcherAndTaxonomy;
96+
}
8597
}

src/main/java/com/yelp/nrtsearch/server/index/FieldUpdateUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public static void parseVirtualField(Field field, FieldAndFacetState.Builder fie
170170
factory.newFactory(
171171
params,
172172
new DocLookup(
173-
fieldStateBuilder.getFields()::get, fieldStateBuilder.getFields()::keySet));
173+
fieldStateBuilder.getFields()::get, fieldStateBuilder.getFields()::keySet, null));
174174

175175
FieldDef virtualFieldDef = new VirtualFieldDef(field.getName(), values);
176176
fieldStateBuilder.addField(virtualFieldDef, field);

src/main/java/com/yelp/nrtsearch/server/index/IndexState.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ public abstract class IndexState implements Closeable {
101101
/**
102102
* Index level doc values lookup. Generates {@link SegmentDocLookup} for a given lucene segment.
103103
*/
104-
public final DocLookup docLookup = new DocLookup(this::getField, () -> getAllFields().keySet());
104+
public final DocLookup docLookup =
105+
new DocLookup(this::getField, () -> getAllFields().keySet(), null);
105106

106107
/**
107108
* Holds the configuration for parallel fetch operations.

src/main/java/com/yelp/nrtsearch/server/search/SearchRequestProcessor.java

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,17 @@ public static SearchContext buildContextForRequest(
135135
.setExplain(searchRequest.getExplain())
136136
.setWarming(warming);
137137

138-
Map<String, FieldDef> queryVirtualFields = getVirtualFields(indexState, searchRequest);
139-
Map<String, FieldDef> queryRuntimeFields = getRuntimeFields(indexState, searchRequest);
138+
// We do this to make the searcher available to scripting engines, by adding it to the
139+
// DocLookup. It would be better to change the script factory interface to take a
140+
// context object containing these objects separately. We should consider making
141+
// these changes in a future minor version.
142+
DocLookup indexLookupWithSearcher =
143+
new DocLookup(
144+
indexState::getField, () -> indexState.getAllFields().keySet(), searcherAndTaxonomy);
145+
Map<String, FieldDef> queryVirtualFields =
146+
getVirtualFields(indexLookupWithSearcher, searchRequest);
147+
Map<String, FieldDef> queryRuntimeFields =
148+
getRuntimeFields(indexLookupWithSearcher, searchRequest);
140149

141150
Map<String, FieldDef> queryFields = new HashMap<>(queryVirtualFields);
142151

@@ -148,7 +157,7 @@ public static SearchContext buildContextForRequest(
148157
getRetrieveFields(searchRequest.getRetrieveFieldsList(), queryFields);
149158
contextBuilder.setRetrieveFields(Collections.unmodifiableMap(retrieveFields));
150159

151-
DocLookup docLookup = new DocLookup(queryFields::get, queryFields::keySet);
160+
DocLookup docLookup = new DocLookup(queryFields::get, queryFields::keySet, searcherAndTaxonomy);
152161
contextBuilder.setDocLookup(docLookup);
153162

154163
String rootQueryNestedPath =
@@ -363,7 +372,7 @@ private static void setVectorTotalHits(
363372
* @throws IllegalArgumentException if there are multiple virtual fields with the same name
364373
*/
365374
private static Map<String, FieldDef> getVirtualFields(
366-
IndexState indexState, SearchRequest searchRequest) {
375+
DocLookup docLookup, SearchRequest searchRequest) {
367376
if (searchRequest.getVirtualFieldsList().isEmpty()) {
368377
return new HashMap<>();
369378
}
@@ -378,7 +387,7 @@ private static Map<String, FieldDef> getVirtualFields(
378387
ScriptService.getInstance().compile(vf.getScript(), ScoreScript.CONTEXT);
379388
Map<String, Object> params = ScriptParamsUtils.decodeParams(vf.getScript().getParamsMap());
380389
FieldDef virtualField =
381-
new VirtualFieldDef(vf.getName(), factory.newFactory(params, indexState.docLookup));
390+
new VirtualFieldDef(vf.getName(), factory.newFactory(params, docLookup));
382391
virtualFields.put(vf.getName(), virtualField);
383392
}
384393
return virtualFields;
@@ -390,7 +399,7 @@ private static Map<String, FieldDef> getVirtualFields(
390399
* @throws IllegalArgumentException if there are multiple runtime fields with the same name
391400
*/
392401
private static Map<String, FieldDef> getRuntimeFields(
393-
IndexState indexState, SearchRequest searchRequest) {
402+
DocLookup docLookup, SearchRequest searchRequest) {
394403
if (searchRequest.getRuntimeFieldsList().isEmpty()) {
395404
return Map.of();
396405
}
@@ -404,8 +413,7 @@ private static Map<String, FieldDef> getRuntimeFields(
404413
RuntimeScript.Factory factory =
405414
ScriptService.getInstance().compile(vf.getScript(), RuntimeScript.CONTEXT);
406415
Map<String, Object> params = ScriptParamsUtils.decodeParams(vf.getScript().getParamsMap());
407-
RuntimeScript.SegmentFactory segmentFactory =
408-
factory.newFactory(params, indexState.docLookup);
416+
RuntimeScript.SegmentFactory segmentFactory = factory.newFactory(params, docLookup);
409417
FieldDef runtimeField = new RuntimeFieldDef(vf.getName(), segmentFactory);
410418
runtimeFields.put(vf.getName(), runtimeField);
411419
}

src/test/java/com/yelp/nrtsearch/server/doc/DocLookupTest.java

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.HashSet;
2626
import java.util.function.Function;
2727
import java.util.function.Supplier;
28+
import org.apache.lucene.facet.taxonomy.SearcherTaxonomyManager;
2829
import org.apache.lucene.index.LeafReaderContext;
2930
import org.junit.Before;
3031
import org.junit.Test;
@@ -33,6 +34,7 @@ public class DocLookupTest {
3334

3435
private Function<String, FieldDef> mockFieldDefLookup;
3536
private Supplier<Collection<String>> mockAllFieldNamesSupplier;
37+
private SearcherTaxonomyManager.SearcherAndTaxonomy mockSearcherAndTaxonomy;
3638
private FieldDef mockFieldDef1;
3739
private FieldDef mockFieldDef2;
3840
private DocLookup docLookup;
@@ -42,6 +44,7 @@ public class DocLookupTest {
4244
public void setUp() {
4345
mockFieldDefLookup = mock(Function.class);
4446
mockAllFieldNamesSupplier = mock(Supplier.class);
47+
mockSearcherAndTaxonomy = mock(SearcherTaxonomyManager.SearcherAndTaxonomy.class);
4548
mockFieldDef1 = mock(FieldDef.class);
4649
mockFieldDef2 = mock(FieldDef.class);
4750

@@ -53,7 +56,8 @@ public void setUp() {
5356
when(mockFieldDef2.getType()).thenReturn("INT");
5457
when(mockFieldDef2.getFacetValueType()).thenReturn(IndexableFieldDef.FacetValueType.NO_FACETS);
5558

56-
docLookup = new DocLookup(mockFieldDefLookup, mockAllFieldNamesSupplier);
59+
docLookup =
60+
new DocLookup(mockFieldDefLookup, mockAllFieldNamesSupplier, mockSearcherAndTaxonomy);
5761
}
5862

5963
@Test
@@ -217,7 +221,7 @@ public void testGetSegmentLookup() {
217221
public void testConstructor_NullFieldDefLookup() {
218222
// Act & Assert
219223
try {
220-
new DocLookup(null, mockAllFieldNamesSupplier);
224+
new DocLookup(null, mockAllFieldNamesSupplier, null);
221225
// Constructor doesn't validate null parameters, so this should not throw
222226
} catch (Exception e) {
223227
fail("Constructor should not throw for null fieldDefLookup: " + e.getMessage());
@@ -228,10 +232,68 @@ public void testConstructor_NullFieldDefLookup() {
228232
public void testConstructor_NullAllFieldNamesSupplier() {
229233
// Act & Assert
230234
try {
231-
new DocLookup(mockFieldDefLookup, null);
235+
new DocLookup(mockFieldDefLookup, null, null);
232236
// Constructor doesn't validate null parameters, so this should not throw
233237
} catch (Exception e) {
234238
fail("Constructor should not throw for null allFieldNamesSupplier: " + e.getMessage());
235239
}
236240
}
241+
242+
@Test
243+
public void testGetSearcherAndTaxonomy_ValidSearcherAndTaxonomy() {
244+
// Act
245+
SearcherTaxonomyManager.SearcherAndTaxonomy result = docLookup.getSearcherAndTaxonomy();
246+
247+
// Assert
248+
assertEquals(mockSearcherAndTaxonomy, result);
249+
}
250+
251+
@Test
252+
public void testGetSearcherAndTaxonomy_NullSearcherAndTaxonomy() {
253+
// Arrange
254+
DocLookup docLookupWithNull =
255+
new DocLookup(mockFieldDefLookup, mockAllFieldNamesSupplier, null);
256+
257+
// Act
258+
SearcherTaxonomyManager.SearcherAndTaxonomy result = docLookupWithNull.getSearcherAndTaxonomy();
259+
260+
// Assert
261+
assertNull(result);
262+
}
263+
264+
@Test
265+
public void testGetSearcherAndTaxonomy_MultipleCalls() {
266+
// Act
267+
SearcherTaxonomyManager.SearcherAndTaxonomy result1 = docLookup.getSearcherAndTaxonomy();
268+
SearcherTaxonomyManager.SearcherAndTaxonomy result2 = docLookup.getSearcherAndTaxonomy();
269+
270+
// Assert
271+
assertEquals(mockSearcherAndTaxonomy, result1);
272+
assertEquals(mockSearcherAndTaxonomy, result2);
273+
assertSame(result1, result2); // Should return the same instance
274+
}
275+
276+
@Test
277+
public void testConstructor_NullSearcherAndTaxonomy() {
278+
// Act & Assert
279+
try {
280+
new DocLookup(mockFieldDefLookup, mockAllFieldNamesSupplier, null);
281+
// Constructor doesn't validate null parameters, so this should not throw
282+
} catch (Exception e) {
283+
fail("Constructor should not throw for null searcherAndTaxonomy: " + e.getMessage());
284+
}
285+
}
286+
287+
@Test
288+
public void testConstructor_AllParameters() {
289+
// Act & Assert
290+
try {
291+
DocLookup testDocLookup =
292+
new DocLookup(mockFieldDefLookup, mockAllFieldNamesSupplier, mockSearcherAndTaxonomy);
293+
assertNotNull(testDocLookup);
294+
assertEquals(mockSearcherAndTaxonomy, testDocLookup.getSearcherAndTaxonomy());
295+
} catch (Exception e) {
296+
fail("Constructor should not throw for valid parameters: " + e.getMessage());
297+
}
298+
}
237299
}

0 commit comments

Comments
 (0)