Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cbrt function to the PPL #171

Merged
merged 4 commits into from
Nov 23, 2022
Merged

Conversation

margarit-h
Copy link

@margarit-h margarit-h commented Nov 18, 2022

Signed-off-by: Margarit Hakobyan margarith@bitquilltech.com

Description

Calculates the cube root of a number

Argument type: INTEGER/LONG/FLOAT/DOUBLE

Return type DOUBLE:

(Non-negative) INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
(Negative) INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE

Example::

opensearchsql> source=location | eval `CBRT(8)` = CBRT(8), `CBRT(9.261)` = CBRT(9.261), `CBRT(-27)` = CBRT(-27) | fields `CBRT(8)`, `CBRT(9.261)`,`CBRT(-27)`;
fetched rows / total rows = 2/2
+-----------+---------------+-------------+
| CBRT(8)   | CBRT(9.261)   | CBRT(-27)   |
|-----------+---------------+-------------|
| 2.0       | 2.1           | -3.0        |
| 2.0       | 2.1           | -3.0        |
+-----------+---------------+-------------+

Issues Resolved

[List any issues this PR will resolve]

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.

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #171 (a709fa9) into integ-add-cbrt-to-ppl (d4d289c) will decrease coverage by 2.52%.
The diff coverage is n/a.

@@                     Coverage Diff                     @@
##             integ-add-cbrt-to-ppl     #171      +/-   ##
===========================================================
- Coverage                    98.28%   95.76%   -2.53%     
  Complexity                    3454     3454              
===========================================================
  Files                          345      355      +10     
  Lines                         8588     9246     +658     
  Branches                       547      666     +119     
===========================================================
+ Hits                          8441     8854     +413     
- Misses                         142      334     +192     
- Partials                         5       58      +53     
Flag Coverage Δ
query-workbench 62.76% <ø> (?)
sql-engine 98.28% <ø> (ø)

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

Impacted Files Coverage Δ
...ch/public/components/QueryResults/QueryResults.tsx 61.60% <0.00%> (ø)
workbench/public/components/PPLPage/PPLPage.tsx 56.52% <0.00%> (ø)
workbench/public/components/Header/Header.tsx 100.00% <0.00%> (ø)
...ublic/components/QueryResults/QueryResultsBody.tsx 68.32% <0.00%> (ø)
...h/public/components/QueryLanguageSwitch/Switch.tsx 85.71% <0.00%> (ø)
workbench/public/components/app.tsx 0.00% <0.00%> (ø)
workbench/public/components/SQLPage/SQLPage.tsx 100.00% <0.00%> (ø)
workbench/public/application.tsx 0.00% <0.00%> (ø)
workbench/public/utils/PanelWrapper.tsx 100.00% <0.00%> (ø)
workbench/public/components/Main/main.tsx 53.00% <0.00%> (ø)

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

Signed-off-by: Margarit Hakobyan <margarith@bitquilltech.com>
Signed-off-by: Margarit Hakobyan <margarith@bitquilltech.com>
Signed-off-by: Margarit Hakobyan <margarith@bitquilltech.com>
Signed-off-by: Margarit Hakobyan <margarith@bitquilltech.com>
@margarit-h margarit-h changed the title [WIP:] Add cbrt function to the PPL Add cbrt function to the PPL Nov 21, 2022

Example::

opensearchsql> source=location | eval `CBRT(8)` = CBRT(8), `CBRT(9.261)` = CBRT(9.261), `CBRT(-27)` = CBRT(-27) | fields `CBRT(8)`, `CBRT(9.261)`, `CBRT(-27)`;

Choose a reason for hiding this comment

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

If we use source=people, do we just get a single row of results?

Copy link
Author

@margarit-h margarit-h Nov 22, 2022

Choose a reason for hiding this comment

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

No, we get 12 rows, locations seems to be the shortest.
Here is the people:
opensearchsql> source=people | eval CBRT(8) = CBRT(8), CBRT(9.261) = CBRT(9.261), CBRT(-27) = CBRT(-27) | fields CBRT(8), CBRT(9.261),CBRT(-27);
fetched rows / total rows = 12/12
+-----------+---------------+-------------+
| CBRT(8) | CBRT(9.261) | CBRT(-27) |
|-----------+---------------+-------------|
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
| 2.0 | 2.1 | -3.0 |
+-----------+---------------+-------------+

@GumpacG
Copy link

GumpacG commented Nov 23, 2022

If it makes sense, can there be more tests to test field types float, long, double, null, negative. Currently only integer is being tested in IT. calcs may be nice to use for this. It has a good variety of data types and values.

@@ -662,3 +662,27 @@ Example::
| 2.0 | 2.1 |
+-----------+--------------+

CBRT

Choose a reason for hiding this comment

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

I think the functions in here might be organized in Alphabetical order? It might be good to move this function documentation up.

Copy link

@GabeFernandez310 GabeFernandez310 left a comment

Choose a reason for hiding this comment

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

Looks good!

@margarit-h margarit-h merged commit d363f60 into integ-add-cbrt-to-ppl Nov 23, 2022
Yury-Fridlyand pushed a commit that referenced this pull request Nov 29, 2022
* Add cbrt function to the PPL (#171)

Signed-off-by: Margarit Hakobyan <margarith@bitquilltech.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
4 participants