Skip to content
Draft
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 @@ -68,6 +68,7 @@ private static class NestedUpdateProcessor extends UpdateRequestProcessor {
private static final String SINGULAR_VALUE_CHAR = "";
private boolean storePath;
private boolean storeParent;
private final boolean hasMultiValuedVectorField;
private String uniqueKeyFieldName;
private IndexSchema schema;

Expand All @@ -78,87 +79,128 @@ private static class NestedUpdateProcessor extends UpdateRequestProcessor {
this.storePath = storePath;
this.uniqueKeyFieldName = req.getSchema().getUniqueKeyField().getName();
this.schema = req.getSchema();
this.hasMultiValuedVectorField = hasMultiValuedVectorField(schema);
}

/** Whether any field, explicit or dynamic, could yield vectors to split into nested docs. */
private static boolean hasMultiValuedVectorField(IndexSchema schema) {
for (SchemaField field : schema.getFields().values()) {
if (isMultiValuedVectorField(field)) {
return true;
}
}
for (IndexSchema.DynamicField dynamicField : schema.getDynamicFields()) {
if (isMultiValuedVectorField(dynamicField.getPrototype())) {
return true;
}
}
return false;
}

private static boolean isMultiValuedVectorField(SchemaField sfield) {
return sfield.getType() instanceof DenseVectorField && sfield.multiValued();
}

@Override
public void processAdd(AddUpdateCommand cmd) throws IOException {
SolrInputDocument doc = cmd.getSolrInputDocument();
processDocChildren(doc, null);
final String rootPath = rootPathPrefix(doc);
processDocChildren(doc, rootPath);
if (hasMultiValuedVectorField) {
// after the children; the docs it generates must not be walked as children themselves
processMultiValuedVectorFields(doc, rootPath);
}
Comment on lines 105 to +112

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alessandrobenedetti , I see you added multi-valued vector field detection to this class. You added logic to processDocChildren so that only at the root level, it did it's work. I found this made processDocChildren much longer and somewhat more complex than if it was a separate pass scoped to this. So I did that here and I'd like your opinion. My only concern is that we lookup the SchemaField twice per root doc field instead of once. Probably a minor concern but still. I added a boolean hasMultiValuedVectorField to this URP to pre-compute wether it's impossible or not.

I suppose it'd be handy if SolrInputField was expanded to also include the SchemaField so that any processing relative to the schema by any URP + final Document building does a lookup just once.

super.processAdd(cmd);
}

private boolean processDocChildren(SolrInputDocument doc, String fullPath) {
boolean isNested = false;
/**
* A nest path already on the root doc prefixes its children's, e.g. {@code /myRoot} yields
* {@code /myRoot/children#0}. Null when absent or at the top, else without a trailing '/'.
* Disclaimer: for advanced/expert usage.
*/
private String rootPathPrefix(SolrInputDocument rootDoc) {
Object value = rootDoc.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME);
if (value == null) {
return null;
}
String path = value.toString();
if (path.endsWith(PATH_SEP_CHAR)) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Bad nest path field format");
}
return path;
}

private void processDocChildren(SolrInputDocument doc, String fullPath) {
for (SolrInputField field : doc.values()) {
int childNum = 0;
boolean isSingleVal = !(field.getValue() instanceof Collection);
for (Object val : field) {
if (!(val instanceof SolrInputDocument cDoc)) {
// either all collection items are child docs or none are.
break;
}
final String fieldName = field.getName();

if (fieldName.contains(PATH_SEP_CHAR)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"Field name: '"
+ fieldName
+ "' contains: '"
+ PATH_SEP_CHAR
+ "' , which is reserved for the nested URP");
}
final String sChildNum = isSingleVal ? SINGULAR_VALUE_CHAR : String.valueOf(childNum);
if (!cDoc.containsKey(uniqueKeyFieldName)) {
String parentDocId = doc.getField(uniqueKeyFieldName).getFirstValue().toString();
cDoc.setField(
uniqueKeyFieldName, generateChildUniqueId(parentDocId, fieldName, sChildNum));
}
final String lastKeyPath = PATH_SEP_CHAR + fieldName + NUM_SEP_CHAR + sChildNum;
// concat of all paths children.grandChild => /children#1/grandChild#
final String childDocPath = fullPath == null ? lastKeyPath : fullPath + lastKeyPath;
processChildDoc(cDoc, doc, childDocPath);
++childNum;
}
}
}

/** Replaces each multi-valued vector field on the root doc with a nested doc per value. */
private void processMultiValuedVectorFields(SolrInputDocument doc, String fullPath) {
List<String> originalVectorFieldsToRemove = new ArrayList<>();
ArrayList<SolrInputDocument> vectors = new ArrayList<>();
List<SolrInputDocument> vectors = new ArrayList<>();
for (SolrInputField field : doc.values()) {
SchemaField sfield = schema.getFieldOrNull(field.getName());
if (sfield == null || !isMultiValuedVectorField(sfield)) {
continue;
}
int childNum = 0;
boolean isSingleVal = !(field.getValue() instanceof Collection);
boolean firstLevelChildren = fullPath == null;
if (firstLevelChildren && sfield != null && isMultiValuedVectorField(sfield)) {
for (Object vectorValue : field.getValues()) {
SolrInputDocument singleVectorNestedDoc = new SolrInputDocument();
singleVectorNestedDoc.setField(field.getName(), vectorValue);
final String sChildNum = isSingleVal ? SINGULAR_VALUE_CHAR : String.valueOf(childNum);
String parentDocId = doc.getField(uniqueKeyFieldName).getFirstValue().toString();
singleVectorNestedDoc.setField(
uniqueKeyFieldName, generateChildUniqueId(parentDocId, field.getName(), sChildNum));

if (!isNested) {
isNested = true;
}
final String lastKeyPath = PATH_SEP_CHAR + field.getName() + NUM_SEP_CHAR + sChildNum;
final String childDocPath = firstLevelChildren ? lastKeyPath : fullPath + lastKeyPath;
if (storePath) {
setPathField(singleVectorNestedDoc, childDocPath);
}
if (storeParent) {
setParentKey(singleVectorNestedDoc, doc);
}
++childNum;
vectors.add(singleVectorNestedDoc);
for (Object vectorValue : field.getValues()) {
SolrInputDocument singleVectorNestedDoc = new SolrInputDocument();
singleVectorNestedDoc.setField(field.getName(), vectorValue);
final String sChildNum = isSingleVal ? SINGULAR_VALUE_CHAR : String.valueOf(childNum);
String parentDocId = doc.getField(uniqueKeyFieldName).getFirstValue().toString();
singleVectorNestedDoc.setField(
uniqueKeyFieldName, generateChildUniqueId(parentDocId, field.getName(), sChildNum));

final String lastKeyPath = PATH_SEP_CHAR + field.getName() + NUM_SEP_CHAR + sChildNum;
final String childDocPath = fullPath == null ? lastKeyPath : fullPath + lastKeyPath;
if (storePath) {
setPathField(singleVectorNestedDoc, childDocPath);
}
originalVectorFieldsToRemove.add(field.getName());
} else {
for (Object val : field) {
if (!(val instanceof SolrInputDocument cDoc)) {
// either all collection items are child docs or none are.
break;
}
final String fieldName = field.getName();

if (fieldName.contains(PATH_SEP_CHAR)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"Field name: '"
+ fieldName
+ "' contains: '"
+ PATH_SEP_CHAR
+ "' , which is reserved for the nested URP");
}
final String sChildNum = isSingleVal ? SINGULAR_VALUE_CHAR : String.valueOf(childNum);
if (!cDoc.containsKey(uniqueKeyFieldName)) {
String parentDocId = doc.getField(uniqueKeyFieldName).getFirstValue().toString();
cDoc.setField(
uniqueKeyFieldName, generateChildUniqueId(parentDocId, fieldName, sChildNum));
}
if (!isNested) {
isNested = true;
}
final String lastKeyPath = PATH_SEP_CHAR + fieldName + NUM_SEP_CHAR + sChildNum;
// concat of all paths children.grandChild => /children#1/grandChild#
final String childDocPath = firstLevelChildren ? lastKeyPath : fullPath + lastKeyPath;
processChildDoc(cDoc, doc, childDocPath);
++childNum;
if (storeParent) {
setParentKey(singleVectorNestedDoc, doc);
}
++childNum;
vectors.add(singleVectorNestedDoc);
}
originalVectorFieldsToRemove.add(field.getName());
}
this.cleanOriginalVectorFields(doc, originalVectorFieldsToRemove);
if (vectors.size() > 0) {
if (!vectors.isEmpty()) {
doc.setField(NESTED_VECTORS_PSEUDO_FIELD_NAME, vectors);
}
return isNested;
}

private void cleanOriginalVectorFields(
Expand All @@ -168,10 +210,6 @@ private void cleanOriginalVectorFields(
}
}

private static boolean isMultiValuedVectorField(SchemaField sfield) {
return sfield.getType() instanceof DenseVectorField && sfield.multiValued();
}

private void processChildDoc(
SolrInputDocument child, SolrInputDocument parent, String fullPath) {
if (storePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,36 @@ public void testDeeplyNestedURPSanity() throws Exception {
singularChild.toString());
}

/** Children extend a nest path the root doc already carries. */
@Test
public void testRootPathIsHonored() throws Exception {
SolrInputDocument doc =
sdoc(
"id",
"1",
"children",
sdocs(
sdoc("id", "2", "name_s", "Yaz", "grandChild", sdoc("id", "3", "name_s", "Gaz"))));
doc.setField(IndexSchema.NEST_PATH_FIELD_NAME, "/myRoot");

AddUpdateCommand cmd = new AddUpdateCommand(req());
cmd.solrDoc = doc;
new NestedUpdateProcessorFactory().getInstance(req(), null, null).processAdd(cmd);

assertEquals(
"the root's own path is left alone",
"/myRoot",
doc.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME));

SolrInputDocument child = (SolrInputDocument) doc.get("children").getFirstValue();
assertEquals("/myRoot/children#0", child.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME));

SolrInputDocument grandChild = (SolrInputDocument) child.get("grandChild").getValue();
assertEquals(
"/myRoot/children#0/grandChild#",
grandChild.getFieldValue(IndexSchema.NEST_PATH_FIELD_NAME));
}

@Test
public void testDeeplyNestedURPChildrenWoId() throws Exception {
final String rootId = "1";
Expand Down Expand Up @@ -277,6 +307,25 @@ private void indexSampleData(String cmd) throws Exception {
assertU(commit());
}

/** Adding a doc deletes every existing doc sharing its {@code _root_}, not just its own id. */
@Test
public void testAddDeletesEntireRootBlock() {
SolrInputDocument first = sdoc("id", "1", "name_s", "first");
first.setField(IndexSchema.ROOT_FIELD_NAME, "77");
assertU(adoc(first));

SolrInputDocument second = sdoc("id", "2", "name_s", "second");
second.setField(IndexSchema.ROOT_FIELD_NAME, "77");
assertU(adoc(second));
assertU(commit());

assertQ(
"adding doc 2 deleted doc 1, since they share a _root_",
req("q", IndexSchema.ROOT_FIELD_NAME + ":77", "fl", "id"),
"//*[@numFound='1']",
"//doc/str[@name='id']='2'");
}

/** Test the {@code filters} local-param works with {@code parentPath}. */
@Test
public void testFiltersWithParentPath() {
Expand Down
Loading