Skip to content

Fix SystemFunctionsIT.testTypeofArithmetic for int-operand widening - #22466

Merged
bharath-techie merged 1 commit into
opensearch-project:mainfrom
aasom143:build-fix
Jul 20, 2026
Merged

Fix SystemFunctionsIT.testTypeofArithmetic for int-operand widening#22466
bharath-techie merged 1 commit into
opensearch-project:mainfrom
aasom143:build-fix

Conversation

@aasom143

Copy link
Copy Markdown
Contributor

Description

SystemFunctionsIT.testTypeofArithmetic has been failing consistently on main:

Value mismatch for query: source=calcs | where key='key00' | head 1 | eval t = typeof(int0 * 2) | fields t
expected:<[]INT> but was:<[BIG]INT>

This is an expected behavior change, not a regression. opensearch-project/sql#5603 widens narrow integer operands (byte/short/int) before +, -, * so the result cannot overflow the inferred type. As a result, int0 * 2 now infers as BIGINT instead of INT, and typeof(int0 * 2) correctly returns "BIGINT". The QA assertion was written against the old (overflow-prone) INT inference and needs to reflect the new, correct type.

Change

Update the typeof(int0 * 2) assertion (and its comment) to expect "BIGINT". Test-only; no production code change.

Testing

  • ./gradlew ':sandbox:qa:analytics-engine-rest:integTest' --tests 'org.opensearch.analytics.qa.SystemFunctionsIT.testTypeofArithmetic' (sandbox, JDK 25).

Check List

  • Commits are signed per the DCO using --signoff.

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

@aasom143
aasom143 requested a review from a team as a code owner July 14, 2026 14:29
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 4ac064f)

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

Empty test body

The testTypeofArithmetic method body only contains a single assertFirstRowString call; however, in the diff the previous assertFirstRowString call was removed (in the __old hunk__) and now only the new one is present. Verify the test method still asserts what is intended — it now only checks that typeof(int0 * 2) returns BIGINT and no longer verifies the pre-existing INT case. If the intent was to keep both cases (e.g., also assert on a non-widening arithmetic form), the assertion coverage has been reduced.

public void testTypeofArithmetic() throws IOException {
    assertFirstRowString(oneRow("key00") + "| eval t = typeof(int0 * 2) | fields t", "BIGINT");
}

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 527e93f: SUCCESS

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.41%. Comparing base (c93e9af) to head (4ac064f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #22466      +/-   ##
============================================
- Coverage     73.43%   73.41%   -0.03%     
+ Complexity    76472    76452      -20     
============================================
  Files          6104     6104              
  Lines        346573   346573              
  Branches      49886    49886              
============================================
- Hits         254514   254435      -79     
- Misses        71798    71925     +127     
+ Partials      20261    20213      -48     

☔ 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.

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 7defc3e

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 7defc3e: SUCCESS

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 594ed79

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 594ed79: 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

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 8821e4b

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 8821e4b: null

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

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 7b7b2bf

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit ec0e9a8

@github-actions

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit aa6f7cd

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for aa6f7cd: SUCCESS

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit fcef072

@github-actions

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit a3af830

opensearch-project/sql#5603 widens narrow integer operands (byte/short/int)
before +, -, * so the result cannot overflow the inferred type, which makes
`int0 * 2` infer as BIGINT instead of INT. Update the typeof assertion (and its
comment) to expect "BIGINT", matching the merged SQL behavior. This IT has been
failing consistently on main since that grammar/lowering change landed.
Test-only change..

Signed-off-by: Kai Huang <ahkcs@amazon.com>

Update plan shape goldens for int-operand widening to BIGINT

The int-operand widening fix (aa6f7cd) changed arithmetic
expressions to cast integer operands to BIGINT before addition or
subtraction. Update the q30 and q36 golden files to reflect the new
plan shapes in both logical (nested ANNOTATED_PROJECT_EXPR with
CAST(:BIGINT)) and physical (Int32 → Int64, CAST(... AS Int64))
layers.

Signed-off-by: Somesh Gupta <guptasom@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 4ac064f

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 4ac064f: SUCCESS

@bharath-techie
bharath-techie merged commit 39fe516 into opensearch-project:main Jul 20, 2026
16 checks passed
@aasom143
aasom143 deleted the build-fix branch July 20, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants