Payload null value exception handling [LUCENE-8424]

[asfimport]

If-Condition to check payload null value in PayloadScoreQuery.java was removed in #9086. Because of that, regarding of the payload value, payloadsSeen is always getting increment. This has compromised the overall score of the document(for instance, as payloadSeens is always greater than 0, docScore() in MinPayloadFunction.java only returns payloadScore which can be zero depending on the payload value). Ideally, it should've returned 1 in case of payload==null.

I have added a simple patch to get started, Please let me know if it needs any improvements.  Thanks in advance.

---

Migrated from LUCENE-8424 by Tapan Vaishnav, updated Nov 01 2018 Attachments: LUCENE-8424.patch (versions: 7)

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Thanks for opening the issue!

We can't just return immediately when the payload is null, due to #8795.  I think the correct action here is to only increment payloadsSeen if the payload is not null, but still calculate the altered score.

It would also be good to add a test to TestPayloadScoreQuery that checks behaviour when some and/or all payloads are null.

[asfimport]

Tapan Vaishnav (migrated from JIRA)

Hi @romseygeek, Thanks for your quick response. 

We can't just return immediately when the payload is null, due to LUCENE-7744.

I see, thanks for pointing it out. I will change accordingly.

It would also be good to add a test to TestPayloadScoreQuery that checks behaviour when some and/or all payloads are null.

I tried doing so in TestPayloadScoreQuery but couldn't quite figure out where to look for, I will try again to add some test cases.

[asfimport]

Tapan Vaishnav (migrated from JIRA)

Hi @romseygeek,

I have added the updated patch. After changing the payload value for the second query term as null, AveragePayloadFunction and MinPayloadFunction scores have been changed and updated accordingly. Also, the If-Condition has been put in a more appropriate position.

Please have a look and let me know if this needs any changes. Thanks in advance.

[asfimport]

Tapan Vaishnav (migrated from JIRA)

Hi @romseygeek I would really appreciate if you could provide some feedback regarding the patch. Sorry to bother you in your busy schedule.

Thanks in advance.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 16s	master passed
Patch Compile Tests
+1	compile	0m 16s	the patch passed
+1	javac	0m 16s	the patch passed
+1	Release audit (RAT)	0m 16s	the patch passed
+1	Check forbidden APIs	0m 16s	the patch passed
-1	Validate source patterns	0m 16s	Validate source patterns validate-source-patterns failed
Other Tests
+1	unit	0m 15s	queries in the patch passed.
	2m 14s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934058/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 9306922
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
Validate source patterns	https://builds.apache.org/job/PreCommit-LUCENE-Build/66/artifact/out/patch-validate-source-patterns-root.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/66/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/66/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 18s	master passed
Patch Compile Tests
+1	compile	0m 15s	the patch passed
+1	javac	0m 15s	the patch passed
+1	Release audit (RAT)	0m 15s	the patch passed
+1	Check forbidden APIs	0m 15s	the patch passed
-1	Validate source patterns	0m 15s	Validate source patterns validate-source-patterns failed
Other Tests
+1	unit	0m 14s	queries in the patch passed.
	2m 40s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934902/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / cb1db48
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
Validate source patterns	https://builds.apache.org/job/PreCommit-LUCENE-Build/69/artifact/out/patch-validate-source-patterns-root.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/69/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/69/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 16s	master passed
Patch Compile Tests
+1	compile	0m 16s	the patch passed
+1	javac	0m 16s	the patch passed
+1	Release audit (RAT)	0m 16s	the patch passed
+1	Check forbidden APIs	0m 16s	the patch passed
-1	Validate source patterns	0m 16s	Validate source patterns validate-source-patterns failed
Other Tests
+1	unit	0m 15s	queries in the patch passed.
	2m 25s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934928/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / cb1db48
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
Validate source patterns	https://builds.apache.org/job/PreCommit-LUCENE-Build/70/artifact/out/patch-validate-source-patterns-root.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/70/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/70/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 15s	master passed
Patch Compile Tests
+1	compile	0m 16s	the patch passed
+1	javac	0m 16s	the patch passed
+1	Release audit (RAT)	0m 16s	the patch passed
+1	Check forbidden APIs	0m 16s	the patch passed
-1	Validate source patterns	0m 16s	Validate source patterns validate-source-patterns failed
Other Tests
+1	unit	0m 14s	queries in the patch passed.
	2m 13s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934959/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / cb1db48
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
Validate source patterns	https://builds.apache.org/job/PreCommit-LUCENE-Build/71/artifact/out/patch-validate-source-patterns-root.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/71/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/71/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

✔ +1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 15s	master passed
Patch Compile Tests
+1	compile	0m 17s	the patch passed
+1	javac	0m 17s	the patch passed
+1	Release audit (RAT)	0m 17s	the patch passed
+1	Check forbidden APIs	0m 17s	the patch passed
+1	Validate source patterns	0m 17s	the patch passed
Other Tests
+1	unit	0m 15s	queries in the patch passed.
	2m 18s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934987/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 6c11284
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/72/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/72/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Hey Tapan Vaishnav, apologies for the delay in responding, I've been away on holiday for the past few weeks.

Having mulled this over while I've been away, I think we may be going about this the wrong way.  Rather than deciding in the parent query what to do with null payloads, instead we should allow consumers to decide this directly.  I'll upload an example patch soonish which should explain what I mean.

[asfimport]

Tapan Vaishnav (migrated from JIRA)

Hello @romseygeek, Thanks for your reply.

apologies for the delay in responding, I've been away on holiday for the past few weeks.

No worries, apologies from my side too for tagging you while you were on holidays.

Rather than deciding in the parent query what to do with null payloads, instead we should allow consumers to decide this directly.

It's only one example but if I understood correctly, handling null exception in docscore function(Consumer). https://github.com/apache/lucene-solr/blob/branch_7_3/lucene/queries/src/java/org/apache/lucene/queries/payloads/MinPayloadFunction.java#L35

`@Override`
  public float docScore(int docId, String field, int numPayloadsSeen, float payloadScore) { 
    return numPayloadsSeen > 0 ? (payloadScore >0 ? payloadScore : 1) : 1;
  }

'll upload an example patch soonish which should explain what I mean.

Got it, Thank you.

[asfimport]

Tapan Vaishnav (migrated from JIRA)

Hi @romseygeek, Do you have any updates for this issue??

[asfimport]

Lucene/Solr QA (migrated from JIRA)

✔ +1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 18s	master passed
Patch Compile Tests
+1	compile	0m 20s	the patch passed
+1	javac	0m 20s	the patch passed
+1	Release audit (RAT)	0m 20s	the patch passed
+1	Check forbidden APIs	0m 20s	the patch passed
+1	Validate source patterns	0m 20s	the patch passed
Other Tests
+1	unit	0m 16s	queries in the patch passed.
	5m 28s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8424
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12934987/LUCENE-8424.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene2-us-west.apache.org 4.4.0-112-generic #135-Ubuntu SMP Fri Jan 19 11:48:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 05f72a77
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018
Default Java	1.8.0_172
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/115/testReport/
modules	C: lucene/queries U: lucene/queries
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/115/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.