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 1ef751a)

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 Prefix Over-Matching The prefix filter uses StringHelper.startsWith(val, prefix) where prefix is built from getDVPrefix(rootFieldName) + getPathPrefix(name()). If getPathPrefix returns something like detail.name= for subfield detail.name, a sibling field named detail.name_extra could share the same prefix and cause false matches. Verify the prefix includes a proper terminator (e.g., trailing = separator) so it cannot match longer subfield names sharing the same prefix. 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); Incomplete DocValues Wrapping fielddataBuilder only wraps the ScriptDocValues path (via the builder's script-values factory). Other consumers of the returned IndexFieldData (aggregations, sorting, getBytesValues()) will still see the raw subfield=value internal format for subfield access, since only the script-values factory is overridden. If subfield aggregations/sorting are supported, they will produce incorrect results; if not supported, they should be explicitly rejected. public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) { failIfNoDocValues(); 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 PrefixFilteredSortedBinaryDocValues.advanceExact performs BytesRef.deepCopyOf for each matching value on every document iteration, allocating new byte arrays on the hot path. For docs with many flat_object subfields, this creates significant GC pressure. Consider reusing buffers or holding references to the underlying bytes if the upstream SortedBinaryDocValues guarantees stability across nextValue calls. 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 1ef751a

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Load doc values from the correct field The builder is passed valueFieldType().name() as the field name, but the underlying doc values for a subfield are stored on the flat object's doc-values field (with the concatenated key=value entries), not the _value subfield. Confirm the correct doc-values field name is used here, otherwise SortedSetDocValues will be loaded from the wrong field and the prefix filter will find no matches. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [235-241] if (isSubField()) { String prefix = getDVPrefix(rootFieldName) + getPathPrefix(name()); - return new SortedSetOrdinalsIndexFieldData.Builder(valueFieldType().name(), (SortedSetDocValues sdv) -> { + return new SortedSetOrdinalsIndexFieldData.Builder(rootFieldName, (SortedSetDocValues sdv) -> { SortedBinaryDocValues sbdv = FieldData.toString(sdv); return new ScriptDocValues.Strings(new PrefixFilteredSortedBinaryDocValues(sbdv, prefix)); }, CoreValuesSourceType.BYTES); } Suggestion importance[1-10]: 6 __ Why: This raises a plausible concern about which field's doc-values are being loaded for a subfield, since the prefix filter operates on the flat object's key=value entries. However, without full context it's uncertain whether valueFieldType().name() correctly resolves to the root field's doc values.	Low
	Ensure separator is stripped from values The stripped value still contains the encoded key=value pair minus the key prefix, which likely includes a separator (e.g., =) before the actual value. Verify a separator is stripped as well; otherwise users will see values prefixed by the separator character in scripts. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [264-267] -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)) { + // Skip the key/value separator that follows the prefix, if any + int start = val.offset + prefix.length; + int len = val.length - prefix.length; + BytesRef stripped = new BytesRef(val.bytes, start, len); matches.add(BytesRef.deepCopyOf(stripped)); } Suggestion importance[1-10]: 5 __ Why: The concern about a separator being included in the stripped value is valid if the prefix does not itself include the separator character. However, getPathPrefix may already include the separator, making the concern speculative. The improved code doesn't actually change behavior meaningfully.	Low
General	Reset state when no prefix matches exist The current logic returns false when the document has values but none match the prefix. However, the caller may not distinguish this from advanceExact returning false due to missing values, and the state of docValueCount could still be leaked from a prior doc if not reset. Ensure docValueCount and index are reset to 0 before returning false when no matches are found, to avoid stale state being observed. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [261-271] 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; +if (docValueCount == 0) { + return false; +} +return true; Suggestion importance[1-10]: 3 __ Why: The existing code already sets docValueCount = matches.size() and index = 0 before returning, so state is already reset. The suggestion offers only a marginal readability change with no functional impact.	Low

---

Previous suggestions

Suggestions up to commit ba1f167

Category	Suggestion	Impact
Possible issue	Filter ordinals for subfield, not only scripts The script-values factory passed here only overrides the script view, but the underlying SortedSetDocValues returned by LeafOrdinalsFieldData.getOrdinalsValues() remains unfiltered. Callers that use ordinals directly (aggregations, sorting) on a subfield will still see the full flat_object values, not those matching the subfield prefix — producing incorrect results. Consider wrapping/filtering the ordinals themselves or restricting subfield fielddata usage to the script path only. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [235-241] if (isSubField()) { String prefix = getDVPrefix(rootFieldName) + getPathPrefix(name()); + // NOTE: also filter ordinals so aggregations/sort see only subfield values 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: This is a valid correctness concern: the ordinals returned by SortedSetOrdinalsIndexFieldData remain unfiltered, so aggregations and sorting on a subfield would see all flat_object values, not just those matching the subfield prefix. The improved_code however is identical and doesn't actually resolve the issue, just adds a comment.	Medium
	Reset state before advancing doc values When advanceExact returns true from in but no values match the prefix, this method returns false, which is inconsistent with the underlying ordinals iterator's state. Downstream ordinal-based consumers (e.g., SortedSetOrdinalsIndexFieldData) may still see the doc as having values via the ordinals path, leading to inconsistent behavior. Also, matches.clear() should happen regardless of the advanceExact result to avoid stale state, and docValueCount should be reset to 0. server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java [258-274] @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]: 5 __ Why: The suggestion correctly identifies that returning false when there are values but none match the prefix is reasonable behavior for the script view, and resetting state up-front is a minor defensive improvement. However, the concern about ordinal inconsistency is only cosmetic since this class only wraps SortedBinaryDocValues for scripts.	Low

Suggestions up to commit c7eb9e7

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

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?

[kkewwei]

@Aadityasharma1-programmer Can you solve the spotlessJavaCheck by "./gradlew spotlessApply"?

[github-actions]

Persistent review updated to latest commit ba1f167

[github-actions]

Persistent review updated to latest commit 1ef751a

[github-actions]

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