You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "abumarjikar (via GitHub)" <gi...@apache.org> on 2026/06/09 07:45:15 UTC
[PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
abumarjikar opened a new pull request, #4512:
URL: https://github.com/apache/solr/pull/4512
https://issues.apache.org/jira/browse/SOLR-18197
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# 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:
- [ ] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) 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](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) 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](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
- [ ] I have added a [changelog entry](https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc) for my change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on PR #4512:
URL: https://github.com/apache/solr/pull/4512#issuecomment-4831657631
Changes Done as per the review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on PR #4512:
URL: https://github.com/apache/solr/pull/4512#issuecomment-4922522410
@dsmiley LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3503728404
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -76,6 +76,15 @@ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal
"Field " + field.getName() + " cannot be queried with a null.");
}
+ if (externalVal.contains("\\")) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Field "
+ + field.getName()
+ + " query value contains backslashes ('\\'). "
+ + "For root document queries, pass the raw single slash '/' without escaping.");
Review Comment:
Hey @dsmiley Do we want to validate this in combination.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3410860667
##########
solr/core/src/test/org/apache/solr/schema/TestNestPathField.java:
##########
Review Comment:
Changes Made
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3492107047
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -691,7 +691,9 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
} else {
return params(
"q",
- "{!parent which='(*:* -_nest_path_:*)'}(+" + inner_child_query + " +_nest_path_:*)");
+ "{!parent which='{!field f=_nest_path_ v=/}'}(+"
+ + inner_child_query
+ + " +_nest_path_:*)");
Review Comment:
we can keep this non-verbose (we went from 1 line to 3). You chose this syntax:
` {!field f=_nest_path_ v=/} `
In the Lucene syntax, this is:
`_nest_path_:\\/`
I think. I doubled the backslash to escape.
It occurred to me that it'd be nice if a backslash in the field query causes a 400 error to alert the user they did too much escaping. Such a mistake would likely go unnoticed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3500201346
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -76,6 +76,15 @@ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal
"Field " + field.getName() + " cannot be queried with a null.");
}
+ if (externalVal.contains("\\")) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Field "
+ + field.getName()
+ + " query value contains backslashes ('\\'). "
+ + "For root document queries, pass the raw single slash '/' without escaping.");
Review Comment:
Come to think of it, we could further validate that the input query value starts with a "/".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3421353997
##########
solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java:
##########
@@ -697,7 +698,7 @@ public void testBlockJoin() throws Exception {
final String parent_path = "/aa/bb";
try (SolrQueryRequest req =
req(
- "parent_filt", "(*:* -{!prefix f='_nest_path_' v='" + parent_path + "/'})",
+ "parent_filt", "({!field f='_nest_path_' v='" + parent_path + "/'})",
Review 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.
##########
solr/solr-ref-guide/modules/query-guide/pages/searching-nested-documents.adoc:
##########
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3381265315
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -62,4 +67,16 @@ public void setArgs(IndexSchema schema, Map<String, String> args) {
setIndexAnalyzer(new TokenizerChain(customAnalyzer));
// leave queryAnalyzer as literal
}
+
+ @Override
+ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal) {
+ if (externalVal == null || externalVal.isEmpty() || "/".equals(externalVal)) {
Review Comment:
I'm suspicious of null/empty. Do you think it should be implied the user means only root docs? Maybe we should just throw a SolrException BAD_REQUEST.
##########
changelog/unreleased/SOLR_18197_nest_path_easy_of_use.yml:
##########
@@ -0,0 +1,7 @@
+title: Add root document query shortcut support to NestPathField
+type: changed
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3506657480
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -76,6 +76,15 @@ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal
"Field " + field.getName() + " cannot be queried with a null.");
}
+ if (externalVal.contains("\\")) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Field "
+ + field.getName()
+ + " query value contains backslashes ('\\'). "
+ + "For root document queries, pass the raw single slash '/' without escaping.");
Review Comment:
I think so; it helps the user by letting they know they screwed up for sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3385975968
##########
changelog/unreleased/SOLR_18197_nest_path_easy_of_use.yml:
##########
@@ -0,0 +1,7 @@
+title: Add root document query shortcut support to NestPathField
+type: changed
Review 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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3411072711
##########
solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java:
##########
@@ -2000,6 +2000,13 @@ public void testHashRangeQuery() throws Exception {
"{!hash_range l='107347968' u='214695935' f='x_id'}");
}
+ @Test
+ public void testNestPathRootShortcut() throws Exception {
+ try (SolrQueryRequest req = req("df", "_nest_path_")) {
+ assertQueryEquals(null, req, "{!field f=_nest_path_ v=''}", "{!field f=_nest_path_}/");
+ }
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #4512:
URL: https://github.com/apache/solr/pull/4512
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4512:
URL: https://github.com/apache/solr/pull/4512#issuecomment-4763162039
Thanks. TestNestedUpdateProcessor contains plenty of references to the old confusing syntax that can be updated. Maybe other tests too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on PR #4512:
URL: https://github.com/apache/solr/pull/4512#issuecomment-4756937366
@dsmiley I audited the usage of `IndexSchema#NEST_PATH_FIELD_NAME ` as suggested, and here is where I updated the logic (and where I intentionally left it out):
1. Updated `buildAllParentsFilterFromPath`: > I replaced the manual, verbose `BooleanQuery` builder block for the root path (/ or empty) by directly delegating to my new `nestPathField.getType().getFieldQuery(this, nestPathField, "/") `logic. This completely removes the redundant manual construction of the "match all minus existence" pattern and unifies it under the new shortcut.
2. Left `wrapWithParentPathConstraint` mostly as-is: > I evaluated using the new `getFieldQuery` here, but for the non-`MatchAllDocsQuery` path, forcing the new method would have required nesting a structured BooleanQuery `([+MatchAllDocsQuery, -ExistenceQuery])` inside a `FILTER` or `MUST` clause. This would have introduced an unnecessary extra layer to Lucene's internal query tree structure. Keeping the `flat .add(newNestPathExistsQuery(), Occur.MUST_NOT)` here keeps the execution path flat and fast, matching Lucene's design patterns perfectly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3497334999
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -691,7 +691,9 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
} else {
return params(
"q",
- "{!parent which='(*:* -_nest_path_:*)'}(+" + inner_child_query + " +_nest_path_:*)");
+ "{!parent which='{!field f=_nest_path_ v=/}'}(+"
+ + inner_child_query
+ + " +_nest_path_:*)");
Review Comment:
Hey @dsmiley I tired to use the same in my previous tries, but it fails the test with returning 0 result with the the escaping. I will commit the other logic in next commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3385869958
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -62,4 +67,16 @@ public void setArgs(IndexSchema schema, Map<String, String> args) {
setIndexAnalyzer(new TokenizerChain(customAnalyzer));
// leave queryAnalyzer as literal
}
+
+ @Override
+ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal) {
+ if (externalVal == null || externalVal.isEmpty() || "/".equals(externalVal)) {
Review Comment:
@dsmiley My initial thought with including null and empty string checks was to cover the different ways a user might try to target the root level via LocalParams v. I was thinking about these three variations:
q={!field f=_nest_path_ v="/"} (Explicit forward-slash shortcut)
q={!field f=_nest_path_ v=""} (Empty string)
q={!field f=_nest_path_} (Omitting v entirely, evaluating to null)
I actually added the empty string check because I recalled your comment on Jira mentioning that "An empty string should be equivalent as well." >
That said, your point here about null or omitted values potentially masking client-side bugs or malformed application variables is very fair. If you feel it's safer to narrow this down strictly to "/"" and """—and throw a SolrException(ErrorCode.BAD_REQUEST, ...) for an omitted/null value—let me know! I'm happy to adjust the code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3388513894
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -62,4 +67,16 @@ public void setArgs(IndexSchema schema, Map<String, String> args) {
setIndexAnalyzer(new TokenizerChain(customAnalyzer));
// leave queryAnalyzer as literal
}
+
+ @Override
+ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal) {
+ if (externalVal == null || externalVal.isEmpty() || "/".equals(externalVal)) {
Review Comment:
Ah. Okay an empty string can be equivalent as well. But null/omitting feels more like a bug / ambiguous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3421368411
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -699,7 +699,7 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
final String path = parent_path + "/";
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 + "'})",
Review Comment:
again, these are not equivalent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3483104621
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -686,12 +686,17 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
if (verbose) {
return params(
"q", "{!parent which=$parent_filt v=$child_q}",
- "parent_filt", "(*:* -_nest_path_:*)",
+ "parent_filt", "{!field f=_nest_path_ v=$fieldQ}",
+ "fieldQ", "/",
"child_q", "(+" + inner_child_query + " +_nest_path_:*)");
} else {
return params(
"q",
- "{!parent which='(*:* -_nest_path_:*)'}(+" + inner_child_query + " +_nest_path_:*)");
+ "{!parent which='{!field f=_nest_path_ v=$fieldQ}'}(+"
+ + inner_child_query
+ + " +_nest_path_:*)",
+ "fieldQ",
+ "/");
Review Comment:
there's irony in you modifying the non-verbose code path .... to make it more verbose. surely you can keep it simple. I'll leave it as a test for you :-)
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -686,12 +686,17 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
if (verbose) {
return params(
"q", "{!parent which=$parent_filt v=$child_q}",
- "parent_filt", "(*:* -_nest_path_:*)",
+ "parent_filt", "{!field f=_nest_path_ v=$fieldQ}",
+ "fieldQ", "/",
Review Comment:
mabye leave the "verbose" codepath as-is.... it should be "verbose" after all :-)
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -781,19 +786,24 @@ private SolrParams childQueryMaker(String parent_path, String inner_parent_query
if (verbose) {
return params(
"q", "{!child of=$parent_filt v=$parent_q})",
- "parent_filt", "(*:* -_nest_path_:*)",
+ "parent_filt", "{!field f=_nest_path_ v=$fieldQ}",
+ "fieldQ", "/",
"parent_q", "(+" + inner_parent_query + " -_nest_path_:*)");
} else {
return params(
"q",
- "{!child of='(*:* -_nest_path_:*)'}(+" + inner_parent_query + " -_nest_path_:*)");
+ "{!child of='{!field f=_nest_path_ v=$fieldQ}'}(+"
+ + inner_parent_query
+ + " -_nest_path_:*)",
+ "fieldQ",
+ "/");
Review Comment:
same critique as down here and below
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3502804096
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -76,6 +76,15 @@ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal
"Field " + field.getName() + " cannot be queried with a null.");
}
+ if (externalVal.contains("\\")) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Field "
+ + field.getName()
+ + " query value contains backslashes ('\\'). "
+ + "For root document queries, pass the raw single slash '/' without escaping.");
Review Comment:
And that the value doesn't end with a "/".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3440445001
##########
solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc:
##########
@@ -558,7 +558,7 @@ Here is an example of a `knn` search using a `parents.preFilter`:
[source,text]
?q={!knn f=vector topK=3 parents.preFilter=$someParents childrenOf=$allParents}[1.0, 2.0, 3.0, 4.0]
-&allParents=*:* -_nest_path_:*
+&allParents={!field f=_nest_path_ v=''}
Review Comment:
Sure, I will update it in next commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4512:
URL: https://github.com/apache/solr/pull/4512#issuecomment-4753688928
Audit usages of `org.apache.solr.schema.IndexSchema#NEST_PATH_FIELD_NAME` and you will find code that can be updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3433225490
##########
solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc:
##########
@@ -558,7 +558,7 @@ Here is an example of a `knn` search using a `parents.preFilter`:
[source,text]
?q={!knn f=vector topK=3 parents.preFilter=$someParents childrenOf=$allParents}[1.0, 2.0, 3.0, 4.0]
-&allParents=*:* -_nest_path_:*
+&allParents={!field f=_nest_path_ v=''}
Review Comment:
Even though blank is equivalent to `/`, it's not intuitively obvious. I'd rather we simply add this one character (wherever applicable) to make it clearer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3500187537
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -76,6 +76,15 @@ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal
"Field " + field.getName() + " cannot be queried with a null.");
}
+ if (externalVal.contains("\\")) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Field "
+ + field.getName()
+ + " query value contains backslashes ('\\'). "
+ + "For root document queries, pass the raw single slash '/' without escaping.");
Review Comment:
This line (the last sentence) here doesn't seem useful to me; IMO it's somewhat a distraction. This check is not coupled to querying for the root document; it's applicable for *any* field value since *any* field value will need a forward slash (and no backward slashes). I think the beginning part of the exception message is fine. Someone seeing it will have enough information to know there's a critical problem and roughly what the issue is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3500226983
##########
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java:
##########
@@ -691,7 +691,9 @@ private SolrParams parentQueryMaker(String parent_path, String inner_child_query
} else {
return params(
"q",
- "{!parent which='(*:* -_nest_path_:*)'}(+" + inner_child_query + " +_nest_path_:*)");
+ "{!parent which='{!field f=_nest_path_ v=/}'}(+"
+ + inner_child_query
+ + " +_nest_path_:*)");
Review Comment:
hmm... a little surprising. Try quoting it instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "abumarjikar (via GitHub)" <gi...@apache.org>.
abumarjikar commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3388759195
##########
solr/core/src/java/org/apache/solr/schema/NestPathField.java:
##########
@@ -62,4 +67,16 @@ public void setArgs(IndexSchema schema, Map<String, String> args) {
setIndexAnalyzer(new TokenizerChain(customAnalyzer));
// leave queryAnalyzer as literal
}
+
+ @Override
+ public Query getFieldQuery(QParser parser, SchemaField field, String externalVal) {
+ if (externalVal == null || externalVal.isEmpty() || "/".equals(externalVal)) {
Review Comment:
ohk, then as mentioned I will throw bad request exception when its null. I will update this in upcoming commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18197: Add root document query shortcut support to NestPathField [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4512:
URL: https://github.com/apache/solr/pull/4512#discussion_r3396611319
##########
solr/core/src/test/org/apache/solr/schema/TestNestPathField.java:
##########
Review Comment:
Instead see QueryEqualityTest for query parsing/equality tests, which could be done with a single statement there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org