Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading #134253

Kubik42 · 2025-09-05T21:20:55Z

This addresses #134096

More specifically, this PR does the following:

refactors how ignore_above is initialized. More specifically, FieldTypes and FieldMappers no longer have a simple integer representing ignore_above as thats been proved to be error prone. For example, in Text fields incorrectly fallback to source in block loader for logsdb indices #134096 we checked the wrong default value. With this change, ignore_above is now represented by a new class IgnoreAbove, which doesn't allow the setting of the default value and instead infers it from IndexMode and IndexVersion.
introduces IgnoreAbove class, which provides nice utility functions like isIgnored() and isSet(), which are reused across several field mappers. This eliminates the need to save ignoreAboutDefault and removes duplicate code code blocks such as string.length() > ignoreAbove
introduces TextRollingUpgradeIT, which is a nearly identical copy of MatchOnlyTextRollingUpgradeIT. This BWC test was previously missing, hence its addition. The addition of this test also helps validate Don't store keyword multi fields when they trip ignore_above #132962
adds some unit test coverage, especially around the use of syntheticSourceDelegate inside of blockLoader(). See my comments for more details

elasticsearchmachine · 2025-09-05T21:21:41Z

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

Kubik42 · 2025-09-05T23:25:07Z

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

        }

+        public boolean isIgnoreAboveSet() {
+            return ignoreAbove != ignoreAboveDefaultValue;


I'm not a huge fan of storing ignoreAboveDefaultValue, but its better than the alternatives:

add indexMode and indexCreatedVersion as class fields in KeywrodFieldType and then call getIgnoreAboveDefaultValue() every time isIgnoreAboveSet() is called

pass indexMode and indexCreatedVersion as parameters into isIgnoreAboveSet()

decide whether ignore_above is set in the constructor; ie. have an extra field called isIgnoreAboveSet at the class level - this is error prone as it must be manually set to false in 3/5 constructors that KeywordFieldType has.

Perhaps we could revisit the default value for ignore above in the future and just default everything to one value rather than picking and choosing based on the index mode, but thats out of scope for this change.

I don't think keeping that of default ignored above isn't too bad here. I think it is better than the alternative.

Perhaps we could revisit the default value for ignore above in the future and just default everything to one value rather than picking and choosing based on the index mode, but thats out of scope for this change.

Also that would require more input from other teams.

Kubik42 · 2025-09-05T23:27:30Z

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

        private final FieldValues<String> scriptValues;
        private final boolean isDimension;
        private final boolean isSyntheticSource;
-        private final IndexMode indexMode;


removed - wasn't being used anywhere

Kubik42 · 2025-09-05T23:33:28Z

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

+    /**
+     * Generates a string containing a random number of random length alphas, all delimited by space.
+     */
+    public static String randomAlphasDelimitedBySpace(int maxAlphas, int minCodeUnits, int maxCodeUnits) {


this helps with BWC tests by providing randomized strings that are more accurate to what customers would use; ie. space separated tokens

Let's keep this in the TextRollingUpgradeIT? This is the only user for now.

elasticsearchmachine · 2025-09-05T23:36:15Z

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

elasticsearchmachine · 2025-09-05T23:37:48Z

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

martijnvg · 2025-09-08T07:55:49Z

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

        }

+        public boolean isIgnoreAboveSet() {
+            return ignoreAbove != ignoreAboveDefaultValue;


I don't think keeping that of default ignored above isn't too bad here. I think it is better than the alternative.

martijnvg · 2025-09-08T07:56:24Z

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

        }

+        public boolean isIgnoreAboveSet() {
+            return ignoreAbove != ignoreAboveDefaultValue;


Perhaps we could revisit the default value for ignore above in the future and just default everything to one value rather than picking and choosing based on the index mode, but thats out of scope for this change.

Also that would require more input from other teams.

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

elasticsearchmachine · 2025-09-08T22:01:55Z

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

Kubik42 · 2025-09-09T01:21:20Z

Update: so while I was brainstorming for how to cleanly handle ignore_above getting set an index-level, I was digging around for the original motivation for these changes and found #113538.

tldr: when we have an index-level ignore_above, we use that as a default for this.ignoreAbove. As a result, the statement ignoreAbove != ignoreAboveDefault would evaluate to false. This is a problem bc we use this statement here to evaluate whether ignore_above is set. This actually used to be an issue until it was fixed in #113570.

elasticsearchmachine · 2025-09-09T01:25:47Z

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

martijnvg

Left two more comments, LGTM otherwise.

martijnvg · 2025-09-10T19:11:41Z

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

+    /**
+     * Generates a string containing a random number of random length alphas, all delimited by space.
+     */
+    public static String randomAlphasDelimitedBySpace(int maxAlphas, int minCodeUnits, int maxCodeUnits) {


Let's keep this in the TextRollingUpgradeIT? This is the only user for now.

server/src/main/java/org/elasticsearch/index/IgnoreAbove.java

…ut of IndexSettings and into IgnoreAbove

elasticsearchmachine · 2025-09-10T22:21:39Z

💔 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
❌	8.18	Commit could not be cherrypicked due to conflicts
❌	9.0	Commit could not be cherrypicked due to conflicts

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

…word multi fields for block loading (elastic#134253) * Added keyword multi field with ignore_above to match only text bwc tests * Rework ignore_above * Added unit tests * Undo changed to match only text bwc test * formatting * Removed indexMode from field type * Added another test case * Fixed failing bwc tests * Improved msg * Added additional tests * Added IgnoreAbove record, addressed index-level ignore above * Fixed test typos * Added IgnoreAboveTest * Enforce at least one value or defaultValue to always be non-null when IgnoreAbove is initialized * When initializing IgnoreAbove, dont use defaultValue from builder - this fixes failing BWC test * Fixed typo * Switched IgnoreAbove to constructor only, removed the ability to set default directly * Update docs/changelog/134253.yaml * Update 134253.yaml * Move IgnoreAbove into Mapper and make it final, move everything new out of IndexSettings and into IgnoreAbove * Fixed typo * Added helpful constructor to IgnoreAbove * Added helpful constructor to IgnoreAbove (cherry picked from commit 2c3bee7) # Conflicts: # server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java # server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

Kubik42 · 2025-09-11T00:35:25Z

💚 All backports created successfully

Status	Branch	Result
✅	9.0

Questions ?

Please refer to the Backport tool documentation

…word multi fields for block loading (elastic#134253) * Added keyword multi field with ignore_above to match only text bwc tests * Rework ignore_above * Added unit tests * Undo changed to match only text bwc test * formatting * Removed indexMode from field type * Added another test case * Fixed failing bwc tests * Improved msg * Added additional tests * Added IgnoreAbove record, addressed index-level ignore above * Fixed test typos * Added IgnoreAboveTest * Enforce at least one value or defaultValue to always be non-null when IgnoreAbove is initialized * When initializing IgnoreAbove, dont use defaultValue from builder - this fixes failing BWC test * Fixed typo * Switched IgnoreAbove to constructor only, removed the ability to set default directly * Update docs/changelog/134253.yaml * Update 134253.yaml * Move IgnoreAbove into Mapper and make it final, move everything new out of IndexSettings and into IgnoreAbove * Fixed typo * Added helpful constructor to IgnoreAbove * Added helpful constructor to IgnoreAbove (cherry picked from commit 2c3bee7) # Conflicts: # modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java # qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/TextRollingUpgradeIT.java # server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java # server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java # server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java # server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldTypeTests.java # server/src/test/java/org/elasticsearch/index/mapper/MultiFieldsTests.java # x-pack/plugin/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java

…ir keyword multi fields for block loading (#134253) (#134516) * Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading (#134253)

Kubik42 · 2025-09-11T23:09:44Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Questions ?

Please refer to the Backport tool documentation

…word multi fields for block loading (elastic#134253)

…word multi fields for block loading (#134253) (#134591)

…word multi fields for block loading (elastic#134253) (elastic#134591)

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

elasticsearchmachine added the v9.2.0 label Sep 5, 2025

Kubik42 force-pushed the kubik42-isignoreaboveset branch from f0bcd32 to 8843756 Compare September 5, 2025 23:22

Kubik42 commented Sep 5, 2025

View reviewed changes

Kubik42 marked this pull request as ready for review September 5, 2025 23:37

martijnvg reviewed Sep 8, 2025

View reviewed changes

Kubik42 marked this pull request as draft September 8, 2025 15:09

Kubik42 force-pushed the kubik42-isignoreaboveset branch from 9c6eee7 to 577aa8b Compare September 8, 2025 22:00

Kubik42 changed the title ~~Refactor how ignore_above is set~~ Fixed text fields block loader Sep 8, 2025

Kubik42 force-pushed the kubik42-isignoreaboveset branch 4 times, most recently from 0a92f63 to 6f8b187 Compare September 8, 2025 23:10

Kubik42 force-pushed the kubik42-isignoreaboveset branch 3 times, most recently from a725363 to b07ddf4 Compare September 9, 2025 06:18

Kubik42 added 4 commits September 9, 2025 13:50

Added keyword multi field with ignore_above to match only text bwc tests

004d04c

Rework ignore_above

be849ea

Added unit tests

5bc89e8

Undo changed to match only text bwc test

78ba81c

Kubik42 changed the title ~~Fixed text fields block loader~~ Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading Sep 10, 2025

martijnvg approved these changes Sep 10, 2025

View reviewed changes

Kubik42 and others added 5 commits September 10, 2025 13:24

Move IgnoreAbove into Mapper and make it final, move everything new o…

186776b

…ut of IndexSettings and into IgnoreAbove

Fixed typo

44d9ce8

Added helpful constructor to IgnoreAbove

42c82a9

Added helpful constructor to IgnoreAbove

d26fad8

Merge branch 'main' into kubik42-isignoreaboveset

03943bc

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

Kubik42 merged commit 2c3bee7 into elastic:main Sep 10, 2025
33 of 34 checks passed

elasticsearchmachine added the backport pending label Sep 10, 2025

Kubik42 mentioned this pull request Sep 10, 2025

[9.1] Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading (#134253) #134516

Merged

Kubik42 removed v8.18.8 v9.0.8 labels Sep 10, 2025

Kubik42 mentioned this pull request Sep 11, 2025

[9.0] Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading (#134253) #134518

Closed

Kubik42 deleted the kubik42-isignoreaboveset branch September 11, 2025 17:29

This was referenced Sep 11, 2025

Text fields incorrectly fallback to source in block loader for logsdb indices #134096

Closed

[8.19] Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading (#134253) #134591

Merged

Kubik42 added a commit to Kubik42/elasticsearch that referenced this pull request Sep 12, 2025

Fixed a bug where text fields in LogsDB indices did not use their key…

f775702

…word multi fields for block loading (elastic#134253)

Kubik42 added a commit that referenced this pull request Sep 12, 2025

Fixed a bug where text fields in LogsDB indices did not use their key…

725a860

…word multi fields for block loading (#134253) (#134591)

sarog pushed a commit to portsbuild/elasticsearch that referenced this pull request Sep 19, 2025

Fixed a bug where text fields in LogsDB indices did not use their key…

deb2a87

…word multi fields for block loading (elastic#134253) (elastic#134591)

Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading #134253

Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading #134253

Uh oh!

Conversation

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

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 5, 2025

Uh oh!

Kubik42 Sep 5, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Sep 5, 2025

Uh oh!

elasticsearchmachine commented Sep 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 8, 2025

Uh oh!

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

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 9, 2025

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 10, 2025

💔 Backport failed

Uh oh!

Kubik42 commented Sep 11, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Kubik42 commented Sep 11, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kubik42 commented Sep 5, 2025 •

edited

Loading

Kubik42 Sep 5, 2025 •

edited

Loading

Kubik42 commented Sep 9, 2025 •

edited

Loading