You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/30 16:13:19 UTC

[GitHub] [solr] telendt opened a new pull request, #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

telendt opened a new pull request, #1051:
URL: https://github.com/apache/solr/pull/1051

   This commit removes unnecessary check for `omitNorms` from `FieldLengthFeatureScorer` that otherwise causes `NullPointerException` in `FieldLengthFeature` for non-stored or missing fields.
   
   https://issues.apache.org/jira/browse/SOLR-13219
   
   <!--
   _(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
   
   I recently encountered a problem with using `FieldLengthFeature` with non-stored fields where feature extraction/LTR reranking would result in `NullPointerException`. I check the `FieldLengthFeature` docs and briefly inspected implementation to check if for using this feature storing fields is really required. According to my expectations it's not required - the only required information are norms, that my field has enabled.
   
   I search Solr Jira and found [SOLR-13219](https://issues.apache.org/jira/browse/SOLR-13219) that seemed very similar in nature. Upon investigation I found that the two issues have indeed the same origin. The root cause seems to be the following line:
   
   ```java
   final IndexableField idxF = searcher.doc(0).getField(field);
   ```
   That returns null for non-stored missing fields.
   
   Honestly, I don't understand why `FieldLengthFeatureScorer `even checks whether the field omits norms or not. It's only used in `FieldLengthFeature#scorer` and this method already checks whether norms are there or not (and if they are missing a constant 0 ValueFeatureScorer is returned).
   
   # Solution
   
   I removed the problematic code.
   
   # Tests
   
   I modified `title` and `description` fields (`stored="false"`) in `schema.xml` proving that existing tests still work as long as the problematic code is removed (without it `TestFieldLength#testRanking` is failing). With this change I also had to slightly change `TestFeatureLogging#testGeneratedFeatures` test, but without changing its logic.
   
   I also added `TestFieldLength#testRankingDynamicField` test to prove that `TestFieldLength` works correctly with dynamic fields too, and doesn't fail if one (or more) documents from rerank candidate set lacks specific field.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [x] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] 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)
   - [x] I have developed this patch against the `main` branch.
   - [x] I have run `./gradlew check`.
   - [x] 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)
   


-- 
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


[GitHub] [solr] telendt commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
telendt commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1263766897

   @cpoerschke: I would really appreciate if you could review this one as the original author of 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


[GitHub] [solr] sonatype-lift[bot] commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1263809478

   :warning: **314 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/solr/01GE7JRX50CV71WRHRWCNXCJXP?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20solr) for more details.


-- 
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


[GitHub] [solr] cpoerschke commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
cpoerschke commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1270547422

   > Would you like to add a `solr/CHANGES.txt` entry? Tentatively in the 9.1.0 Bug Fixes section?
   
   I have asked on the [dev mailing list](https://lists.apache.org/thread/4n75lfymc3mg3r58s7974dkjlhkfwzql) w.r.t. inclusion in 9.1.0 which is currently in feature freeze. And w.r.t. the changes entry, would suggest to credit both yourself and Nick Veenhof who first reported the issue. Hope that helps.
   


-- 
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


[GitHub] [solr] telendt commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
telendt commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1270566970

   If would be wonderful if it could go to 9.1.0, that way we could probably skip patching ourselves (we need that in the place were I work). Either way, thank you for 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


[GitHub] [solr] cpoerschke merged pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
cpoerschke merged PR #1051:
URL: https://github.com/apache/solr/pull/1051


-- 
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


[GitHub] [solr] telendt commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
telendt commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1270548332

   Sure, can do. What's the rule? Should I put my name and yours as the reviewer?


-- 
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


[GitHub] [solr] telendt commented on pull request #1051: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields

Posted by GitBox <gi...@apache.org>.
telendt commented on PR #1051:
URL: https://github.com/apache/solr/pull/1051#issuecomment-1270551866

   Does it look right?


-- 
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