Remove the opt-in partial-result mode for mapping conflicts

[ahkcs]

Description

Removes the opt-in partial-result mode for text/keyword mapping conflicts added in #5657, keeping the warnings channel that PR introduced.

Why. The mode traded correctness for speed: on a conflicted group-by it narrowed the aggregation to the index subset where the field is aggregatable and labelled the answer with a warning. Returning a knowingly incomplete result to make a query faster is not a good default to offer, and the safeguards around it were three moving parts — a cluster setting, a per-request flag, and a response-format check — each of which had to survive the security plugin's transport-to-worker handoff. That cost three follow-up fixes (#5739, #5743, #5758) without changing what the feature bought.

A conflicted aggregation now returns the complete result over every index again, which since #5646 pushes down as a per-document _source script rather than failing.

Removed

PartialResultAggregatePushdown + test	the index-subset partitioner
CalciteLogicalIndexScan.tryPartialResultAggregate / resolvePartitionFields / allowPartialFallback	the hook, and the pushDownAggregate overload that existed only to make it one-shot
plugins.query.partial_result.on_mapping_conflict.enabled	setting, its registration, and its settings.rst section
per-request partial_result	PPLQueryRequest, TransportPPLQueryRequest, PPLQueryRequestFactory, PPLService, AbstractPlan, CalcitePlanContext
warningsSupported gate	existed only so this feature would not drop data into CSV/RAW/VIZ; the JSON formatter already emits warnings only when present
PushDownContext.cloneWithOsIndex	only the narrowed-subset rebind used it
CalcitePartialResultOnMappingConflictIT, PartialResultSecurityIT	feature ITs

Kept

The warnings channel, whole: Warning (including TYPE_PARTIAL_RESULT), QueryResponse.warnings, CalcitePlanContext.addWarning/drainWarnings and its per-query clearing, QueryResult, and the SimpleJsonResponseFormatter emission. Nothing produces a warning after this commit — #5807 is the next producer, reporting a search that covered only some of its shards — so the channel stays exercised by its own unit tests in the meantime.

For reviewers

Wire format. Dropping partial_result from TransportPPLQueryRequest changes that request's serialization. The class version-gates nothing and the field was appended the same way in 3.9, so this is symmetric with how it arrived — but it does mean a mixed 3.9/3.10 cluster must not route PPL across versions. Say so if you would rather keep a placeholder read/write.

Coordination with #5807. That PR adds a second warning type for shard coverage. With this feature gone there is only one meaning of "partial" left, so #5807 can collapse onto PARTIAL_RESULT and OpenSearch Dashboards needs no change at all. Whichever lands second takes that rename; I would rather land #5807 first since it is reviewed and green.

Testing

./gradlew build -x integTest -x tracingIntegTest -x yamlRestTest -x doctest green: 11,433 unit tests across core (5284), ppl (2141), opensearch (1710), legacy (1444), sql (711), protocol (71), plugin (50), common (22), 0 failures.

Related Issues

Follows #5657. Coordinates with #5807.

Check List

• New functionality includes testing — n/a, this is a removal; the tests for the removed feature are deleted and the retained channel keeps its own.
• New functionality has been documented — the settings.rst section for the removed setting is deleted.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created, if applicable — needed if the setting was published to the docs website.

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

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 8313a0f)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

Persistent review updated to latest commit 8313a0f

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Apply deep copy consistently The single-index case directly puts field mappings without deep copying, which could allow external mutation of the fetched mapping. Apply deep copying consistently for both single and multi-index cases to prevent unintended side effects. opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java [101-115] public Map<String, OpenSearchDataType> getFieldTypes() { Map<String, OpenSearchDataType> fieldTypes = new HashMap<>(); Map<String, IndexMapping> indexMappings = client.getIndexMappings(getLocalIndexNames(indexName.getIndexNames())); if (indexMappings.size() <= 1) { for (IndexMapping indexMapping : indexMappings.values()) { - fieldTypes.putAll(indexMapping.getFieldMappings()); + fieldTypes.putAll(deepCopy(indexMapping.getFieldMappings())); } } else { - // Merge deep copies: MergeRuleHelper mutates the field mappings in place, and a fetched - // mapping must not be altered by being merged. for (IndexMapping indexMapping : indexMappings.values()) { MergeRuleHelper.merge(fieldTypes, deepCopy(indexMapping.getFieldMappings())); } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the single-index case doesn't deep copy while the multi-index case does. However, the comment in the code explicitly states that deep copying is needed because MergeRuleHelper mutates mappings in place. Since the single-index case doesn't call merge, deep copying may not be strictly necessary there, though applying it consistently would be safer and more maintainable.	Medium

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.28%. Comparing base (3f7048b) to head (8313a0f).

Files with missing lines	Patch %	Lines
...org/opensearch/sql/calcite/CalcitePlanContext.java	0.00%	2 Missing ⚠️

❌ Your project check has failed because the head coverage (63.28%) is below the target coverage (99.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5814      +/-   ##
============================================
+ Coverage     63.23%   63.28%   +0.04%     
+ Complexity     8823     8803      -20     
============================================
  Files           938      937       -1     
  Lines         40236    40094     -142     
  Branches       4537     4514      -23     
============================================
- Hits          25445    25374      -71     
+ Misses        13966    13896      -70     
+ Partials        825      824       -1     

Flag	Coverage Δ
sql-engine	63.28% <0.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.