Skip to content

Commit f9648ff

Browse files
committed
Reverting design to use _PARENT notation
1 parent 8b39e35 commit f9648ff

File tree

5 files changed

+114
-219
lines changed

5 files changed

+114
-219
lines changed

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,9 @@
2525
*/
2626
public class DocLookup {
2727
private final Function<String, FieldDef> fieldDefLookup;
28-
private final String queryNestedPath;
2928

3029
public DocLookup(Function<String, FieldDef> fieldDefLookup) {
31-
this(fieldDefLookup, null);
32-
}
33-
34-
public DocLookup(Function<String, FieldDef> fieldDefLookup, String queryNestedPath) {
3530
this.fieldDefLookup = fieldDefLookup;
36-
this.queryNestedPath = queryNestedPath;
3731
}
3832

3933
/**
@@ -43,7 +37,7 @@ public DocLookup(Function<String, FieldDef> fieldDefLookup, String queryNestedPa
4337
* @return lookup accessor for given segment context
4438
*/
4539
public SegmentDocLookup getSegmentLookup(LeafReaderContext context) {
46-
return new SegmentDocLookup(fieldDefLookup, context, queryNestedPath);
40+
return new SegmentDocLookup(fieldDefLookup, context);
4741
}
4842

4943
/**

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

Lines changed: 15 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,14 @@ public class SegmentDocLookup implements Map<String, LoadedDocValues<?>> {
3939
private final Function<String, FieldDef> fieldDefLookup;
4040
private final LeafReaderContext context;
4141
private final Map<String, LoadedDocValues<?>> loaderCache = new HashMap<>();
42-
private final String queryNestedPath;
4342

4443
private int docId = -1;
4544
private int parentDocId = -1;
4645
private SegmentDocLookup parentLookup = null;
4746

4847
public SegmentDocLookup(Function<String, FieldDef> fieldDefLookup, LeafReaderContext context) {
49-
this(fieldDefLookup, context, null);
50-
}
51-
52-
public SegmentDocLookup(
53-
Function<String, FieldDef> fieldDefLookup,
54-
LeafReaderContext context,
55-
String queryNestedPath) {
5648
this.fieldDefLookup = fieldDefLookup;
5749
this.context = context;
58-
this.queryNestedPath = queryNestedPath;
5950
}
6051

6152
/**
@@ -82,7 +73,7 @@ public boolean isEmpty() {
8273

8374
/**
8475
* Check if a given field name is capable of having doc values. This does not mean there is data
85-
* present, just that there can be.
76+
* present, just that there can be. Handles "_PARENT." prefix for parent field access.
8677
*
8778
* @param key field name
8879
* @return if this field may have stored doc values
@@ -94,6 +85,10 @@ public boolean containsKey(Object key) {
9485
}
9586
String fieldName = key.toString();
9687

88+
if (fieldName.startsWith("_PARENT.")) {
89+
fieldName = fieldName.substring("_PARENT.".length());
90+
}
91+
9792
try {
9893
FieldDef field = fieldDefLookup.apply(fieldName);
9994
return field instanceof IndexableFieldDef && ((IndexableFieldDef<?>) field).hasDocValues();
@@ -112,7 +107,7 @@ public boolean containsValue(Object value) {
112107
* cache. The data is loaded for the current set document id.
113108
*
114109
* <p>The system automatically determines if a field requires parent document access based on the
115-
* current nested path. Fields are resolved automatically without requiring explicit notation.
110+
* "_PARENT." prefix in the field name. Fields with this prefix will access the parent document.
116111
*
117112
* @param key field name
118113
* @return {@link LoadedDocValues} implementation for the given field
@@ -125,18 +120,16 @@ public LoadedDocValues<?> get(Object key) {
125120
Objects.requireNonNull(key);
126121
String fieldName = key.toString();
127122

128-
if ((!IndexState.ROOT.equals(queryNestedPath))) {
129-
int parentLevels = computeParentLevels(queryNestedPath, fieldName);
130-
if (parentLevels > 0) {
131-
SegmentDocLookup currentLookup = getParentLookup();
132-
if (currentLookup == null) {
133-
throw new IllegalArgumentException(
134-
"Could not access parent field: "
135-
+ fieldName
136-
+ " (document may not be nested or parent field may not exist)");
137-
}
138-
return currentLookup.get(fieldName);
123+
if (fieldName.startsWith("_PARENT.")) {
124+
String actualFieldName = fieldName.substring("_PARENT.".length());
125+
SegmentDocLookup parentLookup = getParentLookup();
126+
if (parentLookup == null) {
127+
throw new IllegalArgumentException(
128+
"Could not access parent field: "
129+
+ fieldName
130+
+ " (document may not be nested or parent field may not exist)");
139131
}
132+
return parentLookup.get(actualFieldName);
140133
}
141134

142135
LoadedDocValues<?> docValues = loaderCache.get(fieldName);
@@ -222,52 +215,6 @@ private int getParentDocId() {
222215
return docId + offset;
223216
}
224217

225-
/** Computes the number of parent levels to traverse to access a field. */
226-
private static int computeParentLevels(String currentNestedPath, String targetFieldPath) {
227-
if (currentNestedPath == null
228-
|| currentNestedPath.isEmpty()
229-
|| IndexState.ROOT.equals(currentNestedPath)) {
230-
return -1; // Field is at current level or below
231-
}
232-
233-
if (targetFieldPath.startsWith(currentNestedPath + ".")
234-
|| targetFieldPath.equals(currentNestedPath)) {
235-
return -1; // Field is at current level or below
236-
}
237-
238-
String[] currentPathParts = currentNestedPath.split("\\.");
239-
String[] targetPathParts = targetFieldPath.split("\\.");
240-
241-
// Find common prefix
242-
int commonPrefixLength = 0;
243-
int minLength = Math.min(currentPathParts.length, targetPathParts.length);
244-
for (int i = 0; i < minLength; i++) {
245-
if (currentPathParts[i].equals(targetPathParts[i])) {
246-
commonPrefixLength++;
247-
} else {
248-
break;
249-
}
250-
}
251-
252-
int levelsUp = currentPathParts.length - commonPrefixLength;
253-
254-
// TODO - Enable support for more than one level of nesting
255-
if (levelsUp > 1) {
256-
throw new IllegalArgumentException(
257-
"Field access requires "
258-
+ levelsUp
259-
+ " parent levels, but only 1 level of nesting is supported. "
260-
+ "Field: "
261-
+ targetFieldPath
262-
+ ", Current path: "
263-
+ currentNestedPath);
264-
}
265-
266-
// If we need to go up to access the field, return the number of levels
267-
// If levelsUp is 0, it means the field is at the same level or below
268-
return levelsUp > 0 ? levelsUp : -1;
269-
}
270-
271218
@Override
272219
public LoadedDocValues<?> put(String key, LoadedDocValues<?> value) {
273220
throw new UnsupportedOperationException();

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

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -134,36 +134,32 @@ public static SearchContext buildContextForRequest(
134134
.setExplain(searchRequest.getExplain())
135135
.setWarming(warming);
136136

137-
String rootQueryNestedPath =
138-
indexState.resolveQueryNestedPath(searchRequest.getQueryNestedPath());
139-
contextBuilder.setQueryNestedPath(rootQueryNestedPath);
140-
141-
// First, create a basic queryFields map with index fields only
142-
Map<String, FieldDef> queryFields = new HashMap<>();
143-
addIndexFields(indexState, queryFields);
144-
145-
// Create DocLookup with nested path for use by virtual/runtime fields
146-
DocLookup docLookup = new DocLookup(queryFields::get, rootQueryNestedPath);
147-
contextBuilder.setDocLookup(docLookup);
137+
Map<String, FieldDef> queryVirtualFields = getVirtualFields(indexState, searchRequest);
138+
Map<String, FieldDef> queryRuntimeFields = getRuntimeFields(indexState, searchRequest);
148139

149-
// Now add virtual and runtime fields that can use the nested-path-aware DocLookup
150-
Map<String, FieldDef> queryVirtualFields =
151-
getVirtualFields(indexState, searchRequest, docLookup);
152-
Map<String, FieldDef> queryRuntimeFields =
153-
getRuntimeFields(indexState, searchRequest, docLookup);
140+
Map<String, FieldDef> queryFields = new HashMap<>(queryVirtualFields);
154141

155-
addToQueryFields(queryFields, queryVirtualFields);
156142
addToQueryFields(queryFields, queryRuntimeFields);
143+
addIndexFields(indexState, queryFields);
157144
contextBuilder.setQueryFields(Collections.unmodifiableMap(queryFields));
158145

159146
Map<String, FieldDef> retrieveFields =
160147
getRetrieveFields(searchRequest.getRetrieveFieldsList(), queryFields);
161148
contextBuilder.setRetrieveFields(Collections.unmodifiableMap(retrieveFields));
162149

163-
String queryText = searchRequest.getQueryText();
150+
DocLookup docLookup = new DocLookup(queryFields::get);
151+
contextBuilder.setDocLookup(docLookup);
152+
153+
String rootQueryNestedPath =
154+
indexState.resolveQueryNestedPath(searchRequest.getQueryNestedPath());
155+
contextBuilder.setQueryNestedPath(rootQueryNestedPath);
164156
Query query =
165157
extractQuery(
166-
indexState, queryText, searchRequest.getQuery(), rootQueryNestedPath, docLookup);
158+
indexState,
159+
searchRequest.getQueryText(),
160+
searchRequest.getQuery(),
161+
rootQueryNestedPath,
162+
docLookup);
167163
if (profileResult != null) {
168164
profileResult.setParsedQuery(query.toString());
169165
}
@@ -356,7 +352,7 @@ private static Query resolveKnnQueryAndBoost(
356352
* @throws IllegalArgumentException if there are multiple virtual fields with the same name
357353
*/
358354
private static Map<String, FieldDef> getVirtualFields(
359-
IndexState indexState, SearchRequest searchRequest, DocLookup docLookup) {
355+
IndexState indexState, SearchRequest searchRequest) {
360356
if (searchRequest.getVirtualFieldsList().isEmpty()) {
361357
return new HashMap<>();
362358
}
@@ -371,7 +367,7 @@ private static Map<String, FieldDef> getVirtualFields(
371367
ScriptService.getInstance().compile(vf.getScript(), ScoreScript.CONTEXT);
372368
Map<String, Object> params = ScriptParamsUtils.decodeParams(vf.getScript().getParamsMap());
373369
FieldDef virtualField =
374-
new VirtualFieldDef(vf.getName(), factory.newFactory(params, docLookup));
370+
new VirtualFieldDef(vf.getName(), factory.newFactory(params, indexState.docLookup));
375371
virtualFields.put(vf.getName(), virtualField);
376372
}
377373
return virtualFields;
@@ -383,7 +379,7 @@ private static Map<String, FieldDef> getVirtualFields(
383379
* @throws IllegalArgumentException if there are multiple runtime fields with the same name
384380
*/
385381
private static Map<String, FieldDef> getRuntimeFields(
386-
IndexState indexState, SearchRequest searchRequest, DocLookup docLookup) {
382+
IndexState indexState, SearchRequest searchRequest) {
387383
if (searchRequest.getRuntimeFieldsList().isEmpty()) {
388384
return Map.of();
389385
}
@@ -397,7 +393,8 @@ private static Map<String, FieldDef> getRuntimeFields(
397393
RuntimeScript.Factory factory =
398394
ScriptService.getInstance().compile(vf.getScript(), RuntimeScript.CONTEXT);
399395
Map<String, Object> params = ScriptParamsUtils.decodeParams(vf.getScript().getParamsMap());
400-
RuntimeScript.SegmentFactory segmentFactory = factory.newFactory(params, docLookup);
396+
RuntimeScript.SegmentFactory segmentFactory =
397+
factory.newFactory(params, indexState.docLookup);
401398
FieldDef runtimeField = new RuntimeFieldDef(vf.getName(), segmentFactory);
402399
runtimeFields.put(vf.getName(), runtimeField);
403400
}

0 commit comments

Comments
 (0)