SOLR-18197: Add root document query shortcut support to NestPathField#4512
SOLR-18197: Add root document query shortcut support to NestPathField#4512abumarjikar wants to merge 9 commits into
Conversation
dsmiley
left a comment
There was a problem hiding this comment.
Thanks. Looking to see this used widespread in code & docs. The goal is to ensure no user sees that ugly/confusing syntax that was previously necessary.
| @@ -0,0 +1,7 @@ | |||
| title: Add root document query shortcut support to NestPathField | |||
| type: changed | |||
There was a problem hiding this comment.
This isn't "changed" since no existing user/query is going to trigger this functionality. The functionality here is opt-in, hence "added". If you think the changelog template could be improved here, LMK.
There was a problem hiding this comment.
@dsmiley Thanks for pointing out. I will update this to added.
Regarding the changelog types— I think adding a quick cheat-sheet directly into dev-docs/changelog.adoc would be awesome for developers trying to pick between added, changed, or other on the fly.
I can open a quick separate PR to add a small reference table there if you think it's a good idea!
dsmiley
left a comment
There was a problem hiding this comment.
still definitely looking for the application of this in the codebase + doc updates
| try (SolrQueryRequest req = req("df", "_nest_path_")) { | ||
| assertQueryEquals(null, req, "{!field f=_nest_path_ v=''}", "{!field f=_nest_path_}/"); | ||
| } |
There was a problem hiding this comment.
this tests that these two are equivalent but not what they are equivalent to -- the boolean query with matchAll docs negated by an existence query
| "parent_filt", "(*:* -{!prefix f='_nest_path_' v='" + parent_path + "/'})", | ||
| "parent_filt", "({!field f='_nest_path_' v='" + parent_path + "/'})", |
There was a problem hiding this comment.
That change has a different underlying query & semantic effect. Formerly it found all docs separate from that parent path, thus matched not only the root doc but also /aa and /ff if that exists, say. You changed it to only match documents at /aa exactly.
| return params( | ||
| "q", "{!parent which=$parent_filt v=$child_q}", | ||
| "parent_filt", "(*:* -{!prefix f='_nest_path_' v='" + path + "'})", | ||
| "parent_filt", "({!field f='_nest_path_' v='" + path + "'})", |
There was a problem hiding this comment.
again, these are not equivalent
There was a problem hiding this comment.
What I meant was that you search the whole ref guide for _nest_path_ and see if that specific reference is a usage example that can be simplified due to your change. Hint: they exist. I also strongly suspect some tests are doing this but didn't check.
In the past, we sometimes add better ways to do things but didn't do the work of updating Solr to actually leverage what was added. As a consequence, that wasn't helpful: people/LLMs don't know what they don't see used.
https://issues.apache.org/jira/browse/SOLR-18197
Description
This pull request introduces a quick and intuitive shortcut to query root documents within hierarchical/nested data structures using the NestPathField. Currently, finding root-level documents (which do not have a nested path defined) requires a more verbose or explicit query syntax. This change provides a cleaner "ease of use" developer experience when dealing with nested child documents.
Solution
When a query is made against this field with an empty value, a null value, or a root slash ("/"), it automatically constructs and returns a BooleanQuery that targets only top-level root documents. It accomplishes this by matching all documents and explicitly excluding any documents where the nested path field exists:
MUST: MatchAllDocsQuery
MUST_NOT: ExistenceQuery (for the NestPathField)
For all other standard path inputs, it safely falls back to the default super.getFieldQuery(...) behavior.
Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.