Updated EXPM1() and Tests to New Engine by matthewryanwells · Pull Request #215 · Bit-Quill/opensearch-project-sql

matthewryanwells · 2023-02-02T17:20:39Z

Signed-off-by: Matthew Wells matthew.wells@improving.com

Description

Updated EXPM1 function to run on the new engine, as well as added integration tests and unit tests

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.

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

GumpacG · 2023-02-02T17:53:29Z

Actions are failing due to checkstyle.

Yury-Fridlyand

What about PPL?

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

...src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java

GumpacG · 2023-02-02T18:15:42Z

What about PPL?

opensearch-project#1097 for reference

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

codecov · 2023-02-02T18:46:21Z

Codecov Report

Merging #215 (e5a7001) into integ-expm1 (af188a3) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@                Coverage Diff                @@
##             integ-expm1     #215      +/-   ##
=================================================
- Coverage          98.36%   98.36%   -0.01%     
+ Complexity          3643     3636       -7     
=================================================
  Files                343      343              
  Lines               9017     8990      -27     
  Branches             585      585              
=================================================
- Hits                8870     8843      -27     
  Misses               142      142              
  Partials               5        5

Flag	Coverage Δ
sql-engine	`98.36% <100.00%> (-0.01%)`	⬇️

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%> (ø)`
...ssion/operator/arthmetic/MathematicalFunction.java	`100.00% <100.00%> (ø)`

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

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

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

matthewryanwells · 2023-02-02T19:42:38Z

Actions are failing due to checkstyle.

the checkstyle was failing because of the lack of a newline which I have now fixed

matthewryanwells · 2023-02-02T20:51:23Z

What about PPL?

opensearch-project#1097 for reference

I have added in PPL, including testing and documentation

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

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

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

Yury-Fridlyand

You can rebase on upstream/main to make CI pass

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

matthewryanwells · 2023-02-07T17:30:43Z

You can rebase on upstream/main to make CI pass

I have now rebased the changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

...src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java

docs/user/dql/functions.rst

docs/user/ppl/functions/math.rst

GumpacG · 2023-02-07T18:45:38Z

I would suggest to have the math function base extraction be a part of a different task so that more tests can be included for each of the types that the functions supports. This would prevent the task and PR from becoming large.

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

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

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

…ns to use the base Signed-off-by: Matthew Wells <matthew.wells@improving.com>

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

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

GumpacG · 2023-02-09T00:04:41Z

docs/user/ppl/functions/math.rst

    | 7.38905609893065 |
    +------------------+

-


Accidental deletion?

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

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

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

...src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java

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

… done in one commit Signed-off-by: Matthew Wells <matthew.wells@improving.com>

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

* Updated EXPM1() and tests to new engine Signed-off-by: Matthew Wells <matthew.wells@improving.com>

…) (opensearch-project#1348) * Updated EXPM1() and tests to new engine Signed-off-by: Matthew Wells <matthew.wells@improving.com> (cherry picked from commit e990201) Co-authored-by: Matthew Wells <matthew.wells@improving.com>

Updated EXPM1() and tests to new engine

e2a6901

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

Yury-Fridlyand requested review from GabeFernandez310, GumpacG, MaxKsyunz, Yury-Fridlyand, acarbonetto, forestmvey and margarit-h February 2, 2023 17:50

Yury-Fridlyand reviewed Feb 2, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Show resolved Hide resolved

...src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java Outdated Show resolved Hide resolved

Added EXPM1 to PPL Parser

e357cdd

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

matthewryanwells added 2 commits February 2, 2023 11:05

Added documentation for EXPM1 PPL and fixed small mistake in EXP docu…

c152a95

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

Added newline to fix code style issue

b5ab93e

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

matthewryanwells closed this Feb 2, 2023

matthewryanwells reopened this Feb 2, 2023

removed unnecessary integration tests and fixed typo in another test

2958c3e

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

Yury-Fridlyand approved these changes Feb 2, 2023

View reviewed changes

Extracted common code from math functions into external function

3c856a3

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

GumpacG reviewed Feb 6, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Outdated Show resolved Hide resolved

matthewryanwells added 2 commits February 6, 2023 09:05

Added javadoc comment to new function

b1c9fde

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

Improved Javadoc

457a5a6

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

Yury-Fridlyand approved these changes Feb 6, 2023

View reviewed changes

Pulling changes from upstream

f013e7d

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

GumpacG approved these changes Feb 7, 2023

View reviewed changes

GabeFernandez310 approved these changes Feb 7, 2023

View reviewed changes

acarbonetto requested changes Feb 7, 2023

View reviewed changes

forestmvey approved these changes Feb 7, 2023

View reviewed changes

Added and improved EXPM1 tests, and improved documentation for CLI an…

6d09e77

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

Yury-Fridlyand reviewed Feb 8, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Show resolved Hide resolved

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Outdated Show resolved Hide resolved

matthewryanwells added 4 commits February 8, 2023 08:00

Removed EXPM1 from PPL

eecd489

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

Updated math base function to take any type and included more functio…

8b88d1c

…ns to use the base Signed-off-by: Matthew Wells <matthew.wells@improving.com>

Made tiny change to documentation

7a028d4

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

reverted euler function to previous iteration as the one I made didn'…

4574275

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

GumpacG reviewed Feb 9, 2023

View reviewed changes

docs/user/ppl/functions/math.rst

| 7.38905609893065 |

+------------------+

Copy link
Copy Markdown

GumpacG Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental deletion?

GumpacG reviewed Feb 9, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Outdated Show resolved Hide resolved

GumpacG reviewed Feb 9, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Outdated Show resolved Hide resolved

matthewryanwells added 3 commits February 9, 2023 10:38

fixed typo in a comment that I made in a previous commit

62cc771

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

fixed a mistake I made in fixing the typo from my last commit

689a1dc

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

Fixed spacing inconsistencies in math.rst file

5a3b724

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

acarbonetto approved these changes Feb 10, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java Outdated Show resolved Hide resolved

...src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java Show resolved Hide resolved

Yury-Fridlyand approved these changes Feb 10, 2023

View reviewed changes

matthewryanwells added 3 commits February 10, 2023 09:22

removed the base math function so that it can be pushed in a seperate…

8042ebe

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

Reverted reverted change that removes the refactoring as this will be…

a74ae35

… done in one commit Signed-off-by: Matthew Wells <matthew.wells@improving.com>

updated base function to have more descriptive variables

e5a7001

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

matthewryanwells merged commit b82d5b8 into integ-expm1 Feb 13, 2023

matthewryanwells deleted the dev-expm1 branch February 13, 2023 17:25

GumpacG pushed a commit that referenced this pull request Feb 14, 2023

Updated EXPM1() and Tests to New Engine (#215) (opensearch-project#1334)

e990201

* Updated EXPM1() and tests to new engine Signed-off-by: Matthew Wells <matthew.wells@improving.com>

Conversation

matthewryanwells commented Feb 2, 2023

Description

Check List

Uh oh!

GumpacG commented Feb 2, 2023

Uh oh!

Yury-Fridlyand left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GumpacG commented Feb 2, 2023

Uh oh!

codecov bot commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matthewryanwells commented Feb 2, 2023

Uh oh!

matthewryanwells commented Feb 2, 2023

Uh oh!

Uh oh!

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Uh oh!

matthewryanwells commented Feb 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GumpacG commented Feb 7, 2023

Uh oh!

Uh oh!

Uh oh!

GumpacG Feb 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Yury-Fridlyand left a comment •

edited

Loading

codecov bot commented Feb 2, 2023 •

edited

Loading