Skip to content

fix (ae): add scalars for IS_{bool}#22091

Merged
mch2 merged 4 commits into
opensearch-project:mainfrom
Swiddis:fix/case-tests
Jun 11, 2026
Merged

fix (ae): add scalars for IS_{bool}#22091
mch2 merged 4 commits into
opensearch-project:mainfrom
Swiddis:fix/case-tests

Conversation

@Swiddis

@Swiddis Swiddis commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

Quick fix in AE branch for CASE tests, some missing scalar functions

Related Issues

related: opensearch-project/sql#5531 (not blocking)

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.

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis Swiddis requested a review from a team as a code owner June 9, 2026 21:28
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 50b9541)

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

@Swiddis Swiddis added the analytics-engine Issues and PR for the new Analytics engine label Jun 9, 2026
@Swiddis Swiddis changed the title fix: add scalars for is with bool fix (ae): add scalars for IS_BOOL Jun 9, 2026
@Swiddis Swiddis changed the title fix (ae): add scalars for IS_BOOL fix (ae): add scalars for IS_{bool} Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

✅ Gradle check result for f87c887: SUCCESS

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.40%. Comparing base (4a9d0ed) to head (50b9541).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #22091      +/-   ##
============================================
+ Coverage     73.38%   73.40%   +0.02%     
- Complexity    75601    75667      +66     
============================================
  Files          6041     6041              
  Lines        343397   343420      +23     
  Branches      49420    49428       +8     
============================================
+ Hits         252006   252099      +93     
+ Misses        71266    71216      -50     
+ Partials      20125    20105      -20     

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

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 86e2c1d

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle null values in boolean comparisons

Consider the semantic correctness of these boolean comparison operators. In SQL, IS
NOT TRUE evaluates to true for both false and null values, while IS NOT FALSE
evaluates to true for both true and null values. Ensure the implementation handles
null values correctly according to SQL three-valued logic.

sandbox/libs/analytics-framework/src/main/java/org/opensearch/analytics/spi/ScalarFunction.java [41-44]

 IS_TRUE(Category.COMPARISON, SqlKind.IS_TRUE),
 IS_FALSE(Category.COMPARISON, SqlKind.IS_FALSE),
-IS_NOT_TRUE(Category.COMPARISON, SqlKind.IS_NOT_TRUE),
-IS_NOT_FALSE(Category.COMPARISON, SqlKind.IS_NOT_FALSE),
+IS_NOT_TRUE(Category.COMPARISON, SqlKind.IS_NOT_TRUE),  // Returns true for FALSE and NULL
+IS_NOT_FALSE(Category.COMPARISON, SqlKind.IS_NOT_FALSE),  // Returns true for TRUE and NULL
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies the semantic behavior of IS_NOT_TRUE and IS_NOT_FALSE in SQL three-valued logic. However, it only asks to verify/ensure correct handling through comments rather than proposing actual implementation changes, limiting its immediate impact.

Medium

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 86e2c1d: 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 5c58a4d

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 50b9541

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 50b9541: SUCCESS

@mch2 mch2 merged commit 055d09b into opensearch-project:main Jun 11, 2026
16 checks passed
KhishorekumarBS pushed a commit to KhishorekumarBS/OpenSearch that referenced this pull request Jul 3, 2026
* fix: add scalars for is with bool

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* add some test queries

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

* also add is_true/etc as project functions

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

---------

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

analytics-engine Issues and PR for the new Analytics engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants