chore: Remove MariaDB and MySQL from codebase (no-changelog) #23892
Conversation
RicardoE105
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 74 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/@n8n/db/src/entities/abstract-entity.ts">
<violation number="1" location="packages/@n8n/db/src/entities/abstract-entity.ts:22">
P2: Rule violated: **Prefer Typeguards over Type casting**
Using `as keyof typeof` for type narrowing violates the typeguard preference rule. Consider using a type guard function or adding runtime validation:
```typescript
function isValidDbType<T extends Record<string, unknown>>(map: T, type: string): type is keyof T & string {
return type in map;
}
if (!isValidDbType(timestampSyntaxMap, dbType)) {
throw new Error(`Unsupported database type: ${dbType}`);
}
const timestampSyntax = timestampSyntaxMap[dbType];
Alternatively, use an assertion with runtime check before the cast.
P2: Rule violated: **Prefer Typeguards over Type casting**Using as keyof typeof for type narrowing violates the typeguard preference rule. Consider using a type guard function or adding runtime validation before accessing the map, similar to the timestampSyntaxMap case above.
The PR removes MySQL and MariaDB support from the code, but the test file sql-utils.test.ts still contains tests for these database types that will now fail. Tests for addColumnQuery with mysql/mariadb expect backtick quoting and DOUBLE type, and tests for normalizeValueForDatabase expect the datetime format for mysql/mariadb. These tests need to be removed or updated to match the code changes.
</details>
<sub>Reply with feedback, questions, or to request a fix. Tag `@cubic-dev-ai` to re-run a review.</sub>
| @@ -51,9 +51,6 @@ function dataTableColumnTypeToSql( | |||
| switch (dbType) { | |||
There was a problem hiding this comment.
P1: Rule violated: Tests
The PR removes MySQL and MariaDB support from the code, but the test file sql-utils.test.ts still contains tests for these database types that will now fail. Tests for addColumnQuery with mysql/mariadb expect backtick quoting and DOUBLE type, and tests for normalizeValueForDatabase expect the datetime format for mysql/mariadb. These tests need to be removed or updated to match the code changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/data-table/utils/sql-utils.ts, line 123:
<comment>The PR removes MySQL and MariaDB support from the code, but the test file `sql-utils.test.ts` still contains tests for these database types that will now fail. Tests for `addColumnQuery` with mysql/mariadb expect backtick quoting and `DOUBLE` type, and tests for `normalizeValueForDatabase` expect the datetime format for mysql/mariadb. These tests need to be removed or updated to match the code changes.</comment>
<file context>
@@ -123,20 +120,11 @@ export function renameColumnQuery(
- default:
- return `"${name}"`;
- }
+export function quoteIdentifier(name: string, _dbType: DataSourceOptions['type']): string {
+ // PostgreSQL and SQLite both use double quotes
+ return `"${name}"`;
</file context>
| mariadb: 'CURRENT_TIMESTAMP(3)', | ||
| }[dbType]; | ||
| } as const; | ||
| const timestampSyntax = timestampSyntaxMap[dbType as keyof typeof timestampSyntaxMap]; |
There was a problem hiding this comment.
P2: Rule violated: Prefer Typeguards over Type casting
Using as keyof typeof for type narrowing violates the typeguard preference rule. Consider using a type guard function or adding runtime validation:
function isValidDbType<T extends Record<string, unknown>>(map: T, type: string): type is keyof T & string {
return type in map;
}
if (!isValidDbType(timestampSyntaxMap, dbType)) {
throw new Error(`Unsupported database type: ${dbType}`);
}
const timestampSyntax = timestampSyntaxMap[dbType];Alternatively, use an assertion with runtime check before the cast.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/db/src/entities/abstract-entity.ts, line 22:
<comment>Using `as keyof typeof` for type narrowing violates the typeguard preference rule. Consider using a type guard function or adding runtime validation:
```typescript
function isValidDbType<T extends Record<string, unknown>>(map: T, type: string): type is keyof T & string {
return type in map;
}
if (!isValidDbType(timestampSyntaxMap, dbType)) {
throw new Error(`Unsupported database type: ${dbType}`);
}
const timestampSyntax = timestampSyntaxMap[dbType];
Alternatively, use an assertion with runtime check before the cast.
@@ -15,22 +15,19 @@ import { generateNanoId } from '../utils/generators'; - mariadb: 'CURRENT_TIMESTAMP(3)', -}[dbType]; +} as const; +const timestampSyntax = timestampSyntaxMap[dbType as keyof typeof timestampSyntaxMap];export const jsonColumnType = dbType === 'sqlite' ? 'simple-json' : 'json';
</file context>
</details>
✅ Addressed in [`fa962db`](https://github.com/n8n-io/n8n/commit/fa962dbead12c2d9e1afb0db64be5c9c70225321)
RicardoE105
force-pushed
the
cat-1836-remove-mariadb-and-mysql-from-codebase
branch
from
3a47828 to
fa962db
Compare
Remove test cases for MySQL and MariaDB database types that are no longer supported as internal n8n databases.
…utilities - Remove mysqldb from getBootstrapDBOptions parameter type - Remove MySQL/MariaDB database creation logic from init() - Remove MySQL/MariaDB foreign key checks from truncate()
- Cast dbType to string in breaking changes rule to detect legacy configs - Remove MySQL/MariaDB case from data table size calculation - Remove MySQL/MariaDB conditionals from insights repository - Simplify database-specific logic to only handle PostgreSQL/SQLite
Fix syntax error in periodStart transform function
Simplify database type mapping to only handle PostgreSQL
- Add type casts for MySQL/MariaDB in breaking changes rule test - Simplify test teardown to only handle PostgreSQL
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/cli/src/modules/breaking-changes/rules/v2/removed-database-types.rule.ts">
<violation number="1" location="packages/cli/src/modules/breaking-changes/rules/v2/removed-database-types.rule.ts:36">
P2: Rule violated: **Prefer Typeguards over Type casting**
Use type annotation instead of type casting. Per the coding guidelines, prefer `const x: A = y` over `const x = y as A` for type specification.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }; | ||
|
|
||
| const dbType = this.globalConfig.database.type; | ||
| const dbType = this.globalConfig.database.type as string; |
There was a problem hiding this comment.
P2: Rule violated: Prefer Typeguards over Type casting
Use type annotation instead of type casting. Per the coding guidelines, prefer const x: A = y over const x = y as A for type specification.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/breaking-changes/rules/v2/removed-database-types.rule.ts, line 36:
<comment>Use type annotation instead of type casting. Per the coding guidelines, prefer `const x: A = y` over `const x = y as A` for type specification.</comment>
<file context>
@@ -33,7 +33,7 @@ export class RemovedDatabaseTypesRule implements IBreakingChangeInstanceRule {
};
- const dbType = this.globalConfig.database.type;
+ const dbType = this.globalConfig.database.type as string;
if (dbType === 'mysqldb' || dbType === 'mariadb') {
</file context>
- Simplify type casting in abstract-entity.ts - Remove unnecessary eslint-disable comment in auth.roles.service.ts
Run pnpm lint:fix to apply consistent formatting across all packages
There was a problem hiding this comment.
1 issue found across 861 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsLemonade/EmbeddingsLemonade.node.ts">
<violation number="1" location="packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsLemonade/EmbeddingsLemonade.node.ts:52">
P2: Removing the `LemonadeApiCredentialsType` type assertion reduces type safety. The type still exists in `LemonadeApi.credentials.ts` and should be used to ensure `credentials.apiKey` and `credentials.baseUrl` are properly typed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsLemonade/EmbeddingsLemonade.node.ts
Outdated
Show resolved
Hide resolved
This reverts commit 07a8516.
Fix formatting issues found by CI in auth.roles.service.ts, db-connection.test.ts, and RefactorExecutionIndices migration
Fix extra blank lines in data-table.repository.ts and insights-by-period.repository.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
E2E Tests: n8n tests passed after 7m 21.1s Run Details
Groups
This message was posted automatically by
currents.dev | Integration Settings
|
- Remove MariaDB and MySQL test jobs (already disabled with if: false) - Update workflow path reference from ci-postgres-mysql.yml to ci-postgres.yml - Keep only SQLite pooled and Postgres test jobs
|
@RicardoE105 Thanks for doing this! As mentioned earlier in Slack I've no bandwidth for big reviews currently, if you can please reassign 🙏🏻 |
Summary
Title self explanatory
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/CAT-1836/remove-mariadb-and-mysql-from-codebase
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)