OAK-12051: Fix NPE when ordering union query by jcr:score#2746
Open
ChlineSaurus wants to merge 60 commits intoapache:OAK-12051from
Open
OAK-12051: Fix NPE when ordering union query by jcr:score#2746ChlineSaurus wants to merge 60 commits intoapache:OAK-12051from
ChlineSaurus wants to merge 60 commits intoapache:OAK-12051from
Conversation
…ics (apache#2626) Extended logging and error message. --------- Co-authored-by: Thomas Mueller <thomasm@apache.org>
… in reuest payloads (apache#2757)
* OAK-12072 : removed Guava's Monitor and Guard * OAK-12072 : added unit cases for ChangeProcessor class
…ll margin in verification (apache#2763)
* OAK-12010 Simplified index management (improvements) * OAK-12010 Simplified index management (improvements) * OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management
thomasmueller
requested changes
Feb 25, 2026
| PropertyValue[] values = new PropertyValue[] { | ||
| PropertyValues.newString(path), | ||
| PropertyValues.newDouble(score) | ||
| score == null ? null : PropertyValues.newDouble(score) |
Member
There was a problem hiding this comment.
This looks wrong to me. What about:
Suggested change
| score == null ? null : PropertyValues.newDouble(score) | |
| PropertyValues.newDouble(score == null ? 0.0 : score) |
Contributor
Author
There was a problem hiding this comment.
This is only for mocking behavior in tests. The goal is that it returns null, so the change would defeat the purpose in my opinion. The mock should be able to return null to test if the actual code can handle a null return.
What this (somewhat weird) line does is to ensure, that we only try to turn the value to be returned by the mock into a Double if it isn't null, with the goal that the same mocking code can be used for tests with null and actual scores.
Member
There was a problem hiding this comment.
Yes, I see, you are right! Sorry I got confused.
| } | ||
| return scoreValue.getValue(Type.DOUBLE); | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.warn("Failed to get jcr:score for path={}", row.getPath(), e); |
Contributor
Author
There was a problem hiding this comment.
Okey, I can change it. Two questions:
- Should I reduce it to an Info/Debug log (in my understanding this could happen quite often)
- Is path always given, or can it also be null (I don't want to introduce a new NPE when fixing one 😅)
Member
There was a problem hiding this comment.
If it fails quite often, then we should probably change the code so that it first checks if the column is available, in order to not throw an exception in the first place.
thomasmueller
requested changes
Feb 25, 2026
| if (scoreValue == null) { | ||
| return 0.0; | ||
| } | ||
| return scoreValue.getValue(Type.DOUBLE); |
Member
There was a problem hiding this comment.
I wonder if we should also check the type... maybe there are nodes with a "jcr:score" property but it's text? Probably not, but I guess we should be conservative.
* OAK-12112 treat DNS issue as transient --------- Co-authored-by: smiroslav <miroslav@apache.com>
… in bulk request payloads (apache#2764)
…g gc.log (apache#2776) * OAK-12121 : add regression tests for offline compaction not persisting gc.log * OAK-12121 : ignore tests until fix is applied
apache#2772) * OAK-12115 : add monitor and monitor.guard test coverage for SegmentBufferWriterPool * OAK-12115 : fixed sonar issues
…umentStoreException (apache#2771)
…c.log (apache#2779) * OAK-12119 : offline compaction does not persist compacted head into gc.log * OAK-12119 : fix cleanup fallback path and rename tests to camelCase * OAK-12119 : improve test coverage and fix static imports * OAK-12119 : remove volatile from lastCompactionResult and fix test setup
* OAK-11952: Bump up minimal Java version to 17 - wip * OAK-11952: Bump up minimal Java version to 17 * OAK-11952: Bump up minimal Java version to 17 - build.yml
…#2767) (apache#2770) Co-authored-by: Anton Hosgood <ahosgood1@icloud.com> Co-authored-by: Anton Hosgood <ahosgood@adobe.com>
* OAK-12099 build dependant moduels OAK-12099 initial version of AGENTS.md * OAK-12099 note about AGENTS.md in submodules * OAK-12099 link AGENTS.md from CLAUDE.md * Exclude AGENTS.md from the licence header check (same as CONTRIBUTING.md) * Update pom.xml * Update AGENTS.md * OAK-12099: address PR review feedback on AGENTS.md - Set test coverage requirement to >80%, 100% for security modules - Add rule: do not weaken existing tests to make builds pass - Clarify that -DskipTests should only be used for unchanged modules - Add code review reminder to Git Workflow section Made-with: Cursor * OAK-12099: add minimal-code guideline to AGENTS.md (PR review feedback) Made-with: Cursor * OAK-12099: add minimal-code guideline to AGENTS.md --------- Co-authored-by: smiroslav <miroslav@apache.com> Co-authored-by: Thomas Mueller <mueller@adobe.com> Co-authored-by: Thomas Mueller <thomasm@apache.org>
* OAK-12010 Simplified index management * OAK-12010 Simplified index management * OAK-12010 Simplified index management - improve flaky tests
Updated documentation for Composite NodeStore, including changes to section headings and clarifications on seeding and mount behavior.
Corrected headings and improved clarity in the text regarding NodeStore initialization and design limitations.
…ated (apache#2795) * OAK-12133 With the segment store, binary properties can not be aggregated * OAK-12133 With the segment store, binary properties can not be aggregated
* OAK-12136 : add AGENTS.md for oak-store-document module * OAK-12136 : exclude oak-store-document/AGENTS.md from RAT license check * OAK-12136 : use wildcard to exclude all AGENTS.md files from RAT check * OAK-12136 : exclude all AGENTS.md files from RAT license check
…pache#2801) * OAK-12139 : add Claude Code skill for oak-store-document OSGi config * OAK-12139 : move skill to oak-store-document module level * OAK-12139 : fix RAT exclusion to cover nested .claude directories * OAK-12139 : split skill into supporting files for readability * OAK-12139 : fix RAT for submodule-level .claude skills and AGENTS.md * OAK-12139 : fix skill frontmatter and gitignore trailing newline * OAK-12139 : update AGENTS.md skill reference to include direct file path
…guava (apache#2803)" This reverts commit e94ee9f. Reverted due to downstream issues, to be investigated,
…re size (apache#2808) - fix test cleanup; data files should be deleted after FileStore is closed
* OAK-12147 : introduce Oak cache API interfaces (OakCache, OakLoadingCache, OakCacheStats etc.) * OAK-12147 : convert OakCacheStats to a record * OAK-12147 : add javadocs to OakCacheStatsTest
…pache#2811) * OAK-12145 : add compatibility tests for Caffeine migration (PR 2807) Add implementation-independent test coverage for all classes affected by the Guava-to-Caffeine cache migration in OAK-11946. Tests reference only Oak-level types (CacheLIRS, CacheStats, DiffCache, etc.) so the same suite can be cherry-picked to OAK-11946 and run unchanged; any failure there is a migration compatibility gap. Modules covered: - oak-core-spi: AbstractCacheStats, CacheLIRS, EmpiricalWeigher - oak-store-document: DocumentNodeStoreBuilder, NodeDocumentCache, MemoryDiffCache, LocalDiffCache, TieredDiffCache, CachingCommitValueResolver, DocumentNodeStore, PersistentCache - oak-run-commons: DocumentNodeStoreHelper - oak-search: ExtractedTextCache (stats tracking) - oak-search-elastic: ElasticIndexStatistics (cache, refresh, failure) - oak-segment-tar: SegmentCache (loader failure contract) - oak-blob: BlobIdSet (cache miss / persistence semantics) - oak-blob-cloud: S3Backend (expiry, cache enable/disable) - oak-blob-cloud-azure: AzureBlobStoreBackend, AzureBlobStoreBackendV8 (expiry, cache enable/disable) * OAK-12145: Clarify cache compatibility test intent * OAK-12145: Rename S3 compatibility test * OAK-12145: Make cache compatibility tests behavior-based * OAK-12145: Fix SegmentCache compatibility assertion
added 4 commits
March 30, 2026 16:54
- Type check for double extraction, otherwise default to 0.0 - Log failures at warn level
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Original PR: #2687
Moved to merging from trunk to new branch to run evaluation