Used single quotes for SQL string literals for better compatibility #366
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinansfield
force-pushed
the
claude/fix-knex-migrator-blocker-01QyHSqNTPPXHVNUgB1Fq4Gu
branch
from
68db4c7 to
b6f5aa4
Compare
kevinansfield
changed the title
- Add better-sqlite3 v12.5.0 as a dev dependency - Create test config for better-sqlite3 environment - Add better-sqlite3 to the CI testing matrix alongside sqlite3 and mysql8
better-sqlite3 requires Node 20.x or later, so exclude it from the Node 18.12.1 test matrix.
This allows yarn install to succeed on Node 18 where better-sqlite3 is not supported, similar to how sqlite3 is handled.
kevinansfield
changed the base branch from
main
to
claude/add-better-sqlite3-ci-01QYHBgA4t2aaECXK3Nb6Smu
kevinansfield
changed the base branch from
claude/add-better-sqlite3-ci-01QYHBgA4t2aaECXK3Nb6Smu
to
main
…ility The `better-sqlite3` package enforces stricter SQL standards than the original `sqlite3` package. In SQL, double quotes are for identifiers while single quotes are for string literals. This change fixes the sqlite_master query in lib/database.js and updates all test migration files to use proper SQL string literal syntax. Uses template literals to satisfy ESLint single-quote rule while allowing single quotes in SQL. This unblocks the Ghost migration from sqlite3 to better-sqlite3 (see TryGhost/Ghost#25556).
kevinansfield
force-pushed
the
claude/fix-knex-migrator-blocker-01QyHSqNTPPXHVNUgB1Fq4Gu
branch
from
b6f5aa4 to
3ee0c93
Compare
Changes: 1. Use template literals in generateMigrationScript to allow single quotes in SQL strings without escaping issues 2. Fix SQL string literals in test files to use single quotes 3. Add better-sqlite3 error code check (SQLITE_ERROR) alongside sqlite3 errno checks in lib/index.js for isDatabaseOK 4. Update implicit_commits_spec.js to handle better-sqlite3 error format All 95 tests now pass with both sqlite3 and better-sqlite3.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes SQL syntax issues that prevent knex-migrator from working with
better-sqlite3, which enforces stricter SQL standards than the originalsqlite3package.The Issue:
") are for identifiers (column/table names) - Single quotes (') are for string literalsSELECT name FROM sqlite_master WHERE type="table"incorrectly uses double quotes for the string literal"table"better-sqlite3enforces this distinction and rejects the queryThe Fix:
type="table"totype='table'inlib/database.js:183Related: