OAK-12360: Support per-property analyzers in Lucene index definitions - #3089
Draft
bhabegger wants to merge 10 commits into
Draft
OAK-12360: Support per-property analyzers in Lucene index definitions#3089bhabegger wants to merge 10 commits into
bhabegger wants to merge 10 commits into
Conversation
…(known limitation) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a new subsection to the Analyzers section documenting the per-property analyzer feature. Documents the syntax, backward compatibility, and explicitly calls out the two known limitations (aggregated fulltext field and regexp properties). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ertyAnalyzers The pd.isRegexp check in the main getProperties() loop could never fire — regexp property definitions are routed into IndexingRule.namePatterns, never into propDefinitions, so this branch duplicated the (correct, live) warning already emitted by the getNamePatternsProperties() pass below it. Flagged in final review as a minor; removing it since there's no reason to keep dead code around.
…yzers The existing LuceneIndexDefinitionTest coverage drives Analyzer.tokenStream() directly on the constructed Analyzer object - it verifies createAnalyzer() builds the right PerFieldAnalyzerWrapper, but never exercises the actual write path (LuceneDocumentMaker -> IndexWriter) or query path (CONTAINS() via LucenePropertyIndex). Add a test in LuceneFullTextAnalyzerTest, following the existing whitespace-tokenizer pattern in FullTextAnalyzerCommonTest, that commits real content and asserts real query results differ per property based on its declared analyzer. Verified red->green by temporarily short-circuiting collectPerPropertyAnalyzers() to return an empty map: the case-sensitive assertion on "title" failed as expected, confirming the test is genuinely sensitive to the feature rather than passing for a trivial reason.
The existing :fulltext locking test in LuceneIndexDefinitionTest only proves PerFieldAnalyzerWrapper's map never gets a :fulltext key - it never exercises the real nodeScopeIndex -> :fulltext write path. Add a test that commits real content with nodeScopeIndex + a custom per-property analyzer, and asserts jcr:contains(@title, ...) is case-sensitive (custom analyzer) while jcr:contains(., ...) - the aggregated field - stays case-insensitive (default analyzer), through the real indexing/query pipeline.
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
1 participant
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.
OAK-12360
Problem
Today, an
oak:indexLucene definition can declare a custom analyzer only underanalyzers/default. Any other analyzer configured underanalyzers/<name>is silently ignored — there's no way to apply a different tokenizer/stemmer/stopword-set to individual properties (e.g. a language-specific analyzer for one field while keeping the default for the rest of the index).Change
Adds a new optional
analyzerstring property on a property definition:referencing a sibling node
analyzers/<name>, alongside the existinganalyzers/default.LuceneIndexDefinition.createAnalyzer()builds aPerFieldAnalyzerWrapperentry for every analyzed property with a resolvinganalyzerreference, keyed by that property's actual Lucene field name (full:<pname>for index format V2+,<pname>for legacy V1) — the same nameLuceneDocumentMakerwrites documents under, so index-time and query-time (both already resolve throughLuceneIndexDefinition.getAnalyzer()) stay consistent automatically.Properties that don't set
analyzerare completely unaffected — fully backward compatible, no feature toggle needed since the change is purely additive/opt-in.Error handling: a property's
analyzerreference that doesn't resolve to an existinganalyzers/<name>node logs a warning and falls back to the default analyzer for that property only, rather than failing the index build — consistent with the existing convention for other dangling references inIndexDefinition(e.g. an aggregate rule referencing a missing property).Known limitations (explicitly out of scope for this PR)
:fulltextfield (used byCONTAINS(*, ...)) collects raw text from everynodeScopeIndex=trueproperty into one shared field, re-analyzed with a single analyzer — per-property analyzers can't differentiate text once merged into:fulltext.Both fall back to the default analyzer with a logged warning and are documented as known limitations in
oak-doc/src/site/markdown/query/lucene.md.Out of scope
oak-search-elastic) — tracked separately.Testing
New tests in
PropertyDefinitionTestandLuceneIndexDefinitionTestcover: config parsing, the happy path (custom analyzer applied to the declared property's own field while a sibling property keeps the default), a dangling analyzer reference, regexp property definitions, and the:fulltextaggregate-field limitation. Fulloak-searchandoak-lucenemodule suites pass with no regressions.Marked as draft while an internal CI pipeline runs against the branch.