fix: correct relationship direction in SQL import (#1003)

2025-12-17 01:25:09 -06:00 · 2025-12-09 16:35:33 +02:00
parent 8b31944f67
commit 9b6d6dbb3c
4 changed files with 372 additions and 10 deletions
--- a/src/lib/data/sql-import/tests/sql-import.test.ts
+++ b/src/lib/data/sql-import/tests/sql-import.test.ts
@@ -0,0 +1,338 @@
+import { describe, it, expect } from 'vitest';
+import { sqlImportToDiagram } from '../index';
+import { DatabaseType } from '@/lib/domain/database-type';
+
+describe('sqlImportToDiagram', () => {
+    it('should parse a simple PostgreSQL table and return a valid diagram', async () => {
+        const sql = `
+            CREATE TABLE users (
+                id SERIAL PRIMARY KEY,
+                name VARCHAR(255) NOT NULL,
+                email VARCHAR(255) UNIQUE
+            );
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.POSTGRESQL,
+            targetDatabaseType: DatabaseType.POSTGRESQL,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+        expect(diagram.id).toBeDefined();
+        expect(diagram.databaseType).toBe(DatabaseType.POSTGRESQL);
+
+        // Verify table was parsed
+        expect(diagram.tables).toHaveLength(1);
+        expect(diagram.tables?.[0].name).toBe('users');
+
+        // Verify fields were parsed
+        const fields = diagram.tables?.[0].fields;
+        expect(fields).toHaveLength(3);
+
+        const fieldNames = fields?.map((f) => f.name);
+        expect(fieldNames).toContain('id');
+        expect(fieldNames).toContain('name');
+        expect(fieldNames).toContain('email');
+
+        // Verify primary key
+        const idField = fields?.find((f) => f.name === 'id');
+        expect(idField?.primaryKey).toBe(true);
+
+        // Verify nullable constraints
+        const nameField = fields?.find((f) => f.name === 'name');
+        expect(nameField?.nullable).toBe(false);
+
+        // Verify unique constraint
+        const emailField = fields?.find((f) => f.name === 'email');
+        expect(emailField?.unique).toBe(true);
+    });
+
+    it('should parse foreign key constraints properly', async () => {
+        const sql = `
+            CREATE SCHEMA IF NOT EXISTS "public";
+
+CREATE TABLE "public"."playlists" (
+    "playlist_id" SERIAL,
+    "user_id" int NOT NULL,
+    PRIMARY KEY ("playlist_id")
+);
+
+CREATE TABLE "public"."users" (
+    "user_id" SERIAL,
+    PRIMARY KEY ("user_id")
+);
+
+-- Foreign key constraints
+-- Schema: public
+ALTER TABLE "public"."playlists" ADD CONSTRAINT "fk_playlists_user_id_users_user_id" FOREIGN KEY("user_id") REFERENCES "public"."users"("user_id");
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.POSTGRESQL,
+            targetDatabaseType: DatabaseType.POSTGRESQL,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+
+        const playlistTable = diagram.tables?.find(
+            (t) => t.name === 'playlists'
+        );
+        expect(playlistTable).toBeDefined();
+
+        const playlistUserIdField = playlistTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(playlistUserIdField).toBeDefined();
+
+        const usersTable = diagram.tables?.find((t) => t.name === 'users');
+        expect(usersTable).toBeDefined();
+
+        const usersUserIdField = usersTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(usersUserIdField).toBeDefined();
+
+        // verify relationships
+        expect(diagram.relationships).toBeDefined();
+        expect(diagram.relationships).toHaveLength(1);
+
+        const relationship = diagram.relationships?.[0];
+        expect(relationship?.sourceSchema).toBe('public');
+        expect(relationship?.sourceTableId).toBe(usersTable?.id);
+        expect(relationship?.sourceFieldId).toBe(usersUserIdField?.id);
+        expect(relationship?.sourceCardinality).toBe('one');
+
+        expect(relationship?.targetSchema).toBe('public');
+        expect(relationship?.targetTableId).toBe(playlistTable?.id);
+        expect(relationship?.targetFieldId).toBe(playlistUserIdField?.id);
+        expect(relationship?.targetCardinality).toBe('many');
+    });
+
+    it('should parse foreign key constraints properly - MySQL', async () => {
+        const sql = `
+CREATE TABLE \`users\` (
+    \`user_id\` INT AUTO_INCREMENT,
+    PRIMARY KEY (\`user_id\`)
+) ENGINE=InnoDB;
+
+CREATE TABLE \`playlists\` (
+    \`playlist_id\` INT AUTO_INCREMENT,
+    \`user_id\` INT NOT NULL,
+    PRIMARY KEY (\`playlist_id\`),
+    CONSTRAINT \`fk_playlists_user_id\` FOREIGN KEY (\`user_id\`) REFERENCES \`users\`(\`user_id\`)
+) ENGINE=InnoDB;
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.MYSQL,
+            targetDatabaseType: DatabaseType.MYSQL,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+
+        const playlistTable = diagram.tables?.find(
+            (t) => t.name === 'playlists'
+        );
+        expect(playlistTable).toBeDefined();
+
+        const playlistUserIdField = playlistTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(playlistUserIdField).toBeDefined();
+
+        const usersTable = diagram.tables?.find((t) => t.name === 'users');
+        expect(usersTable).toBeDefined();
+
+        const usersUserIdField = usersTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(usersUserIdField).toBeDefined();
+
+        // verify relationships
+        expect(diagram.relationships).toBeDefined();
+        expect(diagram.relationships).toHaveLength(1);
+
+        const relationship = diagram.relationships?.[0];
+        expect(relationship?.sourceTableId).toBe(usersTable?.id);
+        expect(relationship?.sourceFieldId).toBe(usersUserIdField?.id);
+        expect(relationship?.sourceCardinality).toBe('one');
+
+        expect(relationship?.targetTableId).toBe(playlistTable?.id);
+        expect(relationship?.targetFieldId).toBe(playlistUserIdField?.id);
+        expect(relationship?.targetCardinality).toBe('many');
+    });
+
+    it('should parse foreign key constraints properly - MariaDB', async () => {
+        const sql = `
+CREATE TABLE \`users\` (
+    \`user_id\` INT AUTO_INCREMENT,
+    PRIMARY KEY (\`user_id\`)
+) ENGINE=InnoDB;
+
+CREATE TABLE \`playlists\` (
+    \`playlist_id\` INT AUTO_INCREMENT,
+    \`user_id\` INT NOT NULL,
+    PRIMARY KEY (\`playlist_id\`),
+    CONSTRAINT \`fk_playlists_user_id\` FOREIGN KEY (\`user_id\`) REFERENCES \`users\`(\`user_id\`)
+) ENGINE=InnoDB;
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.MARIADB,
+            targetDatabaseType: DatabaseType.MARIADB,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+
+        const playlistTable = diagram.tables?.find(
+            (t) => t.name === 'playlists'
+        );
+        expect(playlistTable).toBeDefined();
+
+        const playlistUserIdField = playlistTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(playlistUserIdField).toBeDefined();
+
+        const usersTable = diagram.tables?.find((t) => t.name === 'users');
+        expect(usersTable).toBeDefined();
+
+        const usersUserIdField = usersTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(usersUserIdField).toBeDefined();
+
+        // verify relationships
+        expect(diagram.relationships).toBeDefined();
+        expect(diagram.relationships).toHaveLength(1);
+
+        const relationship = diagram.relationships?.[0];
+        expect(relationship?.sourceTableId).toBe(usersTable?.id);
+        expect(relationship?.sourceFieldId).toBe(usersUserIdField?.id);
+        expect(relationship?.sourceCardinality).toBe('one');
+
+        expect(relationship?.targetTableId).toBe(playlistTable?.id);
+        expect(relationship?.targetFieldId).toBe(playlistUserIdField?.id);
+        expect(relationship?.targetCardinality).toBe('many');
+    });
+
+    it('should parse foreign key constraints properly - SQL Server', async () => {
+        const sql = `
+CREATE TABLE [dbo].[users] (
+    [user_id] INT IDENTITY(1,1) NOT NULL,
+    PRIMARY KEY ([user_id])
+);
+
+CREATE TABLE [dbo].[playlists] (
+    [playlist_id] INT IDENTITY(1,1) NOT NULL,
+    [user_id] INT NOT NULL,
+    PRIMARY KEY ([playlist_id]),
+    CONSTRAINT [fk_playlists_user_id] FOREIGN KEY ([user_id]) REFERENCES [dbo].[users]([user_id])
+);
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.SQL_SERVER,
+            targetDatabaseType: DatabaseType.SQL_SERVER,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+
+        const playlistTable = diagram.tables?.find(
+            (t) => t.name === 'playlists'
+        );
+        expect(playlistTable).toBeDefined();
+
+        const playlistUserIdField = playlistTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(playlistUserIdField).toBeDefined();
+
+        const usersTable = diagram.tables?.find((t) => t.name === 'users');
+        expect(usersTable).toBeDefined();
+
+        const usersUserIdField = usersTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(usersUserIdField).toBeDefined();
+
+        // verify relationships
+        expect(diagram.relationships).toBeDefined();
+        expect(diagram.relationships).toHaveLength(1);
+
+        const relationship = diagram.relationships?.[0];
+        expect(relationship?.sourceSchema).toBe('dbo');
+        expect(relationship?.sourceTableId).toBe(usersTable?.id);
+        expect(relationship?.sourceFieldId).toBe(usersUserIdField?.id);
+        expect(relationship?.sourceCardinality).toBe('one');
+
+        expect(relationship?.targetSchema).toBe('dbo');
+        expect(relationship?.targetTableId).toBe(playlistTable?.id);
+        expect(relationship?.targetFieldId).toBe(playlistUserIdField?.id);
+        expect(relationship?.targetCardinality).toBe('many');
+    });
+
+    it('should parse foreign key constraints properly - SQLite', async () => {
+        const sql = `
+CREATE TABLE users (
+    user_id INTEGER PRIMARY KEY AUTOINCREMENT
+);
+
+CREATE TABLE playlists (
+    playlist_id INTEGER PRIMARY KEY AUTOINCREMENT,
+    user_id INTEGER NOT NULL,
+    FOREIGN KEY (user_id) REFERENCES users(user_id)
+);
+        `;
+
+        const diagram = await sqlImportToDiagram({
+            sqlContent: sql,
+            sourceDatabaseType: DatabaseType.SQLITE,
+            targetDatabaseType: DatabaseType.SQLITE,
+        });
+
+        // Verify diagram structure
+        expect(diagram).toBeDefined();
+
+        const playlistTable = diagram.tables?.find(
+            (t) => t.name === 'playlists'
+        );
+        expect(playlistTable).toBeDefined();
+
+        const playlistUserIdField = playlistTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(playlistUserIdField).toBeDefined();
+
+        const usersTable = diagram.tables?.find((t) => t.name === 'users');
+        expect(usersTable).toBeDefined();
+
+        const usersUserIdField = usersTable?.fields?.find(
+            (f) => f.name === 'user_id'
+        );
+        expect(usersUserIdField).toBeDefined();
+
+        // verify relationships
+        expect(diagram.relationships).toBeDefined();
+        expect(diagram.relationships).toHaveLength(1);
+
+        const relationship = diagram.relationships?.[0];
+        expect(relationship?.sourceTableId).toBe(usersTable?.id);
+        expect(relationship?.sourceFieldId).toBe(usersUserIdField?.id);
+        expect(relationship?.sourceCardinality).toBe('one');
+
+        expect(relationship?.targetTableId).toBe(playlistTable?.id);
+        expect(relationship?.targetFieldId).toBe(playlistUserIdField?.id);
+        expect(relationship?.targetCardinality).toBe('many');
+    });
+});
--- a/src/lib/data/sql-import/common.ts
+++ b/src/lib/data/sql-import/common.ts
@@ -917,22 +917,25 @@ export function convertToChartDBDiagram(
        }

        // Use the cardinality from the SQL parser if available, otherwise determine it
+        // Note: In SQLForeignKey, source = table with FK, target = referenced table
+        // In DBRelationship, we want source = referenced table (PK), target = FK table
+        // So we swap them here
        const sourceCardinality =
-            rel.sourceCardinality ||
-            (sourceField.unique || sourceField.primaryKey ? 'one' : 'many');
-        const targetCardinality =
            rel.targetCardinality ||
            (targetField.unique || targetField.primaryKey ? 'one' : 'many');
+        const targetCardinality =
+            rel.sourceCardinality ||
+            (sourceField.unique || sourceField.primaryKey ? 'one' : 'many');

        relationships.push({
            id: generateId(),
            name: rel.name,
-            sourceSchema: sourceTable.schema,
-            targetSchema: targetTable.schema,
-            sourceTableId: sourceTableId,
-            targetTableId: targetTableId,
-            sourceFieldId: sourceField.id,
-            targetFieldId: targetField.id,
+            sourceSchema: targetTable.schema,
+            targetSchema: sourceTable.schema,
+            sourceTableId: targetTableId,
+            targetTableId: sourceTableId,
+            sourceFieldId: targetField.id,
+            targetFieldId: sourceField.id,
            sourceCardinality,
            targetCardinality,
            createdAt: Date.now(),
--- a/src/lib/data/sql-import/dialect-importers/sqlserver/tests/sqlserver-full-flow.test.ts
+++ b/src/lib/data/sql-import/dialect-importers/sqlserver/tests/sqlserver-full-flow.test.ts
@@ -43,7 +43,7 @@ CREATE TABLE [DBO].[SpellComponent](
                r.targetFieldId && // Must have field IDs
                diagram.tables?.some(
                    (t) =>
-                        t.id === r.sourceTableId && t.name === 'SpellDefinition'
+                        t.id === r.targetTableId && t.name === 'SpellDefinition'
                )
        );
        expect(fk1).toBeDefined();
--- a/src/lib/data/sql-import/dialect-importers/sqlserver/sqlserver.ts
+++ b/src/lib/data/sql-import/dialect-importers/sqlserver/sqlserver.ts
@@ -371,6 +371,27 @@ function parseCreateTableManually(
            continue;
        }

+        // Handle standalone PRIMARY KEY definitions (without CONSTRAINT keyword)
+        // Format: PRIMARY KEY (column1, column2, ...)
+        if (part.match(/^\s*PRIMARY\s+KEY/i)) {
+            const pkColumnsMatch = part.match(
+                /PRIMARY\s+KEY(?:\s+CLUSTERED)?\s*\(([\s\S]+?)\)/i
+            );
+            if (pkColumnsMatch) {
+                const pkColumns = pkColumnsMatch[1].split(',').map((c) =>
+                    c
+                        .trim()
+                        .replace(/\[|\]|\s+(ASC|DESC)/gi, '')
+                        .trim()
+                );
+                pkColumns.forEach((col) => {
+                    const column = columns.find((c) => c.name === col);
+                    if (column) column.primaryKey = true;
+                });
+            }
+            continue;
+        }
+
        // Handle constraint definitions
        if (part.match(/^\s*CONSTRAINT/i)) {
            // Parse constraints