From 85992ef328dfa4f24bc16332d5d0ac1e8d9f0db6 Mon Sep 17 00:00:00 2001 From: Wes Rasada Date: Thu, 4 Jun 2026 15:59:40 -0700 Subject: [PATCH] fix(validations): avoid ENGINE != InnoDB false positives on row data 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) --- __fixtures__/validations/bad-sql-dump.sql | 3 +++ __tests__/lib/validations/sql.js | 6 ++++++ src/lib/validations/sql.ts | 5 ++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/__fixtures__/validations/bad-sql-dump.sql b/__fixtures__/validations/bad-sql-dump.sql index 49fc90988..cde2ce474 100644 --- a/__fixtures__/validations/bad-sql-dump.sql +++ b/__fixtures__/validations/bad-sql-dump.sql @@ -39,3 +39,6 @@ ALTER TABLE wp_options ADD KEY `autoload` (`autoload`); SET UNIQUE_CHECKS = 0; + +INSERT INTO wp_posts (ID, post_content) VALUES (1, 'Try our search engine = the best! ENGINE=MyISAM'); +,(2, 'mydumper continuation row with ENGINE = SPAM content') diff --git a/__tests__/lib/validations/sql.js b/__tests__/lib/validations/sql.js index 59197ba32..6b88eadda 100644 --- a/__tests__/lib/validations/sql.js +++ b/__tests__/lib/validations/sql.js @@ -123,6 +123,12 @@ describe( 'lib/validations/sql', () => { it( 'instances of ENGINE != InnoDB', () => { expect( output ).toContain( 'ENGINE != InnoDB on line(s) 14' ); } ); + it( 'no ENGINE != InnoDB false positives on row data containing "engine =" strings', () => { + // Lines 43-44 are an INSERT statement and a mydumper-style `,(...)` VALUES row + // whose *content* mentions non-InnoDB engines; they must not be flagged. The + // trailing period asserts line 14 is the *only* flagged line. + expect( output ).toContain( 'ENGINE != InnoDB on line(s) 14.' ); + } ); it( 'use statement should be ok', () => { expect( output ).not.toContain( "'USE ' should not be present (case-insensitive)" diff --git a/src/lib/validations/sql.ts b/src/lib/validations/sql.ts index 6f86fcc04..0a8fb12d2 100644 --- a/src/lib/validations/sql.ts +++ b/src/lib/validations/sql.ts @@ -368,7 +368,10 @@ const checks: Checks = { recommendation: "Use search-replace to change environment's domain", }, engineInnoDB: { - matcher: /\sENGINE\s?=(?!(\s?InnoDB))/i, + // Skip lines holding row data (`INSERT ...`/`REPLACE ...` statements and `(...)` / + // `,(...)` VALUES rows): user content routinely contains "engine =" strings, which + // produced false positives on large dumps. ENGINE clauses only matter in DDL. + matcher: /^(?!\s*(?:INSERT|REPLACE)\b|\s*[(,]).*\sENGINE\s?=(?!(\s?InnoDB))/i, matchHandler: lineNumber => ( { lineNumber } ), outputFormatter: lineNumberCheckFormatter, results: [],