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 valueOf() to Expression #1055

Merged
merged 2 commits into from
Nov 9, 2022
Merged

Conversation

joshuali925
Copy link
Member

Signed-off-by: Joshua Li joshuali925@gmail.com

Description

Minor change to use valueOf() instead of valueOf(null)

Issues Resolved

closes #976

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: Joshua Li <joshuali925@gmail.com>
@joshuali925 joshuali925 requested a review from a team as a code owner November 9, 2022 00:00
@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.71%. Comparing base (8c91689) to head (f479a60).
Report is 459 commits behind head on 2.x.

Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #1055      +/-   ##
============================================
- Coverage     98.27%   95.71%   -2.57%     
- Complexity     3344     3351       +7     
============================================
  Files           326      337      +11     
  Lines          8443     9115     +672     
  Branches        553      672     +119     
============================================
+ Hits           8297     8724     +427     
- Misses          142      334     +192     
- Partials          4       57      +53     
Flag Coverage Δ
query-workbench 62.76% <ø> (?)
sql-engine 98.27% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dai-chen dai-chen added the maintenance Improves code quality, but not the product label Nov 9, 2022
derek-ho
derek-ho previously approved these changes Nov 9, 2022
Copy link
Collaborator

@derek-ho derek-ho left a comment

Choose a reason for hiding this comment

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

LGTM

Yury-Fridlyand
Yury-Fridlyand previously approved these changes Nov 9, 2022
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Missing:

  • QueryRangeFunctionImplementation::valueOf and its calls
  • QueryRangeFunctionImplementation::buildQueryFromQueryRangeFunction
  • TimeRangeParametersResolver::visitFunction

Signed-off-by: Joshua Li <joshuali925@gmail.com>
@joshuali925 joshuali925 dismissed stale reviews from Yury-Fridlyand and derek-ho via f479a60 November 9, 2022 01:20
@joshuali925
Copy link
Member Author

Missing:

  • QueryRangeFunctionImplementation::valueOf and its calls
  • QueryRangeFunctionImplementation::buildQueryFromQueryRangeFunction
  • TimeRangeParametersResolver::visitFunction

Thanks, i forgot i git ignored /prometheus directory.. updated PR

Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

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

Thanks for refactoring this!

@joshuali925 joshuali925 merged commit 03f30e3 into opensearch-project:2.x Nov 9, 2022
dai-chen added a commit that referenced this pull request Nov 9, 2022
* Fix `FLOAT` -> `DOUBLE` cast. (#1025)

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>

* Fix error messaging from prometheus. (#1029) (#1037)

Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
(cherry picked from commit 4a9cef3)

Co-authored-by: vamsi-amazon <reddyvam@amazon.com>

* Add `query` function as alternate syntax to `query_string` function (#1010)

This maintains backwards compatibility with the v1 engine.

* Update DATE and TIME functions to parse string input as datetime (#991)

Add option to accept datetime like string in both TIME and DATE (eg. accept "1999-01-02 12:12:12" for both TIME and DATE.

Strict check on date for testing for valid dates (eg. Don't accept Feb 30th as a valid date) and throws a SemanticCheckException.

Co-authored-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>

* back quote fix (#1041) (#1050)

Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
(cherry picked from commit d3bb902)

Co-authored-by: vamsi-amazon <reddyvam@amazon.com>

* Catalog to Datasource changes (#1027) (#1049)

Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
(cherry picked from commit 3e30379)

* Bump jackson to 2.14.0 (#1058)

Signed-off-by: Joshua Li <joshuali925@gmail.com>
(cherry picked from commit 5a1adb2)

* Add valueOf() to Expression (#1055)

Signed-off-by: Joshua Li <joshuali925@gmail.com>

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
Co-authored-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>
Co-authored-by: vamsi-amazon <reddyvam@amazon.com>
Co-authored-by: GabeFernandez310 <64448015+GabeFernandez310@users.noreply.github.com>
Co-authored-by: MitchellGale-BitQuill <104795536+MitchellGale-BitQuill@users.noreply.github.com>
Co-authored-by: Joshua Li <joshuali925@gmail.com>
vmmusings pushed a commit to vmmusings/sql that referenced this pull request Nov 12, 2022
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
vmmusings pushed a commit to vmmusings/sql that referenced this pull request Nov 12, 2022
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Improves code quality, but not the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor ExprValue so valueEnv is not required
6 participants