Store ignored malformed fields in binary doc values by Kubik42 · Pull Request #142357 · elastic/elasticsearch

Kubik42 · 2026-02-12T00:53:18Z

Addresses https://github.com/elastic/logs-program/issues/41

Kubik42 · 2026-02-23T21:37:57Z

rest-api-spec/build.gradle

  task.skipTest("search.vectors/42_knn_search_bbq_flat/Vector rescoring has same scoring as exact search for kNN section", "scores have changed slightly with native implementations")
  task.skipTest("search.vectors/41_knn_search_bbq_hnsw/Vector rescoring has same scoring as exact search for kNN section", "scores have changed slightly with native implementations")
+  task.skipTest(
+    "get/100_synthetic_source/fields with ignore_malformed",


Can't use replaceValueInMatch here due to how the yaml is structured - fields with ignore_malformed is large and has many match assertions that specifically check _source.ip. Using replaceValueInMatch here means all of those assertions will be changed, which is not what we want.

I wonder whether instead of skipping the test entirely can we maybe use removeMatch?

that wouldn't work well - each yaml test uses a nested format for all of its match assertions (ex. _source: { ip: [...] }). removeMatch() looks only at the name of the top-level key, (ex. _source), so using it here effectively removes all match assertion and renders the tests useless. This applies to all the test I've skipped in this PR. We would need to unnest some of the match assertions and then backport that in order for the yamlRestCompatTests to pass

Kubik42 · 2026-02-23T21:43:01Z

...r/src/main/java/org/elasticsearch/index/mapper/BinaryDocValuesSyntheticFieldLoaderLayer.java

 import java.io.IOException;

-public final class BinaryDocValuesSyntheticFieldLoaderLayer implements CompositeSyntheticFieldLoader.DocValuesLayer {
+public class BinaryDocValuesSyntheticFieldLoaderLayer implements CompositeSyntheticFieldLoader.DocValuesLayer {


Needed to removed final in order to override BinaryDocValuesSyntheticFieldLoaderLayer.writeValue() to support malformed values. By default writeValue() treats BytesRef values as raw UTF8 strings. This is not the case for malformed values, which can be of any type. For malformed values, we override writeValue() like so:

protected void writeValue(XContentBuilder b, BytesRef value) throws IOException { XContentDataHelper.decodeAndWrite(b, value); }

Kubik42 · 2026-02-23T21:44:10Z

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

+        }
+
+        @Override
+        protected void writeValue(XContentBuilder b, BytesRef value) throws IOException {


here is that override of writeValue() that I mentioned above

Kubik42 · 2026-02-23T21:50:23Z

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

+    }
+
+    /**
+     * Encodes an XContentBuilder's content, normalizing to JSON regardless of the builder's original content type.


This is primarily here for the tests, although its also important to deliver a consistent experience to customers.

Why this is needed for tests: our tests randomize the content type, meanwhile the assertions don't take that into account (ex. yamlRestTests). As a result, the assertion must either be consistent across all content types or we need to change how we store values. For example, we could store things in insertion order as opposed to sorting every time. That said, in an older PR (I can dig it up if needed), we specifically decided to sort everything.

Kubik42 · 2026-02-23T23:30:49Z

...src/main/java/org/elasticsearch/index/mapper/SortedNumericDocValuesSyntheticFieldLoader.java

+            return malformedLoader;
+        }
+        return fieldLoader;
+    }


this is kind of ugly. Most of the other mappers use CompositeSyntheticFieldLoader, which abstracts away how 2+ loaders are handled. I think its worth refactoring SortedNumericDocValuesSyntheticFieldLoader into a Layer that can be used by CompositeSyntheticFieldLoader, but in another PR.

elasticsearchmachine · 2026-02-24T07:44:18Z

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

jordan-powers

LGTM, but @martijnvg should probably have a look too

martijnvg

Looks good @Kubik42! I left a question.

martijnvg · 2026-02-24T17:41:43Z

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

+
+    private static void saveToBinaryDocValues(DocumentParserContext context, String fieldPath, BytesRef encoded) {
+        final String fieldName = name(fieldPath);
+        MultiValuedBinaryDocValuesField.SeparateCount.addToSeparateCountMultiBinaryFieldInDoc(context.doc(), fieldName, encoded, true);


Should we use IntegratedCount here? I think this is what we use for ignore above?

I think integrated count is a better match here, since we don't optimize for fast reading. We're ok with reading value count as part of the actual values. And SeperateCount adds an additional doc value field for all fields that will have malformed values.

hmm ok, I initially chose SeperateCount due to optimizations it makes for single-valued fields.

I think it makes separate count fields make sense for mapped fields, but this like a side field that will not get queried (except when source gets generated) and so optimizing for less doc values fields and storage is better.

martijnvg · 2026-02-24T17:59:28Z

rest-api-spec/build.gradle

  task.skipTest("search.vectors/42_knn_search_bbq_flat/Vector rescoring has same scoring as exact search for kNN section", "scores have changed slightly with native implementations")
  task.skipTest("search.vectors/41_knn_search_bbq_hnsw/Vector rescoring has same scoring as exact search for kNN section", "scores have changed slightly with native implementations")
+  task.skipTest(
+    "get/100_synthetic_source/fields with ignore_malformed",


I wonder whether instead of skipping the test entirely can we maybe use removeMatch?

martijnvg · 2026-02-24T17:59:51Z

x-pack/plugin/build.gradle

    task.skipTest("aggregate-metrics/30_sum_agg/Test sum agg with query", "Default metric cannot be defined, so the query is empty")
    task.skipTest("aggregate-metrics/50_value_count_agg/Test value_count agg with query", "Default metric cannot be defined, so the query is empty")
+    // Malformed values are now stored in binary doc values (sorted) instead of stored fields (insertion order).
+    task.skipTest(


same reason as in the other file?

martijnvg

LGTM 👍

…142357)" This reverts commit f094941.

This reverts commit 26ca5c3.

* Store ignored malformed fields in binary doc values (#142357) This reverts commit 26ca5c3. * Remove extra newline

Kubik42 added >enhancement Team:StorageEngine :StorageEngine/Logs You know, for Logs labels Feb 12, 2026

elasticsearchmachine added the v9.4.0 label Feb 12, 2026

Kubik42 force-pushed the ignore-malformed-binary-doc-values branch 3 times, most recently from 96c4b41 to 9dade60 Compare February 13, 2026 20:56

Kubik42 force-pushed the ignore-malformed-binary-doc-values branch 3 times, most recently from 178d3ff to 057c7ae Compare February 20, 2026 22:59

Kubik42 commented Feb 23, 2026

View reviewed changes

Kubik42 force-pushed the ignore-malformed-binary-doc-values branch from 057c7ae to fa3ffde Compare February 24, 2026 01:35

Kubik42 marked this pull request as ready for review February 24, 2026 07:43

jordan-powers approved these changes Feb 24, 2026

View reviewed changes

martijnvg reviewed Feb 24, 2026

View reviewed changes

Kubik42 added 2 commits February 24, 2026 15:35

Store ignored malformed fields in binary doc values

05e2fff

Addressed feedback

8ce768b

Kubik42 force-pushed the ignore-malformed-binary-doc-values branch from 7eb8e23 to 8ce768b Compare February 24, 2026 23:35

martijnvg approved these changes Feb 25, 2026

View reviewed changes

Merge branch 'main' into ignore-malformed-binary-doc-values

45e0bcb

Kubik42 merged commit f094941 into elastic:main Feb 25, 2026
3 of 6 checks passed

Kubik42 deleted the ignore-malformed-binary-doc-values branch February 25, 2026 16:21

Kubik42 added a commit to Kubik42/elasticsearch that referenced this pull request Feb 25, 2026

Revert "Store ignored malformed fields in binary doc values (elastic#…

26ca5c3

…142357)" This reverts commit f094941.

Kubik42 added a commit to Kubik42/elasticsearch that referenced this pull request Feb 25, 2026

Store ignored malformed fields in binary doc values (elastic#142357)

3523cf6

This reverts commit 26ca5c3.

Kubik42 added a commit to Kubik42/elasticsearch that referenced this pull request Feb 25, 2026

Store ignored malformed fields in binary doc values (elastic#142357)

e6a8330

This reverts commit 26ca5c3.

Kubik42 added a commit that referenced this pull request Feb 25, 2026

Store ignored malformed fields in binary doc values (#142357) (#143087)

b61d7cc

* Store ignored malformed fields in binary doc values (#142357) This reverts commit 26ca5c3. * Remove extra newline

romseygeek mentioned this pull request Feb 26, 2026

[CI] ScaledFloatFieldMapperTests testSyntheticSourceWithTranslogSnapshot failing #143124

Closed

Kubik42 mentioned this pull request Feb 26, 2026

Fixed wrong malformed value ordering in synthetic source tests #143187

Merged

Conversation

Kubik42 commented Feb 12, 2026

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!

Kubik42 Feb 23, 2026 • 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!

elasticsearchmachine commented Feb 24, 2026

Uh oh!

jordan-powers left a comment

Choose a reason for hiding this comment

Uh oh!

martijnvg left a comment

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!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kubik42 Feb 23, 2026 •

edited

Loading