OAK-12051: Fix NPE when ordering union query by jcr:score#2687
OAK-12051: Fix NPE when ordering union query by jcr:score#2687ChlineSaurus wants to merge 3 commits intoapache:trunkfrom
Conversation
| * @return the jcr:score as a double | ||
| * Precondition: {@link #isScorePresent(Query)} must be true. If the row lacks a jcr:score, 0.0 is returned and | ||
| * the issue is logged. | ||
| * @return the jcr:score as a double, or 0.0 if the row lacks a score value |
There was a problem hiding this comment.
The problem is now that the feature toggle needs to be changed. Otherwise we can never enable it, as there is no way to distinguish between the old and the new version.
There was a problem hiding this comment.
Oh, fair enough. How would you do this? Add a second feature toggle? Or is renaming the existing one enough?
There was a problem hiding this comment.
Renaming the existing one is enough.
There was a problem hiding this comment.
There is a large delay between us changing Oak and then enabling the feature toggle. Also, the changed behavior is not tested currently before the customer. I think we should try a different approach: we should enable the new behavior by default, and keep the feature toggle to be able to switch back to the old behavior where needed.
There was a problem hiding this comment.
I responded to the issues. However, I currently have problems when running the tests locally (OOM errors in Oak Segment AWS and Oak Segment Azure, it only works when running with -Dbaseline.skip=true, and even then it is sometimes flakey)
|
@ChlineSaurus could you merge the changes from trunk, and then open a PR against the OAK-12051 branch I just created? That way we will be able to run the tests. |
|
(This branch has conflicts that must be resolved) |
|
I can only run the maven build, but would like to run the Sonar workflow as well, and that can only be done if we merge it into a branch in Oak. |
ChlineSaurus
force-pushed
the
OAK-12051
branch
from
e3dbae8 to
ab14b1c
Compare
I opened a new PR from my forks trunk to the new branch you created. #2746 I closed this PR. |
Ah yes, the removed feature toggles caused merge conflicts. I now rebased everything on top of trunk and I'm currently rerunning the tests locally. |
2 participants
Fixes the NullPointerException during UnionQuery sorting by the JCR score. The problem occurs if there is a score column, but accessing a score of that column returns Null. This can happen if, for example, there is a union between three queries, of which at least one has scores, and at least one doesn't have, and those two results are merged first.
Issue: https://issues.apache.org/jira/browse/OAK-12051
New behavior: If null values occur in such a column, we replace it with a zero. The warning from the illegalArgumentException was removed, as returning 0 occurs frequently and is nothing to worry about.
Question: