[codex] Fix MySQL SET expression parser gaps#54
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends MySQL SQL parsing: SET statements now recognize LOCAL/PERSIST/PERSIST_ONLY scopes and dotted user variables, with subquery callback wiring. Expression parsing gains new tokens/operators (XOR, bitwise, shifts, DIV, MOD, REGEXP, SOUNDS LIKE, MEMBER OF, INTERVAL) and subquery-aware NOT/IN emission, plus corresponding tests. ChangesSET Statement Scope and Variable Handling
Estimated code review effort: 4 (Complex) | ~60 minutes Expression Parsing and Emission Extensions
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Parser as Parser::parse_set
participant SetParser
participant ExpressionParser
participant Emitter
Parser->>SetParser: construct and set_subquery_callback(parse_subquery_select)
SetParser->>SetParser: is_set_assignment_scope_(token)
SetParser->>SetParser: parse_variable_assignment(scope_token)
SetParser->>ExpressionParser: parse RHS expression
ExpressionParser->>ExpressionParser: parse_infix (SOUNDS/MEMBER/NOT REGEXP/quantified subquery)
ExpressionParser->>ExpressionParser: parse_quantified_subquery / parse_subquery_inner (capture span)
ExpressionParser-->>SetParser: expression AST node
SetParser-->>Parser: SET AST
Parser->>Emitter: emit_node(SET AST)
Emitter->>Emitter: emit_unary_op / emit_in_list (NOT IN, NOT BETWEEN, subquery)
Emitter-->>Parser: emitted SQL text
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
include/sql_parser/set_parser.h (1)
563-571: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider validating
actual_nametoken type in dotted@variable branch.The new dotted
@name.actualbranch consumesactual_namewithout checking its token type, consistent with the pre-existing@@branch (lines 579-583). However, the PostgreSQL schema-qualified branch (lines 604-613) does validaterhs.type == TK_IDENTIFIER. Adding the same guard here would catch malformed input like@user.= 1earlier and produce a clearer error instead of building a bogus identifier like@user.=.♻️ Optional: add token type validation
if (tok_.peek().type == TokenType::TK_DOT) { tok_.skip(); Token actual_name = tok_.next_token(); + if (actual_name.type != TokenType::TK_IDENTIFIER) { + tok_.flag_error(); + return nullptr; + } target->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, build_scoped_dotted_identifier_("@", name, actual_name)));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@include/sql_parser/set_parser.h` around lines 563 - 571, Validate the token consumed in the dotted `@name.actual` branch before building the scoped identifier. In set_parser.h, within the logic that handles tok_.peek().type == TokenType::TK_DOT, check that actual_name from tok_.next_token() is a valid identifier (same style as the PostgreSQL schema-qualified branch and consistent with build_scoped_dotted_identifier_ usage), and raise a parser error for malformed input instead of creating an invalid NODE_IDENTIFIER.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@include/sql_parser/expression_parser.h`:
- Around line 16-23: The operator precedence list in the expression parser
places BIT_XOR too low, causing XOR to bind after multiplication instead of
before it. Update the precedence ordering in the expression parser
enum/precedence table so BIT_XOR is moved above MULTIPLICATION, and verify the
parser path that uses this ordering now parses expressions like a * b ^ c as a *
(b ^ c).
---
Nitpick comments:
In `@include/sql_parser/set_parser.h`:
- Around line 563-571: Validate the token consumed in the dotted `@name.actual`
branch before building the scoped identifier. In set_parser.h, within the logic
that handles tok_.peek().type == TokenType::TK_DOT, check that actual_name from
tok_.next_token() is a valid identifier (same style as the PostgreSQL
schema-qualified branch and consistent with build_scoped_dotted_identifier_
usage), and raise a parser error for malformed input instead of creating an
invalid NODE_IDENTIFIER.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05da4a72-7632-4717-a456-537a95f24f00
📒 Files selected for processing (9)
include/sql_parser/emitter.hinclude/sql_parser/expression_parser.hinclude/sql_parser/keywords_mysql.hinclude/sql_parser/set_parser.hinclude/sql_parser/token.hinclude/sql_parser/tokenizer.hsrc/sql_parser/parser.cpptests/test_emitter.cpptests/test_set.cpp
| XOR, // XOR | ||
| AND, // AND | ||
| NOT, // NOT (prefix) | ||
| COMPARISON, // =, <, >, <=, >=, !=, <>, IS, LIKE, IN, BETWEEN | ||
| BIT_OR, // | | ||
| BIT_XOR, // ^ | ||
| BIT_AND, // & | ||
| SHIFT, // <<, >> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
MySQL operator precedence caret bitwise XOR vs multiplication
💡 Result:
In MySQL, the bitwise XOR operator (^) has a higher precedence than the multiplication operator (). According to the official MySQL operator precedence documentation, the caret (^) operator appears earlier in the precedence list—meaning it is evaluated first—than the multiplication () operator [1][2]. The relevant portion of the MySQL operator precedence hierarchy (from highest to lowest) is [1][2]: 1. ^ (Bitwise XOR) 2. *, /, DIV, %, MOD 3. -, + (Addition and Subtraction) Because the bitwise XOR operator has higher precedence, an expression such as 2 ^ 3 * 4 would be evaluated as (2 ^ 3) * 4. To ensure a different order of operations, you must use parentheses explicitly (e.g., 2 ^ (3 * 4)) [1][2].
Citations:
- 1: https://dev.mysql.com/doc/refman/8.4/en/operator-precedence.html
- 2: https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/operator-precedence.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File map =="
git ls-files include/sql_parser/expression_parser.h src/sql_parser | sed -n '1,120p'
echo
echo "== Outline of include/sql_parser/expression_parser.h =="
ast-grep outline include/sql_parser/expression_parser.h --view expanded || true
echo
echo "== Relevant lines in include/sql_parser/expression_parser.h =="
nl -ba include/sql_parser/expression_parser.h | sed -n '1,220p'
echo
echo "== Search for precedence mappings and CARET token handling =="
rg -n "CARET|BIT_XOR|MULTIPLICATION|SHIFT|precedence|operator precedence|infix_precedence|parse_atom|parse_sounds_like|parse_member_of|skip_to_matching_paren|is_keyword_as_identifier" include/sql_parser src/sql_parser
echo
echo "== Relevant parser implementation excerpts =="
for f in $(rg -l "BIT_XOR|infix_precedence|parse_atom|skip_to_matching_paren" include/sql_parser src/sql_parser); do
echo "--- $f ---"
nl -ba "$f" | sed -n '1,260p'
doneRepository: ProxySQL/ParserSQL
Length of output: 1959
BIT_XOR (^) needs to move above MULTIPLICATION. MySQL evaluates ^ before * / DIV % MOD, so this order makes a * b ^ c parse as (a * b) ^ c instead of a * (b ^ c).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/sql_parser/expression_parser.h` around lines 16 - 23, The operator
precedence list in the expression parser places BIT_XOR too low, causing XOR to
bind after multiplication instead of before it. Update the precedence ordering
in the expression parser enum/precedence table so BIT_XOR is moved above
MULTIPLICATION, and verify the parser path that uses this ordering now parses
expressions like a * b ^ c as a * (b ^ c).
1 participant
Summary
Fixes MySQL
SETparsing gaps found while porting ProxySQL PR #5088 TAP coverage to ParserSQL.SET LOCAL,SET PERSIST,SET PERSIST_ONLY, scoped comma items, and dotted single-at user variablesXOR, bit operators, shifts,DIV,MOD,REGEXP,NOT REGEXP,MEMBER OF,SOUNDS LIKE, andINTERVAL n unitliteralsIN (SELECT ...),NOT IN (SELECT ...), scalar subqueries, and quantifiedALL (SELECT ...)Closes #52.
Closes #53.
Root Cause
SetParseronly recognizedGLOBALandSESSIONas MySQL assignment scopes, comma-separated assignments did not re-check scoped forms, and single-at user variable targets stopped before dotted names. The expression parser also lacked tokenization and Pratt precedence entries for several MySQL operators, andSETRHS parsing was not wired to the existing subquery callback/emitter path.Validation
make testSummary by CodeRabbit
New Features
DIV/MOD,REGEXP,SOUNDS LIKE,MEMBER OF,INTERVAL, and quantified subqueries.SETstatement scopes, includingLOCAL,PERSIST, andPERSIST_ONLY, plus dotted user-variable assignments.Bug Fixes
NOT IN,NOT BETWEEN,NOT LIKE, andNOT REGEXP.SET-related subqueries round-trip more consistently.