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

Operator Precedence Inconsistencies #683

Closed
johnedquinn opened this issue Jul 25, 2022 · 2 comments
Closed

Operator Precedence Inconsistencies #683

johnedquinn opened this issue Jul 25, 2022 · 2 comments
Assignees
Labels
bug Something isn't working documentation

Comments

@johnedquinn
Copy link
Member

johnedquinn commented Jul 25, 2022

Description

While working on replacing the SqlLexer and SqlParser, I was trying to figure out the order of precedence for operators, and there is a clear inconsistency with the way the current parser handles certain operators.

Also, since we pass all the tests, our tests are also wrong and need to be fixed.

Here's a breakdown:

  • BETWEEN and LIKE have the SAME precedence as <, >, <=, and >=.
  • BETWEEN and LIKE have the SAME precedence as IS.
  • BETWEEN and LIKE have HIGHER precedence than IN.
  • IN has the SAME precedence as =, !=, and <>.

Now, our issue seemingly comes from IS.

  • BETWEEN and LIKE have the SAME precedence as IS.
  • IS has the SAME precedence as IN. This doesnt' make sense because BETWEEN/LIKE have HIGHER precedence than IN.
  • IS has the SAME precedence as <, >, <=, and >=.
  • IS has the SAME precedence as =, !=, and <>.

TLDR

So, TLDR, we have a problem with inconsistency in our grammar. This can be resolved be creating a formal precedence table.

Visual TLDR:

Screen Shot 2022-07-25 at 11 11 21 AM

Textual TLDR:

  • IS has the SAME precedence as BETWEEN/LIKE, but they both have different precedences in comparison to < and IN.

To Reproduce

The following shows that IS and = have the SAME precedence.

PartiQL> a IS BOOL = b
!!
PartiQL> a = b IS BOOL
!!

The following shows that IS and LIKE have the SAME precedence

PartiQL> a IS BOOL LIKE c
!!
PartiQL> a LIKE c IS BOOL
!!

The following shoes that LIKE has HIGHER precedence than =

PartiQL> a = b LIKE c
!!
PartiQL> a LIKE b = c
!!

NOTE: You can see more inconsistencies when you use IN and BETWEEN also.

NOTE: When the AST is printed out with !!, whichever operator is nested is the one with higher precedence (if you test both back and forth). If the LHS is always nested, then we can say that the two operators have equal precedence.

Why

To me, this should be prioritized. PartiQL is a language, and our basic operations don't have an established precedence yet.

To Fix

  • Define a precedence table
  • Fix the parser (either the old one, or just the new one)
  • We need to update documentation to define a formal precedence table to convery to our customers
@johnedquinn johnedquinn added bug Something isn't working documentation labels Jul 25, 2022
@johnedquinn johnedquinn self-assigned this Jul 25, 2022
@jpschorr
Copy link
Contributor

the partiql-lang-rust parser defines the precedence levels as:

// PartiQL's precedence levels:
// |-------+-------------------+---------------+------------------------------------|
// | Level | Operator          | Associativity | Description                        |
// |-------+-------------------+---------------+------------------------------------|
// |     1 | <Path Expression> | left          | e.g., `field`,  `binding.field[2]` |
// |     2 | <Function call>   | left          | e.g., `upper(field_reference)`     |
// |     3 | + -               | right         | unary plus, unary minus            |
// |     4 | ^                 | left          | exponentiation                     |
// |     5 | * / %             | left          | multiplication, division, modulo   |
// |     6 | + -               | left          | addition, subtraction              |
// |     7 | <other>           | left          | other operators, e.g., `||`
// |     8 | BETWEEN IN LIKE   |               | range/set/pattern compare          |
// |     9 | < > <= >=         |               | comparison operators               |
// |    10 | = <> !=           |               | equality operators                 |
// |    11 | IS                |               | IS [NOT] NULL                      |
// |    12 | NOT               | right         | logical negate                     |
// |    13 | AND               | left          | logical conjuct                    |
// |    14 | OR                | left          | logical disjunct                   |
// |-------+-------------------+---------------+------------------------------------|

-- https://github.com/partiql/partiql-lang-rust/blob/main/partiql-parser/src/parse/partiql.lalrpop#L462-L480

@johnedquinn
Copy link
Member Author

Closing this as the new PartiQLParser handles this.

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

No branches or pull requests

2 participants