Fix regex for some sqlite databases, there the table name was quoted#157
Fix regex for some sqlite databases, there the table name was quoted#157jogibear9988 merged 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request attempts to fix a regex pattern used to parse foreign key constraints from SQLite CREATE TABLE scripts. The fix addresses an issue where table names were quoted with double quotes, which the original regex pattern could not match.
Changes:
- Modified regex pattern in
GetForeignKeyConstraintsmethod to include double quotes in the table name matching pattern after the REFERENCES keyword
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var createTableScript = GetSqlCreateTableScript(tableName); | ||
| // GeneratedRegex | ||
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+\w+\s*\([^)]+\)"); | ||
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+[\w""]+\s*\([^)]+\)"); |
There was a problem hiding this comment.
The regex pattern [\w""]+ is insufficient for matching quoted SQLite table names. This pattern would incorrectly match malformed names like Tab"le"Name or """"", and would fail to match properly quoted table names containing special characters like "table-name" or "table name" (with spaces or hyphens).
Since SQLite uses double quotes for quoting identifiers (as seen in SQLiteDialect.cs:57 QuoteTemplate => "\"{0}\"") and the code generates foreign keys with quoted table names via QuoteTableNameIfRequired (line 1453), the regex should match either an unquoted identifier OR a properly quoted identifier.
The pattern should be: (?:\w+|"[^"]*") which matches either:
\w+for unquoted table names"[^"]*"for quoted table names (double quotes with any content except quotes inside)
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+[\w""]+\s*\([^)]+\)"); | |
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+(?:\w+|""[^""]*"")\s*\([^)]+\)"); |
| var createTableScript = GetSqlCreateTableScript(tableName); | ||
| // GeneratedRegex | ||
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+\w+\s*\([^)]+\)"); | ||
| var regEx = new Regex(@"CONSTRAINT\s+\w+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+[\w""]+\s*\([^)]+\)"); |
There was a problem hiding this comment.
The existing tests for GetForeignKeyConstraints do not cover scenarios where table names are quoted (e.g., when using SQLite reserved words like "order", "group", "key", or table names with special characters). Consider adding test cases that verify the regex correctly matches foreign key constraints with quoted parent table names, such as a foreign key referencing a table named "group" or "my-table".
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
please implement a test
|
@jogibear9988 I've opened a new pull request, #158, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.