fix(mysql): route routine DDL through text protocol#348
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| || head.starts_with("LOCK TABLES") | ||
| || head.starts_with("UNLOCK TABLES") | ||
| || head.starts_with("DROP PROCEDURE") | ||
| || head.starts_with("CREATE PROCEDURE") |
There was a problem hiding this comment.
WARNING: Missing CREATE OR REPLACE variants for PROCEDURE and FUNCTION
MySQL (and MariaDB) support CREATE OR REPLACE PROCEDURE and CREATE OR REPLACE FUNCTION (with or without a DEFINER clause). These statements are not permitted in the prepared-statement protocol, but the current starts_with checks only match CREATE PROCEDURE and CREATE FUNCTION — they will miss queries that include OR REPLACE between CREATE and the routine type.
Examples that will currently fail to route through text protocol:
CREATE OR REPLACE PROCEDURE my_proc() ...CREATE OR REPLACE FUNCTION my_func() ...CREATE OR REPLACE DEFINER=root PROCEDURE my_proc() ...CREATE OR REPLACE DEFINER=root FUNCTION my_func() ...
Consider normalising the prefix check, e.g. using a regex or token-based approach, to catch the OR REPLACE optional clause.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in commit 3ee7d39. The classifier now routes CREATE OR REPLACE PROCEDURE and CREATE OR REPLACE FUNCTION through text protocol, including compact and spaced DEFINER variants.
I also tightened the matching so CREATE [OR REPLACE] DEFINER ... VIEW does not route through text protocol just because the view body mentions PROCEDURE or FUNCTION.
Verification: cargo test text_protocol --lib passed via the Tabularis build-skill Rust test path, 5 passed, 0 failed.
| || head.starts_with("LOCK TABLES") | ||
| || head.starts_with("UNLOCK TABLES") | ||
| || head.starts_with("DROP PROCEDURE") | ||
| || head.starts_with("CREATE PROCEDURE") |
There was a problem hiding this comment.
WARNING: Missing CREATE OR REPLACE variants for PROCEDURE and FUNCTION
MySQL (and MariaDB) support CREATE OR REPLACE PROCEDURE and CREATE OR REPLACE FUNCTION (with or without a DEFINER clause). These statements are not permitted in the prepared-statement protocol, but the current starts_with checks only match CREATE PROCEDURE and CREATE FUNCTION — they will miss queries that include OR REPLACE between CREATE and the routine type.
Examples that will currently fail to route through text protocol:
CREATE OR REPLACE PROCEDURE my_proc() ...CREATE OR REPLACE FUNCTION my_func() ...CREATE OR REPLACE DEFINER=root PROCEDURE my_proc() ...CREATE OR REPLACE DEFINER=root FUNCTION my_func() ...
Consider normalising the prefix check, e.g. using a regex or token-based approach, to catch the OR REPLACE optional clause.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in commit 3ee7d39. This duplicate finding is covered by the same update: CREATE OR REPLACE routine DDL now uses the text-protocol path for PROCEDURE/FUNCTION statements, with tests for direct, compact DEFINER, and spaced DEFINER = 'root' @ 'localhost' forms.
Verification: cargo test text_protocol --lib passed via the Tabularis build-skill Rust test path, 5 passed, 0 failed.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
Previously Reported Issues
Files Reviewed (3 files)
Previous Review Summaries (2 snapshots, latest commit 3ee7d39)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3ee7d39)Status: No Issues Found | Recommendation: Merge Overview
Previously Reported Issues
Files Reviewed (3 files)
Previous review (commit 661ba81)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by laguna-xs.2-20260421:free · Input: 114K · Output: 1.2K · Cached: 3.8K |
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
|
Stiwar0098
left a comment
Stiwar0098
left a comment
There was a problem hiding this comment.
Addressed the Kilo review feedback.
This update routes CREATE OR REPLACE PROCEDURE and CREATE OR REPLACE FUNCTION through the MySQL text-protocol path, including DEFINER variants. I also tightened the classifier so CREATE [OR REPLACE] DEFINER ... VIEW statements do not match just because their body mentions PROCEDURE or FUNCTION.
Verification: cargo test text_protocol --lib passed via the Tabularis build-skill Rust test path, 5 passed, 0 failed.
…-ddl-text-protocol # Conflicts: # .gitignore
…-ddl-text-protocol # Conflicts: # src-tauri/src/drivers/mysql/tests.rs
|
@Stiwar0098 Looks good overall — the DEFINER/VIEW-body handling is the tricky part and it's done right. A few nits:
Let's get 1/2/3 in and then it's good to merge. |
2 participants
Closes #347
PR Type
Summary
.atl/and.opencode/tooling folders.Changes
.gitignoresrc-tauri/src/drivers/mysql/mod.rssrc-tauri/src/drivers/mysql/tests.rsTest Plan
cargo test text_protocol --libNotes
MySQL documentation lists the SQL statements permitted in prepared statements and routine DDL is not included. The reported
DROP PROCEDURE IF EXISTS sociedades_close;now routes throughsqlx::raw_sql()via the existing MySQL text-protocol path.