[cnoramut]

Description

When the indices behind one source map a field to two different number types, the plugin keeps the type of whichever index the merge reads last, and that order can change between nodes and after a restart. Under the wrong winner a value changes or comes back null with HTTP 200, so a long 5000000000 read as integer returns 705032704.

f in index 0	f in index 1	merged type on main	fields f
long 5000000000	integer 3	int	705032704, 3
long 5000000000	integer 3	bigint	5000000000, 3
double 1.5	long 5	bigint	null, 5
double 1.5	long 5	double	1.5, 5.0
long 16777217	half_float 1.5	float	16777216.0, 1.5
long 16777217	half_float 1.5	bigint	16777217, null

Fix. A new NumericWideningRule runs ahead of LatestRule in MergeRuleHelper and merges a numeric conflict to one type, whatever the read order. Two integer types merge to the wider one, half_float and float merge to float, and any other numeric pair merges to double. The PPL limitations page now states the same rule.

Each pair gave one merged type across 12 index name prefixes. Twelve indices covering all eight number types also merged to double, and twelve integer indices to bigint, under five index name prefixes each.

f in index 0	f in index 1	merged type with the rule	fields f
long 5000000000	integer 3	bigint	5000000000, 3
double 1.5	long 5	double	1.5, 5.0
long 16777217	half_float 1.5	double	16777217.0, 1.5

Notes, as mentioned in #5901

1. Why an integer with float gives double - Core's WideningTypeRule would pick float, and a float holds integers exactly only up to 2^24, so 16777217 would read 16777216.0. The cost is that an integer row next to a floating point index renders as a double, so 3 reads 3.0. A pushed min over a float index also prints 0.1 as 0.10000000149011612, the stored float's exact value.
2. What still reaches LatestRule - A number against a string or date type, and an alias of a number field, keep today's behaviour.

Related Issues

Part of #5828 - This fixes the number against number rows. Its other rows, such as long against date, are unchanged, so this PR does not resolve it.
Resolves #5901

Testing

• NumericWideningRuleTest, 13 cases that merge through MergeRuleHelper in both orders, plus three type sets in all six orders. Taking the rule out of RULES fails 6 of them, and making half_float with float return double fails 2.
• issues/5901.yml, 3 cases. The first spans byte, short, integer and long, the others pair long with double and with half_float. Each asserts the schema type and the values. With the rule taken out of RULES, the first and third failed on the type, reporting int and float. The long with half_float case always fails without the rule, since its expected double is a type neither index holds. The other two fail only when a narrower index is read last, which depends on the index names in each JVM, so the long with double case passed in that run.
• :opensearch:test ran 1835 tests with 0 failures.
• Each engine was measured on all 28 number pairs in both read orders.

Engine	Result against main
PPL on Calcite, the rule alone against main at 31eea45a8, about 100 templates	results that depend on read order fell from 1,627 of 5,768 to 0, 108 errors became a 200, and none got worse
PPL on V2, the same sweep with Calcite off, against main at f36c99f84	none worse
SQL, 50 queries and 1,400 results, against main at f36c99f84	none worse, and 558 improved

The SQL results improved where main read floating point values as null, read a short 300 as 44 under tinyint, or cut a pushed MAX to an integer. The Calcite sweep changed no single-index result.

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.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add precondition validation for numeric types The method assumes both parameters are numeric types but doesn't validate this precondition. If called with non-numeric types (e.g., through future code changes), it will silently return MappingType.Double, which could lead to incorrect type merging. Add a precondition check or assertion to fail fast if non-numeric types are passed. opensearch/src/main/java/org/opensearch/sql/opensearch/util/MergeRules/NumericWideningRule.java [58-68] private static MappingType widenedType(MappingType a, MappingType b) { + if (!isNumeric(a) || !isNumeric(b)) { + throw new IllegalArgumentException("Both types must be numeric"); + } if (INTEGER_TYPES_BY_WIDTH.contains(a) && INTEGER_TYPES_BY_WIDTH.contains(b)) { return INTEGER_TYPES_BY_WIDTH.indexOf(a) > INTEGER_TYPES_BY_WIDTH.indexOf(b) ? a : b; } if (FLOAT_TYPES.contains(a) && FLOAT_TYPES.contains(b)) { return MappingType.Float; } // Every other pair merges to double. A 32-bit float holds integers exactly only up to 2^24, // so an integer next to a half_float or float lands here too. return MappingType.Double; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that widenedType doesn't validate its inputs, but this is a defensive programming improvement rather than a critical bug. The method is private and only called from mergeInto, which is only invoked after isMatch confirms both types are numeric and different. The validation would add safety against future changes but isn't addressing an existing issue.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.21%. Comparing base (9249643) to head (69d10c9).

❌ Your project check has failed because the head coverage (75.21%) 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    #5902      +/-   ##
============================================
+ Coverage     75.19%   75.21%   +0.01%     
- Complexity    10362    10385      +23     
============================================
  Files           962      963       +1     
  Lines         41098    41123      +25     
  Branches       4683     4689       +6     
============================================
+ Hits          30905    30932      +27     
+ Misses         8896     8895       -1     
+ Partials       1297     1296       -1     

Flag	Coverage Δ
sql-engine	75.21% <100.00%> (+0.01%)	⬆️

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.

[cnoramut]

Thank you for the approval @Swiddis but closing this after offline sync with @dai-chen.