Skip to content

Conversation

@LantaoJin
Copy link
Member

@LantaoJin LantaoJin commented Apr 16, 2025

Description

Support CASE function with Calcite (v3) and v2 engine.

  1. Its syntax should be support in PPL OpenSearch as well.
    https://github.com/opensearch-project/opensearch-spark/blob/6832737577119221819c49235c212020c0214fed/ppl-spark-integration/src/main/antlr4/OpenSearchPPLParser.g4#L489-L491

  2. Fix the bug of string literal: enforce to CHAR(1) type or VARCHAR type to avoid CHAR(n)

Related Issues

Resolves #3536

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
dai-chen
dai-chen previously approved these changes Apr 17, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
qianheng-aws
qianheng-aws previously approved these changes Apr 21, 2025
@penghuo
Copy link
Collaborator

penghuo commented Apr 21, 2025

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
Copy link
Member Author

Update doc? https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/functions/condition.rst

Do you know the reason to make documentation executable? It's hard to debug if doctest fails. @penghuo

@penghuo
Copy link
Collaborator

penghuo commented Apr 22, 2025

Do you know the reason to make documentation executable? It's hard to debug if doctest fails. @penghuo

doctest has grown to have three primary uses, more reading opendistro-for-elasticsearch/sql#497

  • Checking examples in docstrings.
  • Regression testing.
  • Executable documentation / literate testing.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
os> source=accounts | eval result = case(age > 35, firstname, age < 30, lastname else employer) | fields result, firstname, lastname
fetched rows / total rows = 4/4
+--------+-----------+----------+
| result | firstname | lastname |
Copy link
Collaborator

Choose a reason for hiding this comment

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

add age and employer, fields result, age, firstname, lastname, employer

Copy link
Member Author

Choose a reason for hiding this comment

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

done

| null | Dale | Adams |
+--------+-----------+----------+

os> source=accounts | eval result = case(age > 35, firstname, age < 30, lastname) | fields result, firstname, lastname
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto, add age

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin LantaoJin requested a review from penghuo April 24, 2025 06:41
@penghuo penghuo merged commit 47e6a2a into opensearch-project:main Apr 24, 2025
22 checks passed
@LantaoJin
Copy link
Member Author

LantaoJin commented Apr 25, 2025

I will backport this to branch 3.0 since it should be treated as bug:

  1. CASE function is a supported function in SQL query, this PR not only fixed it in PPL query with v3 (enable Calcite), but also fixed it in v2 (disable Calcite).
  2. This PR also fixed the bug of string literal which is critical for v3.

Can you approve it? @penghuo @anasalkouz @model-collapse
Also add related requirement link: #3523 (comment)

@model-collapse
Copy link

Approved to backport

opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 25, 2025
* Support CASE function with Calcite

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* add anonymizer

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* address comment and fix varchar literal bug

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* add doc

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix doctest

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix doctest

Signed-off-by: Lantao Jin <ltjin@amazon.com>

---------

Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 47e6a2a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
penghuo pushed a commit that referenced this pull request Apr 25, 2025
* Support CASE function with Calcite



* add anonymizer



* address comment and fix varchar literal bug



* add doc



* fix doctest



* fix doctest



---------


(cherry picked from commit 47e6a2a)

Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
penghuo pushed a commit that referenced this pull request Jun 16, 2025
* Support CASE function with Calcite

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* add anonymizer

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* address comment and fix varchar literal bug

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* add doc

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix doctest

Signed-off-by: Lantao Jin <ltjin@amazon.com>

* fix doctest

Signed-off-by: Lantao Jin <ltjin@amazon.com>

---------

Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: xinyual <xinyual@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 3.0 calcite calcite migration releated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Support CASE function with Calcite

5 participants