-
Notifications
You must be signed in to change notification settings - Fork 41
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
Create runtime fields #622
Changes from 7 commits
7e28ca4
9ef7aa3
44e5cf2
a55d178
86eda24
8a04251
75c550f
e9099ae
5dd576a
d1dccd5
dda5a8b
0dd6e80
9f47d1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,11 @@ | |
*/ | ||
package com.yelp.nrtsearch.server.luceneserver; | ||
|
||
import com.google.common.collect.Iterables; | ||
import com.google.common.collect.Lists; | ||
import com.google.protobuf.Struct; | ||
import com.google.protobuf.Struct.Builder; | ||
import com.google.protobuf.Value; | ||
import com.google.protobuf.util.JsonFormat; | ||
import com.yelp.nrtsearch.server.grpc.DeadlineUtils; | ||
import com.yelp.nrtsearch.server.grpc.FacetResult; | ||
|
@@ -37,9 +40,11 @@ | |
import com.yelp.nrtsearch.server.luceneserver.field.IndexableFieldDef; | ||
import com.yelp.nrtsearch.server.luceneserver.field.ObjectFieldDef; | ||
import com.yelp.nrtsearch.server.luceneserver.field.PolygonfieldDef; | ||
import com.yelp.nrtsearch.server.luceneserver.field.RuntimeFieldDef; | ||
import com.yelp.nrtsearch.server.luceneserver.field.VirtualFieldDef; | ||
import com.yelp.nrtsearch.server.luceneserver.innerhit.InnerHitFetchTask; | ||
import com.yelp.nrtsearch.server.luceneserver.rescore.RescoreTask; | ||
import com.yelp.nrtsearch.server.luceneserver.script.RuntimeScript; | ||
import com.yelp.nrtsearch.server.luceneserver.search.FieldFetchContext; | ||
import com.yelp.nrtsearch.server.luceneserver.search.SearchContext; | ||
import com.yelp.nrtsearch.server.luceneserver.search.SearchCutoffWrapper.CollectionTimeoutException; | ||
|
@@ -763,7 +768,6 @@ private CompositeFieldValue getFieldForHit( | |
|
||
// We detect invalid field above: | ||
assert fd != null; | ||
|
||
if (fd instanceof VirtualFieldDef) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the runtime field also need to be covered in this code path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Somehow the code never comes here when I put a breakpoint in this line. I assume it's not necessary to do that anymore. It doesn't stop here, even when I have a virtual field in the request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code path is used for doing parallel fetch by chunks of fields, instead of documents. It should probably be supported. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know how I can test that locally? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the config used for the current testing https://github.com/Yelp/nrtsearch/blob/master/src/test/java/com/yelp/nrtsearch/server/grpc/MultiSegmentParallelFieldsFetchTest.java#L25 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I was able to debug this part of the code with the above configs. It now returns the same object as the one with fetching docs. |
||
VirtualFieldDef virtualFieldDef = (VirtualFieldDef) fd; | ||
|
||
|
@@ -794,6 +798,7 @@ public boolean advanceExact(int doc) throws IOException { | |
for (int i = 0; i < docValues.size(); ++i) { | ||
compositeFieldValue.addFieldValue(docValues.toFieldValue(i)); | ||
} | ||
|
||
} | ||
// retrieve stored fields | ||
else if (fd instanceof IndexableFieldDef && ((IndexableFieldDef) fd).isStored()) { | ||
|
@@ -896,6 +901,12 @@ private static void fetchSlice( | |
sliceSegment, | ||
fieldDefEntry.getKey(), | ||
(VirtualFieldDef) fieldDefEntry.getValue()); | ||
} else if (fieldDefEntry.getValue() instanceof RuntimeFieldDef) { | ||
fetchRuntimeFromSegmentFactory( | ||
sliceHits, | ||
sliceSegment, | ||
fieldDefEntry.getKey(), | ||
(RuntimeFieldDef) fieldDefEntry.getValue()); | ||
} else if (fieldDefEntry.getValue() instanceof IndexableFieldDef) { | ||
IndexableFieldDef indexableFieldDef = (IndexableFieldDef) fieldDefEntry.getValue(); | ||
if (indexableFieldDef.hasDocValues()) { | ||
|
@@ -951,6 +962,132 @@ private static void fetchFromValueSource( | |
} | ||
} | ||
|
||
/** Fetch field value from runtime field's Object. */ | ||
private static void fetchRuntimeFromSegmentFactory( | ||
List<SearchResponse.Hit.Builder> sliceHits, | ||
LeafReaderContext sliceSegment, | ||
String name, | ||
RuntimeFieldDef runtimeFieldDef) | ||
throws IOException { | ||
RuntimeScript.SegmentFactory segmentFactory = runtimeFieldDef.getSegmentFactory(); | ||
RuntimeScript values = segmentFactory.newInstance(sliceSegment); | ||
|
||
for (SearchResponse.Hit.Builder hit : sliceHits) { | ||
int docID = hit.getLuceneDocId() - sliceSegment.docBase; | ||
// Check if the value is available for the current document | ||
if (values != null) { | ||
values.setDocId(docID); | ||
Object obj = values.execute(); | ||
SearchResponse.Hit.CompositeFieldValue.Builder compositeFieldValue = | ||
SearchResponse.Hit.CompositeFieldValue.newBuilder(); | ||
if (obj instanceof Float) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setFloatValue((Float) obj)); | ||
} else if (obj instanceof String) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setTextValue(String.valueOf(obj))); | ||
} else if (obj instanceof Double) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setDoubleValue((Double) obj)); | ||
} else if (obj instanceof Long) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setLongValue((Long) obj)); | ||
} else if (obj instanceof Integer) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setIntValue((Integer) obj)); | ||
} else if (obj instanceof Map) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder() | ||
.setStructValue(createStruct((Map<String, Object>) obj))); | ||
} else if (obj instanceof Boolean) { | ||
compositeFieldValue.addFieldValue( | ||
SearchResponse.Hit.FieldValue.newBuilder().setBooleanValue((Boolean) obj)); | ||
} else if (obj instanceof ArrayList) { | ||
vim345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SearchResponse.Hit.FieldValue.Builder fieldValueBuilder = | ||
SearchResponse.Hit.FieldValue.newBuilder(); | ||
SearchResponse.Hit.FieldValue.RepeatedFieldValues.Builder repeatedFieldValuesBuilder = | ||
SearchResponse.Hit.FieldValue.RepeatedFieldValues.newBuilder(); | ||
generateRepeatedFields(repeatedFieldValuesBuilder, obj); | ||
vim345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fieldValueBuilder.setRepeatedFieldValues(repeatedFieldValuesBuilder); | ||
compositeFieldValue.addFieldValue(fieldValueBuilder); | ||
} | ||
hit.putFields(name, compositeFieldValue.build()); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Generates list fields for all types except for lists. | ||
* | ||
* <p>TODO: Support list of lists. | ||
* | ||
* @param repeatedFieldValuesBuilder The builder for repeated field value in the search | ||
* response. | ||
* @param objs Runtime field objects that are supposed to be list. | ||
*/ | ||
private static void generateRepeatedFields( | ||
SearchResponse.Hit.FieldValue.RepeatedFieldValues.Builder repeatedFieldValuesBuilder, | ||
Object objs) { | ||
List<Struct> structObjs = new ArrayList<>(); | ||
for (Object obj : (Iterable<? extends Object>) objs) { | ||
if (obj instanceof String) { | ||
repeatedFieldValuesBuilder.addAllTextValues((Iterable<String>) objs); | ||
return; | ||
} else if (obj instanceof Integer) { | ||
repeatedFieldValuesBuilder.addAllIntValues((Iterable<Integer>) objs); | ||
return; | ||
} else if (obj instanceof Double) { | ||
repeatedFieldValuesBuilder.addAllDoubleValues((Iterable<Double>) objs); | ||
return; | ||
} else if (obj instanceof Float) { | ||
repeatedFieldValuesBuilder.addAllFloatValues((Iterable<Float>) objs); | ||
return; | ||
} else if (obj instanceof Long) { | ||
repeatedFieldValuesBuilder.addAllLongValues((Iterable<Long>) objs); | ||
return; | ||
} else if (obj instanceof Boolean) { | ||
repeatedFieldValuesBuilder.addAllBooleanValues((Iterable<Boolean>) objs); | ||
return; | ||
} else if (obj instanceof Map) { | ||
structObjs.add(createStruct((Map<String, Object>) obj)); | ||
} else { | ||
return; | ||
} | ||
} | ||
|
||
// Construct repeated structs. | ||
if (Iterables.size(structObjs) > 0) { | ||
repeatedFieldValuesBuilder.addAllStructValues(structObjs); | ||
} | ||
} | ||
|
||
/** | ||
* Create a struct for the given map based on object value type | ||
* | ||
* @param map The given map | ||
* @return Struct The protobuf representation of the map using a struct object. | ||
*/ | ||
private static Struct createStruct(Map<String, Object> map) { | ||
vim345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Builder struct = Struct.newBuilder(); | ||
for (Map.Entry<String, Object> entry : map.entrySet()) { | ||
if (entry.getValue() instanceof Number) { | ||
struct.putFields( | ||
entry.getKey(), | ||
Value.newBuilder().setNumberValue(((Number) entry.getValue()).doubleValue()).build()); | ||
} else if (entry.getValue() instanceof String) { | ||
struct.putFields( | ||
entry.getKey(), Value.newBuilder().setStringValue((String) entry.getValue()).build()); | ||
} else if (entry.getValue() instanceof Map) { | ||
struct.putFields( | ||
entry.getKey(), | ||
Value.newBuilder() | ||
.setStructValue(createStruct((Map<String, Object>) entry.getValue())) | ||
.build()); | ||
} | ||
} | ||
return struct.build(); | ||
} | ||
|
||
/** Fetch field value from its doc value */ | ||
private static void fetchFromDocVales( | ||
List<SearchResponse.Hit.Builder> sliceHits, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* Copyright 2020 Yelp Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.yelp.nrtsearch.server.luceneserver.field; | ||
|
||
import com.yelp.nrtsearch.server.luceneserver.field.IndexableFieldDef.FacetValueType; | ||
import com.yelp.nrtsearch.server.luceneserver.script.RuntimeScript; | ||
|
||
/** | ||
* Field definition for a runtime field. Runtime fields are able to produce a value for each given | ||
* index document. | ||
*/ | ||
public class RuntimeFieldDef extends FieldDef { | ||
private final RuntimeScript.SegmentFactory segmentFactory; | ||
private final IndexableFieldDef.FacetValueType facetValueType; | ||
|
||
/** | ||
* Field constructor. | ||
* | ||
* @param name name of field | ||
* @param segmentFactory lucene value source used to produce field value from documents | ||
*/ | ||
public RuntimeFieldDef(String name, RuntimeScript.SegmentFactory segmentFactory) { | ||
super(name); | ||
this.segmentFactory = segmentFactory; | ||
this.facetValueType = FacetValueType.NO_FACETS; | ||
} | ||
|
||
/** | ||
* Get value source for this field. | ||
* | ||
* @return Segment factory to create the expression. | ||
*/ | ||
public RuntimeScript.SegmentFactory getSegmentFactory() { | ||
return segmentFactory; | ||
} | ||
|
||
@Override | ||
public String getType() { | ||
return "RUNTIME"; | ||
} | ||
|
||
/** | ||
* Get the facet value type for this field. | ||
* | ||
* @return field facet value type | ||
*/ | ||
@Override | ||
public IndexableFieldDef.FacetValueType getFacetValueType() { | ||
return facetValueType; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,11 +91,14 @@ public static UpdatedFieldInfo updateFields( | |
FieldAndFacetState.Builder fieldStateBuilder = currentState.toBuilder(); | ||
List<Field> nonVirtualFields = new ArrayList<>(); | ||
List<Field> virtualFields = new ArrayList<>(); | ||
List<Field> runtimeFields = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this used anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it wasn't used. So I removed it. |
||
|
||
for (Field field : updateFields) { | ||
checkFieldName(field.getName()); | ||
if (FieldType.VIRTUAL.equals(field.getType())) { | ||
virtualFields.add(field); | ||
} else if (FieldType.RUNTIME.equals(field.getType())) { | ||
runtimeFields.add(field); | ||
} else { | ||
nonVirtualFields.add(field); | ||
} | ||
|
@@ -119,6 +122,7 @@ public static UpdatedFieldInfo updateFields( | |
parseVirtualField(field, fieldStateBuilder); | ||
newFields.put(field.getName(), field); | ||
} | ||
|
||
return new UpdatedFieldInfo(newFields, fieldStateBuilder.build()); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be easier to use the existing
ListValue
type https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/struct.proto#L92There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer. I spent a lot of time creating this and its builder methods. Using
ListValue
and the existing struct builders would have saved me a lot of time, if I had seen them earlier.