Skip to content

fix(mysql/splitter): distinguish IF control flow from IF() with parenthesized conditions#3336

Merged
Seechi-Yolo merged 3 commits into
mainfrom
sqle/fix-if-control-flow-splitter
Jun 18, 2026
Merged

fix(mysql/splitter): distinguish IF control flow from IF() with parenthesized conditions#3336
Seechi-Yolo merged 3 commits into
mainfrom
sqle/fix-if-control-flow-splitter

Conversation

@LordofAvernus

@LordofAvernus LordofAvernus commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Fixes #3337

Summary

  • Fix MySQL SQL splitter misclassifying IF (cond) AND ... THEN as IF() function calls.
  • Detect control-flow IF blocks by scanning for THEN at parenthesis depth zero.
  • Add regression tests for parenthesized conditions, ELSEIF, IF() in triggers, and nested CASE ... THEN in function arguments.
  • Restore EnableWindowFunc(true) that was accidentally dropped during initial cherry-pick from sqle-ee.

Problem

When a stored procedure uses a parenthesized condition such as IF ( v_lng IS NULL OR v_lat IS NULL ) AND Arg0 IS NOT NULL THEN, the splitter skipped the IfEndIfBlock stack entry. The outer BEGIN...END block ended early at END IF;, the trailing END; was split into a separate statement, and online execution failed with MySQL 1064.

Test plan

  • go test ./sqle/driver/mysql/splitter/...
  • Verified on DMS/SQLE environment 10.186.63.138:21004: split count 3 → 2, workflow execution succeeds after package replacement

…tion uses parentheses

The splitter treated any IF followed by '(' as the IF() function, so stored
procedures using IF (cond) AND ... THEN were split incorrectly and online
execution failed with MySQL 1064. Use THEN at parenthesis depth zero to detect
control-flow IF blocks instead.
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 5c7e247)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to 39edb9b
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
恢复扫描器初始位置

建议在函数内部记录当前扫描器位置,并在返回前调用 SetCursor
恢复初始位置,以避免该函数改变扫描器的状态,从而影响后续解析逻辑。这样可以更加保证函数的副作用最小化和代码的健壮性。

sqle/driver/mysql/splitter/splitter.go [234-252]

 func (s *splitter) ifIsControlFlowStatement() bool {
+	current := s.scanner.Offset()
 	depth := 0
 	for s.scanner.Offset() < len(s.scanner.Text()) {
 		token := s.scanner.NextToken()
 		switch token.Ident() {
 		case "(":
 			depth++
 		case ")":
 			if depth > 0 {
 				depth--
 			}
 		default:
 			if isThenToken(token) && depth == 0 {
+				s.scanner.SetCursor(current)
 				return true
 			}
 		}
 	}
+	s.scanner.SetCursor(current)
 	return false
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly addresses potential side effects by storing and restoring the scanner position in the ifIsControlFlowStatement function. It improves code robustness in a moderate way, making it a sensible change without being overly critical.

Medium

Previous suggestions

Suggestions up to commit 3326f05
CategorySuggestion                                                                                                                                    Impact
Possible issue
增加 nil 检查

建议在该函数开始处增加对传入的 token 是否为 nil 的检查,以防止出现 nil pointer dereference 导致程序 panic 的问题。检查结果为
nil 时返回 false。

sqle/driver/mysql/splitter/block.go [14-16]

 func isThenToken(token *parser.Token) bool {
+	if token == nil {
+		return false
+	}
 	return strings.ToUpper(token.Ident()) == "THEN"
 }
Suggestion importance[1-10]: 8

__

Why: This suggestion adds a nil check for token in the isThenToken function, which is an important safety improvement to avoid potential nil pointer dereference panics. The improved code correctly reflects the intended modification.

Medium
检查 token 是否为 nil

建议在循环中调用 NextToken() 后立即对返回的 token 进行 nil 检查,以避免在 token 为 nil 时调用 token.Ident() 产生
panic。此修改可以提高代码的稳健性,避免意外错误。

sqle/driver/mysql/splitter/splitter.go [232-250]

 func (s *splitter) ifIsControlFlowStatement() bool {
 	depth := 0
 	for s.scanner.Offset() < len(s.scanner.Text()) {
 		token := s.scanner.NextToken()
+		if token == nil {
+			continue
+		}
 		switch token.Ident() {
 		case "(":
 			depth++
 		case ")":
 			if depth > 0 {
 				depth--
 			}
 		default:
 			if isThenToken(token) && depth == 0 {
 				return true
 			}
 		}
 	}
 	return false
 }
Suggestion importance[1-10]: 8

__

Why: By inserting a nil check right after obtaining the token in ifIsControlFlowStatement, this suggestion addresses a potential panic scenario. The improved code accurately implements the check and is well aligned with the PR diff context.

Medium

…in IF fix

Keep the CE window-function parser setup when applying the IF control-flow
splitter fix; the removal came from copying sqle-ee files that lack this CE change.
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 39edb9b

The scanner already sits at currentOffset (right after IF) when entering the
branch, so the first SetCursor was a no-op. Keep only the reset after the
lookahead scan.
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 5c7e247

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@Seechi-Yolo Seechi-Yolo merged commit 8ca4a56 into main Jun 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MySQL/splitter] 存储过程 IF (cond) THEN 被误判为 IF() 导致 SQL 拆分错误、上线 1064

2 participants