[improvement] frontend: add sql to fatal log

[volgariver6]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #21602

What this PR does / why we need it:

add sql to fatal log

---

PR Type

Enhancement, Tests

---

Description

• Enhanced StartStatement to include SQL string parameter.

• Updated all relevant function calls to pass SQL string.

• Improved error logging with SQL context in StartStatement.

• Adjusted and added test cases to validate new functionality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	4 files mysql_cmd_executor.go Pass SQL string to `StartStatement` in executor                    +1/-1      sql_executor.go Updated SQL executor to pass SQL string to `StartStatement` +2/-2      types.go Modified Workspace interface to include SQL string in StartStatement +1/-1      types.go Enhanced `StartStatement` with SQL string and improved logging +5/-2
Tests	6 files txn_mock.go Mock `StartStatement` updated to accept SQL string              +4/-4      txn_test.go Adjusted tests to include SQL string in `StartStatement`  +13/-13  util_test.go Updated mock expectations for `StartStatement` with SQL string +2/-2      disttae_engine_test.go Updated tests to include SQL string in `StartStatement`    +2/-2      util.go Adjusted utility function to pass SQL string to `StartStatement` +1/-1      workspace_test.go Updated workspace tests to include SQL string in `StartStatement` +6/-6

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The StartStatement function now includes SQL in the fatal error message, but it might be better to handle this error more gracefully rather than calling logutil.Fatal which terminates the process. if txn.startStatementCalled { logutil.Fatal("BUG: StartStatement called twice", zap.String("txn", hex.EncodeToString(txn.op.Txn().ID)), zap.String("SQL", sql), ) Empty SQL Parameter Many test cases are passing empty strings as SQL parameters. Consider using more realistic SQL statements in tests to better validate the new functionality. wsp.StartStatement("")

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Avoid fatal program termination Suggestion Impact: The commit partially implemented the suggestion by making the error handling more conditional. It now uses logutil.Error instead of logutil.Fatal, but only in test environments (when flag.Lookup('test.v') is not nil). The function signature was also changed to accept a tree.Statement instead of a string. code diff: if txn.startStatementCalled { - logutil.Fatal("BUG: StartStatement called twice", + var sql string + if stmt != nil { + fmtCtx := tree.NewFmtCtx(dialect.MYSQL, tree.WithQuoteString(true)) + stmt.Format(fmtCtx) + sql = fmtCtx.String() + } + log := logutil.Fatal + if flag.Lookup("test.v") != nil { + log = logutil.Error + } + log("BUG: StartStatement called twice", zap.String("txn", hex.EncodeToString(txn.op.Txn().ID)), zap.String("SQL", sql), ) The current implementation uses logutil.Fatal which will terminate the program when StartStatement is called twice. Consider using a less drastic approach like logging an error and returning, especially since this is now capturing the SQL statement for debugging purposes. pkg/vm/engine/disttae/types.go [528-537] func (txn *Transaction) StartStatement(sql string) { if txn.startStatementCalled { - logutil.Fatal("BUG: StartStatement called twice", + logutil.Error("BUG: StartStatement called twice", zap.String("txn", hex.EncodeToString(txn.op.Txn().ID)), zap.String("SQL", sql), ) + return } txn.startStatementCalled = true txn.incrStatementCalled = false } [Suggestion has been applied] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential reliability issue. Using logutil.Fatal causes program termination when StartStatement is called twice, which is unnecessarily drastic. Logging the error and returning would allow the system to continue operating, making it more resilient while still capturing the debugging information.	Medium
Update

[mergify]

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.

If you want to requeue this pull request, you can post a @mergifyio requeue comment.