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

Update grammar validation settings #3165

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

ykmr1224
Copy link
Collaborator

@ykmr1224 ykmr1224 commented Nov 20, 2024

Description

  • Update grammar validation settings
  • Deny-list bitwise function for S3Glue
  • Generator function enum is added for other datasource.

Related Issues

n/a

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: Tomoyuki Morita <[email protected]>
@@ -561,12 +561,18 @@ private void validateFunctionAllowed(String function) {
case MAP:
validateAllowed(GrammarElement.MAP_FUNCTIONS);
break;
case BITWISE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we need to keep adding case and there is no way to convert function to grammar element?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can have a map if we want. Let me try that approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me go ahead with the current approach for now, I found some test case conflict if I enable validation for all the function types.

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.

the PR content lgtm. there are some code refactor like #3165 (comment)

@ykmr1224 ykmr1224 merged commit 8b2d01e into opensearch-project:main Nov 21, 2024
16 of 17 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Tomoyuki Morita <[email protected]>
(cherry picked from commit 8b2d01e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ykmr1224 pushed a commit that referenced this pull request Nov 21, 2024
(cherry picked from commit 8b2d01e)

Signed-off-by: Tomoyuki Morita <[email protected]>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x maintenance Improves code quality, but not the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants