Outdated operation precedence.

[mykhailynab]

Here's an expression: SELECT ~1 + 2. If you plug it into PostgreSQL interpreter, here's what the output looks like:

postgres=# select ~1 + 2;
 ?column? 
----------
       -4
(1 row)

postgres=# select ~(1 + 2);
 ?column? 
----------
       -4
(1 row)

postgres=# select (~1) + 2;
 ?column? 
----------
        0
(1 row)

As you can see here, and according to operation precedence listed in documentation, the + operation is more powerful than the ~ operation, and thus we have PGBitwiseNot(1 + 2). However, if we plug this into the parser, we get the following AST:

...
projection: [
    UnnamedExpr(
        BinaryOp {
            left: UnaryOp {
                op: PGBitwiseNot,
                expr: Value(
                    Number(
                        "1",
                        false,
                    ),
                ),
            },
            op: Plus,
            right: Value(
                Number(
                    "2",
                    false,
                ),
            ),
        },
    ),
],
...

Which is implying (~1) + 2 instead of ~(1 + 2). This should be fixed.

UPDATE:
In code, I found a reference to an old PostgreSQL precedence table, using which the parser is built. However, this table is for a very old (7.0, released May 8, 2000) version of PostgreSQL. There latest version is 15.1. Shouldn't this be updated?

[AugustoFKL]

@michael-2956, you're right, this has been a problem for a while. Not sure if we have an issue with that, but that is known.

I don't have the time to search about it and implement it, since the precedence may vary from each database, and it's quite a complex part of the code. But I'd love to review it if you feel like doing a PR for it!

[mykhailynab]

@AugustoFKL, Is it ok to change lots of code in parser.rs to match PostgreSQL 15.1 standards? Won't this break any other dialects? I thought so, so I implemented the right precedence in the Postgres dialect in my fork on the precedence-patch branch. This is just a temporary solution for myself, but you can look at the changes I made if you want. I'm still testing it.

[AugustoFKL]

@michael-2956 I prefer to add permissiveness over break backward compatibility.

If the precedence can't be generic for all dialects, I think it's better to specialize it. Of course, this isn't something simple at all, but I don't like the idea of breaking what we have right now.

[mykhailynab]

@AugustoFKL So, putting it into a dialect was a right decision after all?

There is a lot of redundancy in the dialect code on my fork, but I've tested it and I think it works fine (at least in my use cases).

And, in summary, I did have to change some parsing code to fix precedence issues, but mostly I just changed the numbers.

I think that those non-numbers issues I've come to fix were not exclusive to Postgres, so maybe I should move those fixes to parser.rs, and, possibly through some kind of a get_precedence() function, make it possible for the dialect to have its own precedence. Would that be ok?

Unfortunately, modifying get_next_precedence turned out to be not enough for me. That's because the 'numbers' are not only used in the function but all over the parsing code. get_precedence(Token) -> u8 is one possible way to solve this.

[AugustoFKL]

@michael-2956, that would be ok... I'll take a look at your code this week, but, if the changes don't cause significant API changes or break backward compatibility, I don't see the problem.

[samuelcolvin]

I'm not sure if this is the same issue, but currently = is binding more strongly than -> and ->>.

use sqlparser::dialect::PostgreSqlDialect;
use sqlparser::parser::Parser;

fn main() {

    let sql = r#"SELECT 'foo'->'bar'='spam'"#;

    let dialect = PostgreSqlDialect {};

    let ast = Parser::parse_sql(&dialect, sql).unwrap();

    dbg!(ast);
}

Contains:

                        UnnamedExpr(
                            BinaryOp {
                                left: Value(
                                    SingleQuotedString(
                                        "foo",
                                    ),
                                ),
                                op: Arrow,
                                right: BinaryOp {
                                    left: Value(
                                        SingleQuotedString(
                                            "bar",
                                        ),
                                    ),
                                    op: Eq,
                                    right: Value(
                                        SingleQuotedString(
                                            "spam",
                                        ),
                                    ),
                                },
                            },
                        ),