[Bug Fix] Resolve dotted source paths in pushed-down Calcite scripts

[RyanL1997]

Description

When a PPL query pushes work down as a Calcite script that reads a field from _source, the field is addressed by its flattened name (e.g. log.user_agent), while _source stores object subfields nested ({"log": {"user_agent": ...}}). The lookup was a flat Map#get:

public Object getFromSource(String name) {
  return this.sourceLookup.get(name);
}

So any object subfield resolved to null. No error was raised — the query simply produced a wrong answer.

What was actually wrong

All five pushed-down script types — filter, aggregation, string sort, number sort, and field — funnel through this one method via CalciteScript → ScriptDataContext. Measured against main:

query on an object subfield	before	after
where log.user_agent = 'requests/2.32.3'	0 rows	2 rows
where upper(log.user_agent) = '...'	0 rows	2 rows
stats count() by log.ua	[[3, null]] — every document in one null bucket	[[2,"aaa"],[1,"bbb"]]
eval u = upper(log.ua) | sort u, id	[[1,BBB],[2,AAA],[3,AAA]] — ordering not applied	[[2,AAA],[3,AAA],[1,BBB]]

The aggregation case is the most dangerous of the four: a filter returning zero rows is visibly empty, but a stats ... by that silently collapses every document into a single null bucket returns a result that looks entirely plausible.

Root cause note

The issue hypothesised that DeepMergeRule handles nested fields differently from the fix in #5358. That is not the cause — the merge is correct for nested fields:

NESTED merged log.user_agent mappingType = text
NESTED keywordSubField                   = null   <- correctly downgraded

TextKeywordConflictRule does apply to nested subfields through DeepMergeRule, and the engine correctly switches to Source.SOURCE as a result. The explain output confirms the retrieval mode was chosen correctly:

SOURCES:[1,2]  DIGESTS:["log.user_agent", "requests/2.32.3"]

SOURCES=1 is Source.SOURCE — doc_values were correctly avoided. The defect is one level further down, in how that source value is read.

Scope is wider than mixed-type indices

A cross-index type conflict is not required. A single index whose object subfield is plain text already resolves through _source and hit the same path. Mixed text/keyword types merge down to text-without-keyword-subfield, which routes more fields through _source — that is how #5702 surfaced, but it is not the precondition.

Behaviour comparison against baseline

Edge cases were run on main and on this branch to isolate the change:

case	baseline	this PR
literal dotted key under enabled: false object	ok	ok (unchanged)
multi-value select ["a", null, "b"]	["a",null,"b"]	["a",null,"b"] (unchanged)
multi-value in an expression	0	0 (unchanged — documented limitation)
nested-type field filter	0	0 (unchanged — see #4625)
3-level deep object filter	0	1 (also fixed)
object subfield absent from some docs	0	1 (also fixed)
isnull(o.v)	2	2 (unchanged)
direct sort log.ua (doc_values path)	ok	ok (unchanged)
stats sum(log.n) on an integer subfield	6	6 (unchanged)

No unintended behaviour changes; two further latent cases are fixed as a side effect.

Related Issues

Resolves #5702
Follow-up to #5358 / #4659

Testing

CalciteMixedFieldTypeIT — 5 new cases covering filter, script filter, the single-index no-conflict case, stats ... by on an object subfield, and sort on one. Reverting the one-line fix fails exactly these 5 and leaves the 4 pre-existing #4659 cases green.

The sort case asserts with verifyDataRowsInOrder; verifyDataRows compares without regard to order and cannot observe an ordering defect.

integ-test/src/yamlRestTest/.../issues/5702.yml — 2 new scenarios, mirroring 4659.yml.

CalciteMixedFieldTypeIT is already registered in CalciteNoPushdownIT, so the new cases also run with pushdown disabled.

suite	result
full integTest	382 classes, 7227 tests, 0 failures
CalciteNoPushdownIT	2312 tests, 0 failures
full yamlRestTest	180 tests, 0 failures
all module unit tests	pass
spotlessCheck	pass

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

🤖 Generated with Claude Code

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 7724f14)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

📝 TODO sections opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java [236]: should we change to store timestamp as Instant in the future

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 7724f14
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Handle multi-valued field extraction The extractValue method may return a list when multiple values exist for a field. Verify that the calling code handles both single values and lists correctly, or add explicit handling to ensure consistent return types. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java [242-247] public Object getFromSource(String name) { // Resolve the field through the source path, not a flat map lookup: object subfields are // addressed as dotted paths (e.g. "log.user_agent") while _source stores them nested. // SourceLookup#extractValue delegates to XContentMapValues, which walks the nested maps and // still falls back to a literal dotted key when the document has one. - return this.sourceLookup.extractValue(name, null); + Object value = this.sourceLookup.extractValue(name, null); + // Handle multi-valued fields: return first element if list with single item + if (value instanceof List && ((List<?>) value).size() == 1) { + return ((List<?>) value).get(0); + } + return value; } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that extractValue may return a list for multi-valued fields. Adding explicit handling to unwrap single-element lists ensures consistent return types and prevents potential issues in calling code. However, this is a defensive improvement rather than fixing a critical bug demonstrated in the PR's test cases.	Medium

---

Previous suggestions

Suggestions up to commit 366b9a8

Category	Suggestion	Impact
General	Handle multi-valued field extraction The extractValue method may return a list when multiple values exist for the field. Consider handling the case where the returned value is a collection to avoid unexpected behavior in downstream code that expects a single value. opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java [242-247] public Object getFromSource(String name) { // Resolve the field through the source path, not a flat map lookup: object subfields are // addressed as dotted paths (e.g. "log.user_agent") while _source stores them nested. // SourceLookup#extractValue delegates to XContentMapValues, which walks the nested maps and // still falls back to a literal dotted key when the document has one. - return this.sourceLookup.extractValue(name, null); + Object value = this.sourceLookup.extractValue(name, null); + // Handle multi-valued fields by returning the first element if it's a list + if (value instanceof List && !((List<?>) value).isEmpty()) { + return ((List<?>) value).get(0); + } + return value; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that extractValue may return a list for multi-valued fields. However, the PR's test cases only cover single-valued fields, and the suggestion assumes taking the first element is always correct without considering the broader context of how multi-valued fields should be handled in this codebase. This is a reasonable defensive programming practice but may not align with the intended behavior.	Low

[github-actions]

Persistent review updated to latest commit 7724f14