Fix flat_object to support subfield access in Painless scripts (Resolves #7138)

[Aadityasharma1-programmer]

Description

This PR enables Painless Scripts to accurately fetch and evaluate doc values for flat_object subfields.

Previously, if a script attempted to read a subfield via doc['flat_object_field.subfield'].value, it would receive the unparsed internal format containing the path prefix (e.g., subfield=value). This broke script evaluations because the fielddataBuilder was returning the raw _valueAndPath data stream.

How it was solved:

• Modified FlatObjectFieldType.fielddataBuilder() to dynamically wrap IndexFieldData if the requested field is a subfield.
• Introduced PrefixFilteredSortedBinaryDocValues which wraps the standard SortedBinaryDocValues. During iteration within the script context, it filters the _valueAndPath stream for the exact subfield prefix (e.g., subfield=) and strips the prefix before yielding the value.
• Added a new unit test testSubfieldDocValue() in FlatObjectFieldDataTests.java to explicitly test script interactions with flat_object subfields and guarantee accurate filtering.

Related Issues

Resolves #7138

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit c7eb9e7)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Incorrect Prefix Matching The prefix filter matches any doc value whose bytes start with subfield=, which will incorrectly match sibling fields sharing a prefix. For instance, when querying field.detail.name, the prefix becomes detail.name= and could also match values like detail.nametag=... if such a sibling subfield existed. The prefix should include a proper terminator to ensure exact subfield match rather than a startsWith comparison. Additionally, if subfield names contain =, matching by simple prefix could conflict with values that contain = characters. String prefix = getDVPrefix(rootFieldName) + getPathPrefix(name()); return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = FieldData.toString(sdv); return new ScriptDocValues.Strings( new PrefixFilteredSortedBinaryDocValues(sbdv, prefix) ); }, CoreValuesSourceType.BYTES); } return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), CoreValuesSourceType.BYTES); } private static class PrefixFilteredSortedBinaryDocValues extends SortedBinaryDocValues { private final SortedBinaryDocValues in; private final BytesRef prefix; private int docValueCount; private final List<BytesRef> matches = new ArrayList<>(); private int index; PrefixFilteredSortedBinaryDocValues(SortedBinaryDocValues in, String prefix) { this.in = in; this.prefix = new BytesRef(prefix); } @Override public boolean advanceExact(int doc) throws IOException { if (in.advanceExact(doc)) { matches.clear(); int count = in.docValueCount(); for (int i = 0; i < count; i++) { BytesRef val = in.nextValue(); if (val.length >= prefix.length && StringHelper.startsWith(val, prefix)) { BytesRef stripped = new BytesRef(val.bytes, val.offset + prefix.length, val.length - prefix.length); matches.add(BytesRef.deepCopyOf(stripped)); } } Ordinal-based APIs Broken The builder returns a SortedSetOrdinalsIndexFieldData but only overrides the script values path via the scriptFunction. Consumers that access the raw SortedSetDocValues (e.g., aggregations, sorting, terms enum) will still see the unfiltered _valueAndPath stream containing the prefix, so subfield-based aggregations/sorting will misbehave for flat_object subfields even though scripts now work. if (isSubField()) { String prefix = getDVPrefix(rootFieldName) + getPathPrefix(name()); return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = FieldData.toString(sdv); return new ScriptDocValues.Strings( new PrefixFilteredSortedBinaryDocValues(sbdv, prefix) ); }, CoreValuesSourceType.BYTES); } return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), CoreValuesSourceType.BYTES); Unnecessary Allocation BytesRef.deepCopyOf(stripped) is called for every matching value on every doc, and results are stored in a growing ArrayList per doc. Since SortedBinaryDocValues.nextValue() is consumed sequentially before the next advanceExact, this creates significant GC pressure for docs with many subfields. Consider streaming via a stateful iterator over the underlying values instead of materializing a list of deep-copied BytesRefs. public boolean advanceExact(int doc) throws IOException { if (in.advanceExact(doc)) { matches.clear(); int count = in.docValueCount(); for (int i = 0; i < count; i++) { BytesRef val = in.nextValue(); if (val.length >= prefix.length && StringHelper.startsWith(val, prefix)) { BytesRef stripped = new BytesRef(val.bytes, val.offset + prefix.length, val.length - prefix.length); matches.add(BytesRef.deepCopyOf(stripped)); } } docValueCount = matches.size(); index = 0; return docValueCount > 0; } return false; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to c7eb9e7

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Reset state on every advanceExact call When advanceExact returns true but no values match the prefix, the method currently returns false. However, per the SortedBinaryDocValues contract, callers may still call nextValue() docValueCount() times. More importantly, returning false when the underlying doc has values but none match the prefix is correct semantically, but matches should also be cleared when returning false from the outer branch to avoid stale state carryover across doc advances. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [260-276] @Override public boolean advanceExact(int doc) throws IOException { + matches.clear(); + docValueCount = 0; + index = 0; if (in.advanceExact(doc)) { - matches.clear(); int count = in.docValueCount(); for (int i = 0; i < count; i++) { BytesRef val = in.nextValue(); if (val.length >= prefix.length && StringHelper.startsWith(val, prefix)) { BytesRef stripped = new BytesRef(val.bytes, val.offset + prefix.length, val.length - prefix.length); matches.add(BytesRef.deepCopyOf(stripped)); } } docValueCount = matches.size(); - index = 0; return docValueCount > 0; } return false; } Suggestion importance[1-10]: 4 __ Why: Resetting matches, docValueCount, and index unconditionally at the start of advanceExact is a reasonable defensive measure to prevent stale state, though callers typically shouldn't call nextValue() after advanceExact returns false. Minor robustness improvement.	Low
	Ensure prefix aligns with DV separator encoding The doc-value encoding in flat_object typically uses a separator between key path and value (e.g., key=value or key\0value). Using just getDVPrefix(rootFieldName) + getPathPrefix(name()) as the prefix and stripping it directly returns the full remaining bytes including any separator, which will surface the separator character as part of the returned value in scripts. Verify the prefix includes the key/value separator and that the stripped bytes correspond exactly to the value portion. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [235-243] if (isSubField()) { String prefix = getDVPrefix(rootFieldName) + getPathPrefix(name()); + // Ensure prefix includes the key/value separator used by flat_object DV encoding return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = FieldData.toString(sdv); return new ScriptDocValues.Strings( new PrefixFilteredSortedBinaryDocValues(sbdv, prefix) ); }, CoreValuesSourceType.BYTES); } Suggestion importance[1-10]: 3 __ Why: The suggestion only asks the author to verify the separator encoding without providing a concrete code change (the improved_code merely adds a comment). It raises a valid concern but lacks actionable content.	Low

---

Previous suggestions

Suggestions up to commit 6265b68

Category	Suggestion	Impact
Possible issue	Match full path segment, not prefix The flat_object field stores values as path=value where the path and value are separated by a delimiter (typically \0). Simply checking startsWith(prefix) may produce false positives when the prefix matches only partially (e.g., field.detail would match field.detailed). Ensure the byte following the prefix is the actual separator to correctly isolate the subfield, and strip the separator from the returned value. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [264-270] int count = in.docValueCount(); for (int i = 0; i < count; i++) { BytesRef val = in.nextValue(); - if (val.length >= prefix.length && StringHelper.startsWith(val, prefix)) { - BytesRef stripped = new BytesRef(val.bytes, val.offset + prefix.length, val.length - prefix.length); + if (val.length > prefix.length + && StringHelper.startsWith(val, prefix) + && val.bytes[val.offset + prefix.length] == SEPARATOR) { + int start = val.offset + prefix.length + 1; + BytesRef stripped = new BytesRef(val.bytes, start, val.length - prefix.length - 1); matches.add(BytesRef.deepCopyOf(stripped)); } } Suggestion importance[1-10]: 8 __ Why: Valid concern about potential false positives when a prefix matches partial path segments (e.g., field.detail matching field.detailed). Flat object fields typically use a separator between path and value, and not verifying it could return incorrect data. However, the exact separator constant needs verification against the actual encoding.	Medium
General	Filter also applies to non-script consumers The custom ScriptDocValues.Strings supplier is only wired to getScriptValues(). Other consumers of the fielddata (aggregations, sorting, getBytesValues()) will still see the unfiltered underlying doc values including the path=value composite entries, producing incorrect results for subfields. Consider filtering at the LeafOrdinalsFieldData / getBytesValues level as well, or documenting/limiting subfield fielddata to scripting only. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [235-243] if (isSubField()) { String prefix = getPathPrefix(name()); + // TODO: also filter bytes values / ordinals for aggs & sorting on subfields return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = FieldData.toString(sdv); return new ScriptDocValues.Strings( new PrefixFilteredSortedBinaryDocValues(sbdv, prefix) ); }, CoreValuesSourceType.BYTES); } Suggestion importance[1-10]: 7 __ Why: Valid observation that only getScriptValues() is filtered, while aggregations and sorting via getBytesValues() would return unfiltered composite entries. This is an important correctness concern for the broader feature, though the improved_code only adds a TODO comment rather than a fix.	Medium

Suggestions up to commit f79b049

Category	Suggestion	Impact
Possible issue	Match on key separator to avoid false prefixes The flat_object doc_values store entries as key=value with a separator byte, so filtering by prefix like field.detail.name will also match unrelated keys such as field.detail.name2. The stripped value also still contains the leading separator plus the value portion (e.g. \0foo), which is not the raw value. You should match against prefix + SEPARATOR and strip both the prefix and the separator so the returned BytesRef is the pure value (e.g. foo). server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [261-267] int count = in.docValueCount(); for (int i = 0; i < count; i++) { BytesRef val = in.nextValue(); - if (val.length >= prefix.length && org.apache.lucene.util.StringHelper.startsWith(val, prefix)) { - BytesRef stripped = new BytesRef(val.bytes, val.offset + prefix.length, val.length - prefix.length); + if (val.length > prefix.length + && org.apache.lucene.util.StringHelper.startsWith(val, prefix) + && val.bytes[val.offset + prefix.length] == SEPARATOR) { + int off = val.offset + prefix.length + 1; + BytesRef stripped = new BytesRef(val.bytes, off, val.length - prefix.length - 1); matches.add(BytesRef.deepCopyOf(stripped)); } } Suggestion importance[1-10]: 7 __ Why: Valid concern about prefix matching potentially matching sibling keys with similar names (e.g., name vs name2) and about the stored separator byte. However, the exact separator constant and storage format assumptions need verification against the actual flat_object encoding.	Medium
	Ensure prefix matches stored doc_values key format The prefix passed here is the dotted path (e.g. field.detail.name), but flat_object doc_values are stored as concatenated key/value with the field-name portion stripped. You should construct the prefix using the subpath relative to the mapper root (i.e. the portion after rootFieldName), otherwise no doc_values will ever match and the query returns empty. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [251-254] PrefixFilteredSortedBinaryDocValues(org.opensearch.index.fielddata.SortedBinaryDocValues in, String prefix) { this.in = in; + // prefix should be the sub-path relative to the flat_object root, matching the stored key form this.prefix = new BytesRef(prefix); } Suggestion importance[1-10]: 4 __ Why: Raises a valid concern but the improved_code is essentially identical to existing_code with only a comment added, offering minimal actionable improvement. The concern may also be already addressed by getPathPrefix.	Low
General	Filter applies inconsistently across field-data consumers Using SortedSetOrdinalsIndexFieldData still causes ordinal-based operations (sort, terms aggregation) to iterate the whole _value field, so filtering only happens in the script-values path. Aggregations, sorting, and .keyword uses will return unrelated sibling keys' values. Consider wrapping the field data itself (or returning a dedicated IndexFieldData implementation) so all consumers see filtered values consistently. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [232-240] if (isSubField()) { String prefix = getPathPrefix(name()); + // TODO: filter should apply to all consumers (aggs/sort), not only script values return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = org.opensearch.index.fielddata.FieldData.toString(sdv); return new org.opensearch.index.fielddata.ScriptDocValues.Strings( new PrefixFilteredSortedBinaryDocValues(sbdv, prefix) ); }, CoreValuesSourceType.BYTES); } Suggestion importance[1-10]: 6 __ Why: Valid architectural concern that aggregations and sorting would bypass the prefix filter, but the improved_code only adds a TODO comment rather than fixing the issue, limiting its practical value.	Low

[github-actions]

❌ Gradle check result for f79b049: 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?

[github-actions]

Persistent review updated to latest commit 6265b68

[github-actions]

❌ Gradle check result for 6265b68: 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?

[github-actions]

Persistent review updated to latest commit c7eb9e7

[github-actions]

❌ Gradle check result for c7eb9e7: 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?