diff --git a/packages/db/src/cores/view-definition.test.ts b/packages/db/src/cores/view-definition.test.ts new file mode 100644 index 000000000..167213174 --- /dev/null +++ b/packages/db/src/cores/view-definition.test.ts @@ -0,0 +1,61 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, expect, it } from 'vitest'; +import { stripCreateViewHeader } from './view-definition.js'; + +const BODY = 'SELECT id, name FROM customers'; + +describe('stripCreateViewHeader', () => { + it('strips the plain header SQLite stores in sqlite_master.sql', () => { + expect(stripCreateViewHeader(`CREATE VIEW v_customers AS ${BODY};`)).toBe(BODY); + }); + + it('keeps a definition that is already just the query', () => { + expect(stripCreateViewHeader(BODY)).toBe(BODY); + }); + + it.each([ + ['OR REPLACE', `CREATE OR REPLACE VIEW v AS ${BODY}`], + ['TEMP', `CREATE TEMP VIEW v AS ${BODY}`], + ['TEMPORARY', `CREATE TEMPORARY VIEW v AS ${BODY}`], + ['IF NOT EXISTS', `CREATE VIEW IF NOT EXISTS v AS ${BODY}`], + ['schema qualifier', `CREATE VIEW main.v AS ${BODY}`], + ['column list', `CREATE VIEW v (id, name) AS ${BODY}`], + ['lowercase', `create view v as ${BODY}`], + ['extra whitespace', `CREATE VIEW\n v\n AS\n ${BODY}`], + ])('strips a header with %s', (_label, sql) => { + expect(stripCreateViewHeader(sql)).toBe(BODY); + }); + + it.each([ + ['double quotes', `CREATE VIEW "my view" AS ${BODY}`], + ['brackets', `CREATE VIEW [my view] AS ${BODY}`], + ['backticks', `CREATE VIEW \`my view\` AS ${BODY}`], + ['quoted schema and name', `CREATE VIEW "main"."my view" AS ${BODY}`], + ])('strips a header whose name uses %s', (_label, sql) => { + expect(stripCreateViewHeader(sql)).toBe(BODY); + }); + + it('consumes comment lines written above the CREATE', () => { + expect(stripCreateViewHeader(`-- one\n-- two\nCREATE VIEW v AS ${BODY}`)).toBe(BODY); + }); + + it('leaves an inner AS in the query alone', () => { + const body = 'SELECT c.id AS customer_id FROM customers c'; + expect(stripCreateViewHeader(`CREATE VIEW v AS ${body}`)).toBe(body); + }); + + it('does not mistake a CREATE TABLE for a view header', () => { + const sql = 'CREATE TABLE t (id INTEGER)'; + expect(stripCreateViewHeader(sql)).toBe(sql); + }); + + it('answers empty for a missing definition', () => { + expect(stripCreateViewHeader(null)).toBe(''); + expect(stripCreateViewHeader(undefined)).toBe(''); + expect(stripCreateViewHeader('')).toBe(''); + }); +}); diff --git a/packages/db/src/cores/view-definition.ts b/packages/db/src/cores/view-definition.ts new file mode 100644 index 000000000..78ca2a4eb --- /dev/null +++ b/packages/db/src/cores/view-definition.ts @@ -0,0 +1,53 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Some catalogs hand back a view's whole `CREATE VIEW … AS …` text rather than + * just its query: SQLite's `sqlite_master.sql`, DuckDB's `duckdb_views().sql` + * and SQL Server's `sys.sql_modules.definition` all do. Every provider stores + * only the query in `definition`, because the generator re-emits the header + * itself with the target's schema prefix — keeping the original produced + * `CREATE VIEW v AS CREATE VIEW v AS SELECT …`, which no engine accepts. + * + * The header is matched conservatively: a definition that is already just a + * query comes back untouched, so this is safe to apply to any provider's value. + */ + +/** One comment line at the start, e.g. a `-- note` written above the CREATE. */ +const LEADING_COMMENT_RE = /^[ \t]*--[^\n]*\n/; + +/** + * `CREATE [OR REPLACE] [TEMP|TEMPORARY] VIEW [IF NOT EXISTS] [schema.]name + * [(cols)] AS`, where each identifier may be bare, `"quoted"`, `[bracketed]` + * or `` `backticked` ``. + * + * Written as one literal, like the equivalents in the SQL Server and DuckDB + * providers, so the pattern stays visible to the linter rather than hidden + * behind a built string. + * + * The suppression below is a measured false positive, not a shrug: every + * quantifier is over a bounded negated class or `\w`, and the only optional + * group (the schema qualifier) requires a literal `.`, so there is nothing to + * backtrack between. Checked on 50 KB pathological inputs — a 50,000-char + * identifier, whitespace run, `a.`/`a ` repetition, unclosed paren and + * unterminated quote, none of which reach `AS` — all matched in under 1 ms. + */ +// The regex stays on this one line so the directive lands on the pattern +// itself, not on a bare `const … =` continuation line (where it reads as unused). +// eslint-disable-next-line security/detect-unsafe-regex -- see above: bounded classes only; measured linear on 50 KB pathological inputs +const VIEW_CREATE_RE = /^[ \t\n]*CREATE[ \t\n]+(?:OR[ \t\n]+REPLACE[ \t\n]+)?(?:TEMP(?:ORARY)?[ \t\n]+)?VIEW[ \t\n]+(?:IF[ \t\n]+NOT[ \t\n]+EXISTS[ \t\n]+)?(?:(?:"[^"]*"|\[[^\]]*\]|`[^`]*`|\w+)[ \t\n]*\.[ \t\n]*)?(?:"[^"]*"|\[[^\]]*\]|`[^`]*`|\w+)[ \t\n]*(?:\([^)]*\)[ \t\n]*)?AS[ \t\n]+/i; + +/** A view's query, with any `CREATE VIEW … AS` header and trailing `;` removed. */ +export function stripCreateViewHeader(sql: string | null | undefined): string { + let text = sql ?? ''; + // Stripped one line at a time rather than with a `(?:…)+` prefix: a star + // inside a plus is what makes this family of pattern a backtracking risk. + while (LEADING_COMMENT_RE.test(text)) text = text.replace(LEADING_COMMENT_RE, ''); + return text + .replace(VIEW_CREATE_RE, '') + .replace(/;\s*$/, '') + .trim(); +} diff --git a/packages/db/src/providers/sqlLite/sqlLite.provider.ts b/packages/db/src/providers/sqlLite/sqlLite.provider.ts index a74f1caaf..b91753aac 100644 --- a/packages/db/src/providers/sqlLite/sqlLite.provider.ts +++ b/packages/db/src/providers/sqlLite/sqlLite.provider.ts @@ -1,4 +1,5 @@ import { ConnectionFactory } from '../../cores/connection-factory.js'; +import { stripCreateViewHeader } from '../../cores/view-definition.js'; import { dbSchemaToTableSchemas, groupForeignKeyRows, errorMessage } from '@foxschema/sql'; import { type SchemaProvider, @@ -175,7 +176,9 @@ export class SqliteProvider implements SchemaProvider { (views[vw.name] ??= []).push({ name: vw.name, schema: '', - definition: vw.sql ?? '', + // sqlite_master.sql is the FULL "CREATE VIEW name AS " text, but + // `definition` holds only the query — see stripCreateViewHeader. + definition: stripCreateViewHeader(vw.sql), columns: vw.viewColumns, indexes: [], }); diff --git a/packages/server/src/internal/driver-install.ts b/packages/server/src/internal/driver-install.ts index 71871cac3..1e94d680c 100644 --- a/packages/server/src/internal/driver-install.ts +++ b/packages/server/src/internal/driver-install.ts @@ -332,7 +332,9 @@ export function clearNpmRenameStaging(nodeModulesDir: string): void { for (const name of names) { if (!isNpmRenameStagingName(name)) continue; try { - // eslint-disable-next-line security/detect-non-literal-fs-filename -- names are filtered to npm's rename-staging pattern + // names are filtered to npm's rename-staging pattern. No eslint-disable here: + // security/detect-non-literal-fs-filename does not cover rmSync, so a + // directive for it would itself be an "unused disable" error. rmSync(join(nodeModulesDir, name), { recursive: true, force: true }); } catch { // still held open; the retry may still succeed diff --git a/packages/sql/src/modules/migrations/sql-generator.module.test.ts b/packages/sql/src/modules/migrations/sql-generator.module.test.ts index 610900403..056801f94 100644 --- a/packages/sql/src/modules/migrations/sql-generator.module.test.ts +++ b/packages/sql/src/modules/migrations/sql-generator.module.test.ts @@ -1573,3 +1573,56 @@ describe('ClickHouse CREATE TABLE uses ENGINE / ORDER BY, not a PRIMARY KEY cons expect(sql).toMatch(/DROP TABLE IF EXISTS demo_b\.legacy_audit_log;/); }); }); + +describe('SQLite views: no CREATE OR REPLACE, no doubled header', () => { + // `definition` holds only the query — the SQLite provider strips the + // `CREATE VIEW … AS` header that sqlite_master.sql carries + // (cores/view-definition.ts). The generator supplies the header itself. + const body = 'SELECT id, name FROM customers'; + + const addedView: TableDiff = { + tableName: 'V_CUSTOMERS', + objectType: 'VIEW', + status: 'ADDED', + definition: body, + columnDiffs: [], + indexDiffs: [], + foreignKeyDiffs: [], + }; + + /** Only the executable lines — the script's `-- CREATE VIEW:` headers are comments. */ + const executable = (sql: string) => + sql + .split('\n') + .filter((line) => line.trim() && !line.trim().startsWith('--')) + .join('\n'); + + it('creates an added view with valid SQLite syntax', () => { + const sql = executable(gen.generateMigrationSql([addedView], 'sqlite')); + expect(sql).toMatch(/CREATE VIEW IF NOT EXISTS V_CUSTOMERS AS/i); + // SQLite has no CREATE OR REPLACE VIEW — it is `near "OR": syntax error`. + expect(sql).not.toMatch(/OR\s+REPLACE/i); + // and the header must appear once, not wrapped around another CREATE VIEW + expect(sql).not.toMatch(/AS\s+CREATE\s+VIEW/i); + expect(sql.match(/CREATE VIEW/gi)?.length).toBe(1); + }); + + it('replaces a modified view with DROP + CREATE', () => { + const modifiedView: TableDiff = { + ...addedView, + status: 'MODIFIED', + sourceTable: { objectType: 'VIEW', name: 'v_customers', definition: body, columns: [], indices: [], foreignKeys: [] }, + targetTable: { objectType: 'VIEW', name: 'v_customers', definition: 'SELECT id FROM customers', columns: [], indices: [], foreignKeys: [] }, + }; + const sql = executable(gen.generateMigrationSql([modifiedView], 'sqlite')); + expect(sql).toMatch(/DROP VIEW IF EXISTS v_customers;/i); + expect(sql).toMatch(/CREATE VIEW v_customers AS/i); + expect(sql).not.toMatch(/OR\s+REPLACE/i); + expect(sql).not.toMatch(/ALTER\s+VIEW/i); + }); + + it('still uses CREATE OR REPLACE VIEW on Postgres', () => { + const sql = gen.generateMigrationSql([addedView], 'postgres'); + expect(sql).toMatch(/CREATE OR REPLACE VIEW v_customers AS/i); + }); +}); diff --git a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts index 4f75138e1..b296e3f1a 100644 --- a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts +++ b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts @@ -39,3 +39,20 @@ describe('sqlite CREATE INDEX', () => { ).toBe('CREATE INDEX IF NOT EXISTS main.idx_active ON t (id) WHERE active = 1;'); }); }); + +describe('sqlite views', () => { + const BODY = 'SELECT id FROM customers'; + + it('creates a view without CREATE OR REPLACE, which SQLite rejects', () => { + const sql = sqliteSqlDialect.createViewStatement!('v_customers', BODY); + expect(sql).toBe(`CREATE VIEW IF NOT EXISTS v_customers AS\n${BODY}`); + expect(sql).not.toMatch(/OR\s+REPLACE/i); + }); + + it('replaces a changed view by dropping it first — SQLite has no ALTER VIEW', () => { + const sql = sqliteSqlDialect.alterViewStatement!('v_customers', BODY); + expect(sql).toBe(`DROP VIEW IF EXISTS v_customers;\nCREATE VIEW v_customers AS\n${BODY}`); + expect(sql).not.toMatch(/ALTER\s+VIEW/i); + expect(sql).not.toMatch(/OR\s+REPLACE/i); + }); +}); diff --git a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts index fa9a06daa..e43936e99 100644 --- a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts +++ b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts @@ -119,5 +119,22 @@ export const sqliteSqlDialect: SqlDialect = { return `DROP TRIGGER IF EXISTS ${triggerName};`; }, + /** + * SQLite has no `CREATE OR REPLACE VIEW` — the generator's default emitted + * `near "OR": syntax error`. `IF NOT EXISTS` keeps a re-run of the script + * harmless, matching how this dialect emits indexes. + */ + createViewStatement(name: string, body: string): string { + return `CREATE VIEW IF NOT EXISTS ${name} AS\n${body}`; + }, + + /** + * SQLite has no `ALTER VIEW` either, so a changed view is dropped and + * recreated. Views hold no data, so this loses nothing. + */ + alterViewStatement(name: string, body: string): string { + return `DROP VIEW IF EXISTS ${name};\nCREATE VIEW ${name} AS\n${body}`; + }, + ...types, };