Add support for matched_fields with the unified highlighter #18166
Conversation
nomoa
requested review from
anasalkouz,
andrross,
ashking94,
Bukhtawar,
CEHENKLE,
cwperks,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami,
VachaShah and
a team
as code owners
github-actions
bot
added
enhancement
|
❌ Gradle check result for e9a0a85: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
from
e9a0a85 to
ad13011
Compare
|
❌ Gradle check result for ad13011: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
from
ad13011 to
6f75191
Compare
|
❌ Gradle check result for 6f75191: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Last CI failure is: Which I believe is unrelated to this patch? |
|
❌ Gradle check result for 6f75191: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
3 times, most recently
from
f7907c5 to
32061e5
Compare
|
❌ Gradle check result for 32061e5: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Failure I can see from the last build is: |
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
from
32061e5 to
a6fbda7
Compare
|
❌ Gradle check result for a6fbda7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
from
a6fbda7 to
fa503d1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18166 +/- ##
============================================
- Coverage 72.56% 72.42% -0.15%
+ Complexity 67261 67228 -33
============================================
Files 5476 5482 +6
Lines 310478 310674 +196
Branches 45133 45156 +23
============================================
- Hits 225313 224993 -320
- Misses 66840 67359 +519
+ Partials 18325 18322 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .setMapping( | ||
| XContentFactory.jsonBuilder() | ||
| .startObject() | ||
| .startObject("properties") | ||
| .startObject("foo") | ||
| .field("type", "text") | ||
| .field("store", true) | ||
| .field("analyzer", "mock_english") | ||
| .startObject("fields") | ||
| .startObject("plain") | ||
| .field("type", "text") | ||
| .field("analyzer", "standard") | ||
| .endObject() | ||
| .endObject() | ||
| .endObject() | ||
| .endObject() | ||
| .endObject() | ||
| ) |
There was a problem hiding this comment.
Super minor suggestion, more for future reference and not something you need to change...Java now allows multiline strings so you can just write regular JSON to make this a bit more readable:
| .setMapping( | |
| XContentFactory.jsonBuilder() | |
| .startObject() | |
| .startObject("properties") | |
| .startObject("foo") | |
| .field("type", "text") | |
| .field("store", true) | |
| .field("analyzer", "mock_english") | |
| .startObject("fields") | |
| .startObject("plain") | |
| .field("type", "text") | |
| .field("analyzer", "standard") | |
| .endObject() | |
| .endObject() | |
| .endObject() | |
| .endObject() | |
| .endObject() | |
| ) | |
| .setMapping(""" | |
| { | |
| "properties": { | |
| "foo": { | |
| "type": "text", | |
| "store": true | |
| "analyzer": "mock_english" | |
| "fields": { | |
| "plain": { | |
| "type": "text" | |
| "analyzer": "standard" | |
| } | |
| } | |
| } | |
| } | |
| } | |
| """) |
There was a problem hiding this comment.
good call, thanks for the suggestion, if time permits I might try to clean this whole test class in a separate PR if you find this useful.
|
is the code coverage check being green mandatory? The code is being tested in integration tests but if you prefer I can introduce a dedicated test for these lines (might need to make some methods visible for testing, unless there are some facilities to easily create/mock |
This option was silently ignored when running the unified highlighter. Since lucene 10 it is now possible to pass a list of additional fields to fetch matches from and blend into the snippets. Signed-off-by: David Causse <[email protected]>
nomoa
force-pushed
the
unified-highlighter-support-matched-fields
branch
from
fa503d1 to
4d6a2ae
Compare
|
❌ Gradle check result for 4d6a2ae: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Description
This option was silently ignored when running the unified highlighter. Since lucene 10 it is now possible to pass a list of additional fields to fetch matches from and blend into the snippets.
Related Issues
Resolves #18164
Check List
[ ] API changes companion pull request created, if applicable.