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

[BUG] Issues in PPL grammar #3391

Open
vikhy-aws opened this issue Mar 6, 2025 · 5 comments
Open

[BUG] Issues in PPL grammar #3391

vikhy-aws opened this issue Mar 6, 2025 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@vikhy-aws
Copy link

vikhy-aws commented Mar 6, 2025

What is the bug?
PPL grammar currently does not account for precedence for logical operators (AND, OR, XOR, NOT). Also, usage of parentheses is not supported in logical expressions. Also, the grammar is not case-agnostic, only supports uppercase.

How can one reproduce the bug?
All these can be simulated using antlr parser using sample PPL queries and the grammar.

What is the expected behavior?
Precedence must be accounted for logical operators according to the rules of the language. Usage of parentheses must be allowed in logical expressions. Also, support case-agnostic queries.

What is your host/environment?

  • OS: Mac
  • Version [e.g. 22]
  • Plugins

Do you have any screenshots?

  1. The following screenshot shows that operator precedence is not as expected. We would expect the AND operator to be evaluated first before OR.
    Query Used: SOURCE=TEST | WHERE A1 = 'V1' AND A2 = 'V2' AND A3 = 'V3' OR A4 = 'V4'
    Image
  2. The following screenshot show that usage of parentheses is not supported in logical expressions.
    Image
  3. The following screenshots show that lowercase and mixed-case queries are not supported
    Image
    Image

Do you have any additional context?
Precedence can be enforced by having the grammar to generate lower precedence operators first and generate higher precedence operators further down the parse tree. Currently, they are being generated at the same level in the grammar which is causing the issues with precedence.
Support for case-agnostic grammar can be supported using fragments.

@vikhy-aws vikhy-aws added bug Something isn't working untriaged labels Mar 6, 2025
@LantaoJin
Copy link
Member

LantaoJin commented Mar 7, 2025

Thanks for opening issue @vikhy-aws , here is my thoughts

  1. this is parsing order, not execution order, the expression will be evaluated in correct order in planning stage.
  2. this is an known issue which had be fixed in this PR, will merge to main branch soon. But, seems it needs a patch for 2.x as well.
  3. Seems the ANTLR plugin problem, not quite sure.

@LantaoJin
Copy link
Member

For 2, the fixing depends on merging dev branch to main. It reminds me to backport them to 2.x, added an item in #3394.

@vikhy-aws
Copy link
Author

vikhy-aws commented Mar 7, 2025

Thanks for the clarification @LantaoJin. To provide a little context, I'm using only the grammar to generate parse tree and use that parse tree. So for 1, would it be possible to ensure that the precedence rules are accounted for in the grammar?

Thanks for the PR for 2, and for 3 I think it's handled in the before the lexer I guess. So, should be fine.

@LantaoJin
Copy link
Member

So for 1, would it be possible to ensure that the precedence rules are accounted for in the grammar?

I do think so. But seems not a problem for running a PPL query. Do you want to use this grammar file to do some frontend work? If necessary, I can try to fix it. Let's use this issue to track for a patch (I prefer to fix it in the mentioned dev branch first).

@vikhy-aws
Copy link
Author

Yes, I'm build upon the grammar for code generation. The fix would be helpful. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants