SOLR-17736: introducing support for KNN search on nested vectors (block join) by alessandrobenedetti · Pull Request #3316 · apache/solr

alessandrobenedetti · 2025-04-08T11:28:18Z

https://issues.apache.org/jira/browse/SOLR-17736

Description

Introducing support for proper KNN search on children/parents block joins.

Solution

Surface Lucene: apache/lucene#12434

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide

liangkaiwen · 2025-04-08T14:59:39Z

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java

+              vectorField, queryVector, acceptedChildren, topK, allParentsBitSet);
+      return new ToParentBlockJoinQuery(
+          knnChildren, allParentsBitSet, ScoreModeParser.parse(scoreMode));
+    } else if (isFloatKnnQuery(childrenClauses)) {


is the branching on type of Knn query necessary? As far as I can see, the majority of the code blocks are equivalent and the different instance variables of KnnByteVectorQuery/KnnFloatVectorQuery are being used like its generic parent class

Hi @liangkaiwen, the parent class org.apache.lucene.search.AbstractKnnVectorQuery is not public and also when building the DiversifyingChildrenFloatKnnVectorQuery/DiversifyingChildrenByteKnnVectorQuery I am afraid the if branching can not be avoided?

I agree there's boring repeated code, not sure we can avoid it

Makes sense. I did not realize the parent class was package private. Maybe there's an argument to be made to make it public so as not to introduce this kind of branching everywhere, but that's a separate lucene question. This is fine

cpoerschke · 2025-04-11T12:42:09Z

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java

+      } catch (IOException e) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+      }


curious about the addition of a try/catch block here, perhaps we could have a comment w.r.t. what it might catch?

I just double checked and it's related to the searcher.rewrite and the searcher.getDocSet, they throw IOException, not sure how much we can comment here

cpoerschke · 2025-04-11T12:43:11Z

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java

+  private boolean isFloatKnnQuery(List<BooleanClause> childrenClauses) {
+    return childrenClauses.size() == 1
+        && childrenClauses.get(0).getQuery().getClass().equals(KnnFloatVectorQuery.class);
+  }
+
+  private boolean isByteKnnQuery(List<BooleanClause> childrenClauses) {
+    return childrenClauses.size() == 1
+        && childrenClauses.get(0).getQuery().getClass().equals(KnnByteVectorQuery.class);
+  }


wondering if childrenClauses.get(0).getQuery() instances KnnFloatVectorQuery and childrenClauses.get(0).getQuery() instances KnnByteVectorQuery would work too?

if true is returned then later the caller does (repeat) childrenClauses.get(0).getQuery() and type cast instead of check -- perhaps this could be encapsulated

private KnnFloatVectorQuery getFloatKnnQuery(List<BooleanClause> childrenClauses) { if (childrenClauses.size() == 1) { Query query = childrenClauses.get(0).getQuery(); if (query instanceof KnnFloatVectorQuery) { return (KnnFloatVectorQuery) query; } } return null; }

done, take a look if you like it, it may be a little bit less readable but I think it's ok!

solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java

…QParser.java Co-authored-by: Christine Poerschke <cpoerschke@apache.org>

github-actions · 2025-06-16T00:00:19Z

This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!

alessandrobenedetti · 2025-06-24T14:48:34Z

I'll resume this soon, some sponsorship should arrive and my agenda looks more relaxed starting from July

github-actions · 2025-08-24T00:00:28Z

This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!

github-actions · 2025-10-23T00:12:04Z

This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.

alessandrobenedetti · 2025-10-23T16:31:49Z

I'll resume shortly the work on this!

dsmiley · 2025-07-07T01:15:08Z

solr/solr-ref-guide/modules/query-guide/pages/searching-nested-documents.adoc

+
+This may happen, among other use cases, because you chunked the original text into paragraphs, each of them modeled as a nested document with the paragraph text and the vector representation.
+
+Solr doesn't need to have denormalised nested documents, you can still retrieve the children paragraphs by knn vector search and prefilter them using parent level metadata.


Solr doesn't need to have denormalised nested documents

That sentence fragment confuses me. What does "denomalized nested documents" mean? Do you mean simply that Solr doesn't support multiValued vector fields? If so, just say that please.

I agree 100%, I wrote it a few months ago but I don't understand it now, a clear sign that a rephrase is necessary.
Possibly I meant to do it as the work in this PR needed to be finished, anyway, I'll spend some time on it very soon, and update the PR (and comments)

dsmiley · 2025-07-07T01:16:50Z

solr/solr-ref-guide/modules/query-guide/pages/searching-nested-documents.adoc

+
+[source,text]
+----
+$ curl 'http://localhost:8983/solr/gettingstarted/select' -d 'omitHeader=true' --data-urlencode 'fq={!child of=$block_mask filters=$parentsFilter}&q={!knn f=childVectorField topK=5}[1.0,2.5,3.0...]' --data-urlencode 'block_mask=(*:* -{!prefix f="_nest_path_" v="/skus/" parentsFilter="name_s:pen"})'


A new line per param (thus end the line with \) would be helpful to make this more readable. But mostly I think this complicated query needs an english description preceding it.

+1 I'll sort this out and update the PR soon

alessandrobenedetti · 2025-11-20T15:34:13Z

I'm resuming the work on this briefly tomorrow, and then at full capacity the week of the 1st of December. Thanks, David, for the pointers. I'll carefully read them and respond soon.

# Conflicts: # solr/core/src/test/org/apache/solr/search/vector/KnnQParserChildTest.java

alessandrobenedetti · 2025-12-10T16:05:14Z

A first revised draft is fully green after the refactor following @hossman suggestions.

I'll keep it open for a few days, refine it a bit and then merge it unless critical observations!

…ck join) (#3316) * first draft * Only Nested Vectors changes * first tests draft, parent filter and children filter missing as a test * tests cleaned * code cleanup * draft documentation * tidy * tidy * add best child per document transformer * add best child per document transformer * add best child per document transformer * minor refinement to avoid some instructions * minor refactor * Update solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java Co-authored-by: Christine Poerschke <cpoerschke@apache.org> * Update solr/core/src/java/org/apache/solr/search/join/BlockJoinParentQParser.java Co-authored-by: Christine Poerschke <cpoerschke@apache.org> * new approach following feedback * tests fixed for the new approach * tidy + documentation * tidy + documentation * tidy + documentation * tidy + documentation * minor * renaming after feedback --------- Co-authored-by: Christine Poerschke <cpoerschke@apache.org> (cherry picked from commit 26457c3)

alessandrobenedetti added 7 commits April 8, 2025 11:15

first draft

95ed224

Only Nested Vectors changes

69dcae3

first tests draft, parent filter and children filter missing as a test

19ac894

tests cleaned

20792a5

code cleanup

f209d38

draft documentation

79c863b

tidy

cba5473

github-actions bot added documentation Improvements or additions to documentation tests cat:search labels Apr 8, 2025

alessandrobenedetti added the AI label Apr 8, 2025

liangkaiwen reviewed Apr 8, 2025

View reviewed changes

tidy

5f09496

liangkaiwen approved these changes Apr 9, 2025

View reviewed changes

alessandrobenedetti added 3 commits April 10, 2025 12:55

add best child per document transformer

3693141

add best child per document transformer

cdff4ad

add best child per document transformer

88a0536

cpoerschke reviewed Apr 11, 2025

View reviewed changes

alessandrobenedetti and others added 4 commits April 16, 2025 17:44

minor refinement to avoid some instructions

fcbf770

minor refactor

cae962e

Update solr/core/src/java/org/apache/solr/search/join/BlockJoinParent…

4a7fdac

…QParser.java Co-authored-by: Christine Poerschke <cpoerschke@apache.org>

Update solr/core/src/java/org/apache/solr/search/join/BlockJoinParent…

6a6aed8

…QParser.java Co-authored-by: Christine Poerschke <cpoerschke@apache.org>

github-actions bot added the stale PR not updated in 60 days label Jun 16, 2025

github-actions bot removed the stale PR not updated in 60 days label Jun 25, 2025

github-actions bot added the stale PR not updated in 60 days label Aug 24, 2025

github-actions bot added the closed-stale Closed after being stale for 60 days label Oct 23, 2025

github-actions bot closed this Oct 23, 2025

alessandrobenedetti reopened this Oct 23, 2025

github-actions bot removed closed-stale Closed after being stale for 60 days stale PR not updated in 60 days labels Oct 24, 2025

dsmiley reviewed Nov 20, 2025

View reviewed changes

alessandrobenedetti added 7 commits December 5, 2025 16:55

Merge branch 'upstreamMain' into feature/SOLR-17736

b90ab87

new approach following feedback

db232ec

tests fixed for the new approach

8c78b23

tidy + documentation

6e13aca

tidy + documentation

a2aff99

tidy + documentation

bac533e

Merge branch 'upstreamMain' into feature/SOLR-17736

3b6f0c2

# Conflicts: # solr/core/src/test/org/apache/solr/search/vector/KnnQParserChildTest.java

alessandrobenedetti added 4 commits December 10, 2025 17:30

tidy + documentation

de531d2

Merge branch 'upstreamMain' into feature/SOLR-17736

54d0ecb

minor

0e527e5

renaming after feedback

f352be9

alessandrobenedetti merged commit 26457c3 into apache:main Dec 18, 2025
5 checks passed


		This may happen, among other use cases, because you chunked the original text into paragraphs, each of them modeled as a nested document with the paragraph text and the vector representation.

		Solr doesn't need to have denormalised nested documents, you can still retrieve the children paragraphs by knn vector search and prefilter them using parent level metadata.

Conversation

alessandrobenedetti commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 16, 2025

Uh oh!

alessandrobenedetti commented Jun 24, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

github-actions bot commented Oct 23, 2025

Uh oh!

alessandrobenedetti commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alessandrobenedetti commented Nov 20, 2025

Uh oh!

alessandrobenedetti commented Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alessandrobenedetti commented Apr 8, 2025 •

edited

Loading