Skip to content

Updated Arithmetic functions from old engine to new engine#235

Merged
matthewryanwells merged 45 commits intointeg-arithmeticfrom
dev-arithmetic
Mar 9, 2023
Merged

Updated Arithmetic functions from old engine to new engine#235
matthewryanwells merged 45 commits intointeg-arithmeticfrom
dev-arithmetic

Conversation

@matthewryanwells
Copy link
Copy Markdown

Description

Added ADD, SUBTRACT, MULTIPLY, DIVIDE, and MODULUS to the new engine, as well as moved MOD from mathematical functions to arithmetics

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • 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.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

matthewryanwells and others added 20 commits February 10, 2023 15:30
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
… function description to mention relation to * symbol

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
* Add release notes for 2.6.0

Signed-off-by: Rupal Mahajan <maharup@amazon.com>

* Remove incorrect links

Signed-off-by: Rupal Mahajan <maharup@amazon.com>

---------

Signed-off-by: Rupal Mahajan <maharup@amazon.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…, fixed multiply being in mathematical function, updated documentation

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2023

Codecov Report

Merging #235 (7bfcfde) into integ-arithmetic (997b51a) will decrease coverage by 1.58%.
The diff coverage is 100.00%.

❗ Current head 7bfcfde differs from pull request most recent head 5a285d4. Consider uploading reports for the commit 5a285d4 to get more accurate results

@@                  Coverage Diff                   @@
##             integ-arithmetic     #235      +/-   ##
======================================================
- Coverage               99.96%   98.38%   -1.58%     
- Complexity               2467     3696    +1229     
======================================================
  Files                     220      343     +123     
  Lines                    5833     9110    +3277     
  Branches                  359      585     +226     
======================================================
+ Hits                     5831     8963    +3132     
- Misses                      2      142     +140     
- Partials                    0        5       +5     
Flag Coverage Δ
sql-engine 98.38% <100.00%> (-1.58%) ⬇️

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

Impacted Files Coverage Δ
...c/main/java/org/opensearch/sql/expression/DSL.java 100.00% <100.00%> (ø)
...h/sql/expression/function/BuiltinFunctionName.java 100.00% <100.00%> (ø)
...ression/operator/arthmetic/ArithmeticFunction.java 100.00% <100.00%> (ø)

... and 123 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…on to account for that change

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…, fixed multiply being in mathematical function, updated documentation

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…ypes

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…cfunctions

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@GumpacG
Copy link
Copy Markdown

GumpacG commented Mar 2, 2023

Can you add a test where the function has more than two arguments. For example, SELECT ADD(1,1,1).
Legacy has a bug where it takes more that two arguments but it doesn't calculate anything after the second argument.
Screenshot 2023-03-02 at 11 15 38 AM

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
Copy link
Copy Markdown
Author

Can you add a test where the function has more than two arguments. For example, SELECT ADD(1,1,1). Legacy has a bug where it takes more that two arguments but it doesn't calculate anything after the second argument. Screenshot 2023-03-02 at 11 15 38 AM

I have added a test to confirm that all functions fail when working with more than two parameters

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
…rformance hit was too large

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells matthewryanwells marked this pull request as ready for review March 7, 2023 17:39
@matthewryanwells matthewryanwells merged commit 9458adc into integ-arithmetic Mar 9, 2023
@matthewryanwells matthewryanwells deleted the dev-arithmetic branch March 9, 2023 19:19
matthewryanwells added a commit that referenced this pull request Mar 9, 2023
* Updated ADD, SUBTRACT, MULTIPLY, DIVIDE, MODULUS to V2 engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
GabeFernandez310 pushed a commit that referenced this pull request Mar 16, 2023
* Updated Arithmetic functions from old engine to new engine (#235)

* Updated ADD, SUBTRACT, MULTIPLY, DIVIDE, MODULUS to V2 engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
MaxKsyunz pushed a commit that referenced this pull request Mar 29, 2023
…ensearch-project#1448)

* Updated Arithmetic functions from old engine to new engine (#235)

* Updated ADD, SUBTRACT, MULTIPLY, DIVIDE, MODULUS to V2 engine

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
(cherry picked from commit bc39346)

Co-authored-by: Matthew Wells <matthew.wells@improving.com>
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.

6 participants