Repository navigation
fix: bind bitwise operators looser than + and - - #2833
Merged
manticore-projects merged 1 commit intoOct 18, 2026
Merged
Conversation
Since the Pratt rewrite, &, |, #, << and >> share the precedence of + and -, so 1 & 7 - 2 parses as (1 & 7) - 2 and 1 << n - 1 as (1 << n) - 1. PostgreSQL, MySQL and SQLite all bind + and - tighter, as did JSqlParser 5.3, where BitwiseAndOr was built from AdditiveExpression operands. Give the bitwise operators their own level below + - ||, the level that PostgreSQL's || and JSON operators already use with Dialect.POSTGRESQL. A space separated | | concat keeps the level of +. 1&2||3 is now 1 & (2 || 3) by default, as || binds like +, and (1 & 2) || 3 with Dialect.POSTGRESQL, as in PostgreSQL.
Contributor
|
Thank you much! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since the Pratt rewrite (5.4),
&,|,#,<<and>>have the same precedence as+and-. That gives1 & 7 - 2as(1 & 7) - 2and1 << n - 1as(1 << n) - 1. PostgreSQL, MySQL and SQLite all bind+and-tighter:1 & 7 - 21=1 & (7 - 2)11(1 & 7) - 21 & (7 - 2)1 << 2 + 18=1 << (2 + 1)88(1 << 2) + 11 << (2 + 1)a + b >> 2(a + b) >> 25.3 got this right:
BitwiseAndOrtookAdditiveExpressionoperands.The change gives
& | # << >>their own level (prec 4), below+ - ||(prec 5). WithDialect.POSTGRESQL, PostgreSQL's||and its JSON operators already use prec 4. So in that dialect, all of PostgreSQL's "other operators" now share a single level, which matches PostgreSQL. A space-separated| |concat keeps the level of+. The bitwise operators share one left-associative level, as in PostgreSQL and SQLite. MySQL separates|,&and the shifts into different levels, and this PR doesn't change how that case is handled.One existing assertion changes.
testGetSignexpected1&2||3to parse as(1 & 2) || 3. Outside the PostgreSQL dialect,||now binds like+(since the Pratt rewrite), and&binds looser than+. So||binds tighter than&and the expression parses as1 & (2 || 3). That is also how SQLite reads it. WithDialect.POSTGRESQLthe result is still(1 & 2) || 3, as in PostgreSQL, whereSELECT 1 & 2 || 'x'gives'0x'. The test now asserts both. Please tell me if you'd rather keep the old default for that mix.Also unsure: in T-SQL,
&and|sit at the+ -level, so withDialect.SQLSERVERthe master behaviour was right fora & b + c. 5.3 behaved like this PR. If you'd like, I can add aDialect.SQLSERVERbranch.Tests: three cases are added to
ExpressionPrecedenceTest, covering&,|,<<,>>,#(PostgreSQL dialect) against+ -, the shared bitwise level,| |concat, and a round trip.testGetSignis updated as described above.What was run:
./gradlew test --tests ExpressionPrecedenceTestwithout the grammar change:testBitwiseOperatorsBindLooserThanAdditionandtestSpaceSeparatedConcatStillBindsLikeAdditionfail (expected: <BitwiseAnd> but was: <Subtraction>/<Concat>). With the change, both pass../gradlew testwith the change but before updatingtestGetSign: 10,747 tests, onlytestGetSignfailed../gradlew check(JDK 21, with a JDK 17 toolchain): BUILD SUCCESSFUL. That covers 10,748 tests with 0 failures, plus checkstyle, PMD, spotless, spotbugs and jacoco coverage verification.+ - * / % ^ & | # << >>, comparisons,BETWEEN,IN,IS NULL,NOT,CASEand::intwere parsed with JSqlParser. Each tree was re-printed fully parenthesised, and PostgreSQL evaluated both forms. On master, a few percent of the expressions came out different. With this change, the only mismatches left involved^, which fix: bind ^ tighter than *, / and % #2831 fixed. With both changes applied, 0 of 8,623 expressions differ.Not run:
gradle jmh. The only extra work is agetToken(2)check when the next token is|.This is rebased on #2831, which touched the same lines. After the rebase,
ExpressionPrecedenceTestwas run again (without the grammar change,testGetSign,testBitwiseOperatorsBindLooserThanAdditionandtestSpaceSeparatedConcatStillBindsLikeAdditionfail; with it, all 23 pass) along with the full./gradlew test(10,750 tests, 0 failures).