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

Disable unsupported PPL function expressions #478

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

ykmr1224
Copy link
Collaborator

@ykmr1224 ykmr1224 commented Jul 25, 2024

Description

  • Disable unsupported PPL function expressions
  • booleanExpression/relevanceExpression were missed in the previous PR
  • I tested with examples from integration tests in SQL plugin package, and found some more unsupported ones such as: percentile, take, and value expression such as 3 * 5 (since it is converted to Function).

Issues Resolved

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: Tomoyuki Morita <moritato@amazon.com>
@ykmr1224 ykmr1224 force-pushed the unavailable-commands2 branch 2 times, most recently from c07fa54 to eb36ede Compare July 25, 2024 22:20
@ykmr1224 ykmr1224 marked this pull request as ready for review July 25, 2024 22:22
@seankao-az
Copy link
Collaborator

renameCommand is also not available at the moment

Signed-off-by: Tomoyuki Morita <moritato@amazon.com>
@@ -262,7 +262,7 @@ The next samples of PPL queries are currently supported:
- `where` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/where.rst)
- `fields` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/fields.rst)
- `head` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/head.rst)
- `stats` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/stats.rst) (supports AVG, COUNT, MAX, MIN and SUM aggregation functions)
- `stats` - [See details](https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/cmd/stats.rst) (supports AVG, COUNT, DISTINCT_COUNT, MAX, MIN and SUM aggregation functions)
Copy link
Member

Choose a reason for hiding this comment

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

is Distinct count removed by mistake in earlier PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I missed that in the previous PR. (I added supports AVG,... description)

Copy link
Collaborator

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

Any test can be shared on PR?

@ykmr1224
Copy link
Collaborator Author

Here are the test cases I covered to verify. Those are picked up from integration tests in SQL plugin.

  test("succeed") {
    verifySucceed("source=%s")
    verifySucceed("source=%s | fields firstname, lastname")
    verifySucceed("source=%s | fields firstname, age | head")
    verifySucceed("source=opensearch-sql_test_index_account | where age > 30")
    verifySucceed("source=%s | sort city.name | fields city.name, city.location.latitude")
    verifySucceed("source=%s | where firstname='Amber JOHnny' and age=32 | fields firstname, age")
    verifySucceed("source=%s | where age=32 or age=34 | fields age")
    verifySucceed("source=%s | where firstname='Hattie' xor age=36 | fields firstname, age")
    verifySucceed("source=%s 32 = age | fields age")
    verifySucceed("source=%s age != 32 | fields age")
    verifySucceed("source=my_prometheus.prometheus_http_requests_total | stats avg(@value) as agg by span(@timestamp, 15s), `handler`, job")
    verifySucceed("search source=logs-7.10.0-2021.01.11")
    verifySucceed("source=%s | sort balance | fields firstname, balance")
    verifySucceed("source=%s | stats avg(age)")
    verifySucceed("source=%s | stats sum(balance)")
    verifySucceed("source=%s | stats count(account_number)")
    verifySucceed("source=%s | stats distinct_count(gender)")
    verifySucceed("source=%s | stats dc(age)")
    verifySucceed("source=%s | stats min(age)")
    verifySucceed("source=%s | stats max(age)")
    verifySucceed("source=%s | stats sum(balance) as a by state | where a > 780000")
    verifySucceed("source=%s | stats count() by span(age,10)")
    verifySucceed("source=%s | fields firstname | where firstname='Amber' | fields firstname")
  }

  test("syntax check exception") {
    verifySyntaxCheckException("source=%s | eval f = convert_tz('2008-05-15 12:00:00','+00:00','+10:00')")
    verifySyntaxCheckException("source=%s | eval `DATE('2020-09-16') >= TIME('09:07:00')` = d_t_f")
    verifySyntaxCheckException("source=%s | dedup male | fields male")
    verifySyntaxCheckException("describe logs-7.10.0-2021.01.11")
    verifySyntaxCheckException("source=my_prometheus.information_schema.tables | where LIKE(TABLE_NAME, '%http%')")
    verifySyntaxCheckException("source=_wildcard | WHERE Like(TextBody, 'test*') | fields TextBody")
    verifySyntaxCheckException("source=%s | where match_bool_prefix(phrase, 'qui') | fields phrase")
    verifySyntaxCheckException("source=%s | where match_bool_prefix(phrase, '2 tes', minimum_should_match=1, fuzziness=2) | fields phrase")
    verifySyntaxCheckException("source=%s | where match_phrase(phrase, 'quick fox') | fields phrase")
    verifySyntaxCheckException("source=%s | WHERE match_phrase_prefix(Title, 'champagne be') | fields Title")
    verifySyntaxCheckException("source=%s | eval f = log2(age) | fields f")
    verifySyntaxCheckException("source=%s | where age = 31 + 1 | fields age")
    verifySyntaxCheckException("source=%s | where age = 31 - 1 | fields age")
    verifySyntaxCheckException("source=%s | where age = 31 * 1 | fields age")
    verifySyntaxCheckException("source=%s | where age = 31 / 1 | fields age")
    verifySyntaxCheckException("source=%s | where age = 31 % 1 | fields age")
    verifySyntaxCheckException("SOURCE=%s | WHERE multi_match([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste') | fields Id")
    verifySyntaxCheckException("source=%s | where simple_query_string(['*Date'], '2014-01-22')")
    verifySyntaxCheckException("source=%s | where simple_query_string(['*Date'], '2014-01-22')")
    verifySyntaxCheckException("source=%s | where (age+2) * 3 / 2 - 1 = 50 | fields age")
    verifySyntaxCheckException("source=%s like(firstname, 'Hatti_') | fields firstname")
    verifySyntaxCheckException("source=%s | parse email '.+@(?<host>.+)' | fields email, host")
    verifySyntaxCheckException("source=%s | eval f=position(str3 IN str2) | where str2 IN ('one', 'two', 'three')| fields f")
    verifySyntaxCheckException("source=%s | where query_string(['*'], 'taste')")
    verifySyntaxCheckException("source=%s | rare state by gender")
    verifySyntaxCheckException("SOURCE=%s | WHERE simple_query_string(['Tags'], 'taste') | fields Id")
    verifySyntaxCheckException("SOURCE=%s | WHERE multi_match([Title], 'beer taste', operator='OR')")
    verifySyntaxCheckException("SOURCE=%s | WHERE match(Title, 'beer taste', operator='OR')")
    verifySyntaxCheckException("show datasources | where CONNECTOR_TYPE='PROMETHEUS'")
    verifySyntaxCheckException("source=%s | stats avg(abs(age)) as AGE")
    verifySyntaxCheckException("source=%s | stats avg(abs(age) * 2.0)")
    verifySyntaxCheckException("source=%s | eval `str` = typeof('pewpew'), `double` = typeof(1.0), `int` = typeof(12345)," +
      " `long` = typeof(1234567891011), `interval` = typeof(INTERVAL 2 DAY) | fields `str`, `double`, `int`, `long`, `interval`")
    verifySyntaxCheckException("source=%s | eval f=name regexp 'xxx' | fields f")
    verifySyntaxCheckException("source=%s | top 1 state by gender")
    verifySyntaxCheckException("source=%s | where isnull(age) | fields firstname")
    verifySyntaxCheckException("source=%s | where isnotnull(age) and like(firstname, 'Ambe_') | fields firstname")
    verifySyntaxCheckException("source=%s | stats percentile(balance, 50)")
    verifySyntaxCheckException("source=%s | stats take(firstname)")
  }

Copy link
Collaborator

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

@penghuo penghuo merged commit 98bd79a into opensearch-project:main Jul 26, 2024
4 checks passed
@LantaoJin
Copy link
Member

LantaoJin commented Jul 26, 2024

Why we need to remove function expressions, I have to add almost of expressions back for supporting #448. Removing unsupported commands should be satisfied.

@ykmr1224
Copy link
Collaborator Author

Why we need to remove function expressions, I have to add almost of expressions back for supporting #448. Removing unsupported commands should be satisfied.

Intention is to make it align with current available syntax.
Even if we implement them soon, It would take time until actual Flint users can use it.

@dai-chen dai-chen added bug Something isn't working 0.5 backport 0.4 labels Jul 26, 2024
@opensearch-trigger-bot
Copy link

The backport to 0.4 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/opensearch-spark/backport-0.4 0.4
# Navigate to the new working tree
pushd ../.worktrees/opensearch-spark/backport-0.4
# Create a new branch
git switch --create backport/backport-478-to-0.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 98bd79a6ea1d9587d67ae761d417b0db4242a815
# Push it to GitHub
git push --set-upstream origin backport/backport-478-to-0.4
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/opensearch-spark/backport-0.4

Then, create a pull request where the base branch is 0.4 and the compare/head branch is backport/backport-478-to-0.4.

ykmr1224 added a commit to ykmr1224/opensearch-spark that referenced this pull request Jul 29, 2024
Signed-off-by: Tomoyuki Morita <moritato@amazon.com>
(cherry picked from commit 98bd79a)
dai-chen pushed a commit that referenced this pull request Jul 30, 2024
Signed-off-by: Tomoyuki Morita <moritato@amazon.com>
(cherry picked from commit 98bd79a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.5 backport 0.4 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants