Fixed match only text block loader not working when a keyword multi field is present #134582

Kubik42 · 2025-09-11T20:51:33Z

This addresses #134097.

This PR:

fixes block loader in MatchOnlyTextFieldMapper to now correctly delegate to a keyword multi field, if one is present
adds additional test coverage around block loaders for match only text
adjusts MatchOnlyTextRollingUpgradeIT to be similar to TextRollingUpgradeIT. This means, the correct template is now used for the test + other quality of life improvements

Due to the nature of this bug fix, the BWC test MatchOnlyTextRollingUpgradeIT will start to fail if this PR is merged. This is because this bug is still present in 8.19 and 9.1. To overcome that, I've preemptively muted the test. Once this change is merged and backported into 8.19 and 9.1, I'll open another PR that unmutes the test and backport that.

…ield is present

elasticsearchmachine · 2025-09-11T20:52:29Z

Hi @Kubik42, I've created a changelog YAML for you.

...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java

server/src/test/java/org/elasticsearch/index/mapper/TextFieldTypeTests.java

Kubik42 · 2025-09-12T19:53:50Z

Upon diving a bit deeper, I found that this BWC works differently for different release versions. More specifically:

when upgrading from 8.19 / 9.1 -> 9.2 the test will fail. This happens bc of a bug described in Match only text fields don't load data correctly when there is a keyword multi field present #134097
when upgrading from 8.18 / 9.0 -> 9.2 the test will pass. This is bc those versions disregard a keyword multi field as means of reconstructing source. More specifically, in those versions, storeSource depends solely on whether synthetic source is enabled or not. As a result, parseCreateField() is actually quite simple and always produced a StoredField when synthetic source is enabled. On the contrary, in 8.19 and 9.1, we've made changes to how storeSource is set, which makes parseCreateField() vulnerable to the bug I described.

This explains why 8.18 / 9.0 -> 9.2 BWC tests pass while 8.19 / 9.1 -> 9.2 fail.

I've also checked what release versions we test against and they're as follows:

9.2 -> 9.1, 9.0, 8.19, 8.18
9.1 -> 9.0, 8.19, 8.18
9.0 -> 8.18
8.19 -> 8.18, 7.17
8.18 -> 7.17

From above, 9.0 and 8.18 don't need a backport and will work. 9.1 and 8.19 will receive a backport as I've described. This just leaves 7.17, which doesn't even support synthetic source or have block loaders, so it should pass as well.

What this ultimately means is that we don't need a version gate anywhere in this test. All we need to do is backport the fix to 9.1 and 8.19.

Kubik42 · 2025-09-13T00:15:29Z

muted-tests.yml

  method: test {csv-spec:fork.FiveFork}
  issue: https://github.com/elastic/elasticsearch/issues/134560
+- class: org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT
+  issue: https://github.com/elastic/elasticsearch/issues/134097


Preemptively muting this test as it'll fail until the backport is completed. I've ran this test locally and it passes.

Normally we don't manually mute tests.

I think the way this is normally addressed, you'd add a cluster feature representing this change (or use the built-in cluster features representing release versions), then add an assumeTrue gating the test on the presence of that feature.

elasticsearch/x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SyntheticSourceRollingUpgradeIT.java

Line 74 in 0dff3aa

assumeTrue("requires storing leaf array offsets", oldClusterHasFeature("gte_v9.1.0"));

even if this is temporary? My idea was to mute this test now, backport the fix, and then unmute the test. Without that, the test will fail against previous versions.

The convention is to use a node feature for this. Tests are muted for genuine test failures.

So let's add a node feature for this (in MapperFeatures#getTestFeatures(...)). The upside it is that the test can be back ported without having think about older versions not having this change (e.g. when changes to MatchOnlyTextRollingUpgradeIT in this PR land in 8.19 branch).

elasticsearchmachine · 2025-09-15T18:57:06Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

jordan-powers

I had a few comments, but the overall direction looks good

...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java

...-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java

jordan-powers · 2025-09-15T22:00:25Z

muted-tests.yml

  method: test {csv-spec:fork.FiveFork}
  issue: https://github.com/elastic/elasticsearch/issues/134560
+- class: org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT
+  issue: https://github.com/elastic/elasticsearch/issues/134097


Normally we don't manually mute tests.

I think the way this is normally addressed, you'd add a cluster feature representing this change (or use the built-in cluster features representing release versions), then add an assumeTrue gating the test on the presence of that feature.

elasticsearch/x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SyntheticSourceRollingUpgradeIT.java

Line 74 in 0dff3aa

assumeTrue("requires storing leaf array offsets", oldClusterHasFeature("gte_v9.1.0"));

...-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/MatchOnlyTextRollingUpgradeIT.java

…pper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]>

martijnvg

One comment around muted tests and node features, LGTM otherwise

martijnvg · 2025-09-16T11:49:52Z

muted-tests.yml

  method: test {csv-spec:fork.FiveFork}
  issue: https://github.com/elastic/elasticsearch/issues/134560
+- class: org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT
+  issue: https://github.com/elastic/elasticsearch/issues/134097


The convention is to use a node feature for this. Tests are muted for genuine test failures.

So let's add a node feature for this (in MapperFeatures#getTestFeatures(...)). The upside it is that the test can be back ported without having think about older versions not having this change (e.g. when changes to MatchOnlyTextRollingUpgradeIT in this PR land in 8.19 branch).

elasticsearchmachine · 2025-09-18T03:19:31Z

💔 Backport failed

Status	Branch	Result
❌	8.19	Commit could not be cherrypicked due to conflicts
❌	9.1	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 134582

…ield is present (elastic#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]>

…ield is present (elastic#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]> (cherry picked from commit 86227fb) # Conflicts: # server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java

…ield is present (elastic#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]> (cherry picked from commit 86227fb) # Conflicts: # modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java # qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/MatchOnlyTextRollingUpgradeIT.java # server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java

Kubik42 · 2025-09-18T17:27:51Z

💚 All backports created successfully

Status	Branch	Result
✅	9.1
✅	8.19

Questions ?

Please refer to the Backport tool documentation

…ulti field is present (#134582) (#135025) * Fixed match only text block loader not working when a keyword multi field is present (#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]> (cherry picked from commit 86227fb) # Conflicts: # server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java * Removed non existent line in 9.1

…multi field is present (#134582) (#135027) * Fixed match only text block loader not working when a keyword multi field is present (#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]> (cherry picked from commit 86227fb) # Conflicts: # modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java # qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/MatchOnlyTextRollingUpgradeIT.java # server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java * Removed BWC test since its missing way too many dependencies

Fixed match only text block loader not working when a keyword multi f…

536d196

…ield is present

Kubik42 added >bug Team:StorageEngine :StorageEngine/Mapping The storage related side of mappings labels Sep 11, 2025

Kubik42 changed the title ~~Fixed match only text block loader not working when a keyword multi f…~~ Fixed match only text block loader not working when a keyword multi field is present Sep 11, 2025

elasticsearchmachine added the v9.2.0 label Sep 11, 2025

Update docs/changelog/134582.yaml

38a1220

Kubik42 and others added 2 commits September 11, 2025 17:34

Preemptively mute this test

545d546

[CI] Auto commit changes from spotless

757a4b4

Kubik42 added auto-backport Automatically create backport pull requests when merged v8.19.5 v9.1.5 labels Sep 12, 2025

martijnvg reviewed Sep 12, 2025

View reviewed changes

...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java Outdated Show resolved Hide resolved

server/src/test/java/org/elasticsearch/index/mapper/TextFieldTypeTests.java Show resolved Hide resolved

Kubik42 and others added 2 commits September 12, 2025 13:23

Addressed feedback

fdfb970

Merge branch 'main' into 134097

a092a99

Kubik42 commented Sep 13, 2025

View reviewed changes

Merge branch 'main' into 134097

3bddfe6

Kubik42 marked this pull request as ready for review September 15, 2025 18:56

Kubik42 requested a review from jordan-powers September 15, 2025 21:34

jordan-powers reviewed Sep 15, 2025

View reviewed changes

Kubik42 and others added 4 commits September 15, 2025 15:26

Update modules/mapper-extras/src/test/java/org/elasticsearch/index/ma…

752be91

…pper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]>

Preemptively mute this test

254b1df

Fixed copyright

c4cd18b

Merge branch 'main' into 134097

280d444

martijnvg approved these changes Sep 16, 2025

View reviewed changes

Kubik42 and others added 2 commits September 17, 2025 16:34

Gate tests on feature presence

132c8e9

[CI] Auto commit changes from spotless

c3a63e6

Kubik42 and others added 2 commits September 17, 2025 16:47

Merge branch 'main' into 134097

641a16d

Revert muted-tests to main

3deefc5

Kubik42 merged commit 86227fb into elastic:main Sep 18, 2025
34 checks passed

Kubik42 deleted the 134097 branch September 18, 2025 03:18

elasticsearchmachine added the backport pending label Sep 18, 2025

Kubik42 mentioned this pull request Sep 18, 2025

[9.1] Fixed match only text block loader not working when a keyword multi field is present (#134582) #135025

Merged

Kubik42 mentioned this pull request Sep 18, 2025

[8.19] Fixed match only text block loader not working when a keyword multi field is present (#134582) #135027

Merged

Fixed match only text block loader not working when a keyword multi field is present #134582

Fixed match only text block loader not working when a keyword multi field is present #134582

Uh oh!

Conversation

Kubik42 commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 11, 2025

Uh oh!

Uh oh!

Uh oh!

Kubik42 commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kubik42 Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jordan-powers Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Kubik42 Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

martijnvg Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Sep 15, 2025

Uh oh!

jordan-powers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jordan-powers Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

martijnvg Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 18, 2025

💔 Backport failed

Uh oh!

Kubik42 commented Sep 18, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kubik42 commented Sep 11, 2025 •

edited

Loading

Kubik42 commented Sep 12, 2025 •

edited

Loading

Kubik42 Sep 13, 2025 •

edited

Loading