fix(validations): avoid ENGINE != InnoDB false positives on row data - #2873
fix(validations): avoid ENGINE != InnoDB false positives on row data#2873WRasada wants to merge 1 commit into
Conversation
The matcher fired on 'engine =' strings anywhere in a line, including inside INSERT row content. Skip data lines (INSERT/REPLACE statements and VALUES rows); ENGINE clauses only matter in DDL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a false-positive in the SQL validation rule that reports ENGINE != InnoDB by ensuring the matcher doesn’t trigger on row-data lines (e.g., INSERT/REPLACE statements and ,(...) continuation rows) where user content can contain strings like "engine =". This improves validation reliability on large real-world dumps without weakening the intended DDL-focused enforcement.
Changes:
- Updated the
ENGINE != InnoDBregex to ignore row-data lines while still flagging non-InnoDB engines in DDL. - Extended the bad SQL fixture with row-data lines containing engine-like strings to reproduce the prior false positive.
- Added a regression assertion ensuring only the intended offending line is reported.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/lib/validations/sql.ts |
Anchors and refines the ENGINE != InnoDB matcher to skip row-data lines, preventing content-based false positives. |
__tests__/lib/validations/sql.js |
Adds a regression test asserting row-data “engine =” strings do not add extra flagged lines. |
__fixtures__/validations/bad-sql-dump.sql |
Appends INSERT/VALUES-row examples containing engine-like strings to validate the new matcher behavior. |
|
This pull request has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed. This is an automation to keep pull requests manageable and actionable and is not a comment on the quality of this pull request nor on the work done so far. Closed PRs are still valuable to the project and their branches are preserved. |
Problem
SQL validation rejects dumps whose table definitions are 100% InnoDB:
The flagged lines are INSERT row data — user content that happens to contain strings like
engine =(form-submission spam, post content such as "Bigger engine = lower price", etc.). Any sufficiently large real-world dump trips this and is forced to--skip-validate, which also disables every other check.Root cause
The matcher (trunk permalink) is unanchored and case-insensitive:
It fires on
engine =anywhere in any line, including inside multi-megabyte INSERT statements.Fix
Skip lines holding row data —
INSERT/REPLACEstatements and(...)/,(...)VALUES rows (both mysqldump extended-insert and mydumper row formats).ENGINEclauses only matter in DDL:Genuine violations still flag:
) ENGINE=MyISAMtable definitions,ALTER TABLE ... ENGINE=MyISAMconversions, and the existing fixture's commented-out definition (line 14 expectation unchanged).Testing
__fixtures__/validations/bad-sql-dump.sql(an INSERT and a mydumper-style,(...)row containing engine-like strings) + new assertion that they are not flaggedENGINE=InnoDBdefinitions still scannedChangelog Description
Fixed
ENGINE != InnoDBSQL validation error triggered by row content containing "engine =" stringsRelated
USEstatements)-ofix), fix(dev-env): recompute mydumper section sizes after search-replace #2872 (search-replace size fix), perf(dev-env): tune local database performance defaults #2874 (DB tuning)🤖 Generated with Claude Code