mirror/chartdb
1
0
Fork 0
mirror of https://github.com/chartdb/chartdb.git synced 2026-02-09 04:59:29 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: pk indexes sql export (#1024)

Browse Source
This commit is contained in:
Guy Ben-Aharon
2025-12-18 20:02:33 +02:00
committed by GitHub
parent a5d1f40b6b
commit 70605324ec
9 changed files with 259 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/lib/data/import-metadata/import/index.ts
View File

@@ -1,5 +1,9 @@
import type { DatabaseEdition, Diagram } from '@/lib/domain';
import { adjustTablePositions, DatabaseType } from '@/lib/domain';
import {
adjustTablePositions,
DatabaseType,
getTableIndexesWithPrimaryKey,
} from '@/lib/domain';
import { generateDiagramId } from '@/lib/utils';
import type { DatabaseMetadata } from '../metadata-types/database-metadata';
import { createCustomTypesFromMetadata } from './custom-types';
@@ -52,14 +56,19 @@ export const loadFromDatabaseMetadata = async ({
mode: 'perSchema',
});
const sortedTables = adjustedTables.sort((a, b) => {
if (a.isView === b.isView) {
// Both are either tables or views, so sort alphabetically by name
return a.name.localeCompare(b.name);
}
// If one is a view and the other is not, put tables first
return a.isView ? 1 : -1;
});
const sortedTables = adjustedTables
.map((table) => ({
...table,
indexes: getTableIndexesWithPrimaryKey({ table }),
}))
.sort((a, b) => {
if (a.isView === b.isView) {
// Both are either tables or views, so sort alphabetically by name
return a.name.localeCompare(b.name);
}
// If one is a view and the other is not, put tables first
return a.isView ? 1 : -1;
});
const diagram: Diagram = {
id: generateDiagramId(),

200
src/lib/data/sql-export/__tests__/export-sql.test.ts Normal file
View File

@@ -0,0 +1,200 @@
import { describe, it, expect } from 'vitest';
import { exportBaseSQL } from '../export-sql-script';
import { exportPostgreSQL } from '../export-per-type/postgresql';
import { exportMySQL } from '../export-per-type/mysql';
import { exportMSSQL } from '../export-per-type/mssql';
import { exportSQLite } from '../export-per-type/sqlite';
import { DatabaseType } from '@/lib/domain/database-type';
import type { Diagram } from '@/lib/domain/diagram';
import type { DBTable } from '@/lib/domain/db-table';
import type { DBField } from '@/lib/domain/db-field';
describe('SQL Export Tests', () => {
let idCounter = 0;
const testId = () => `test-id-${++idCounter}`;
const testTime = Date.now();
const createField = (overrides: Partial<DBField>): DBField =>
({
id: testId(),
name: 'field',
type: { id: 'text', name: 'text' },
primaryKey: false,
nullable: true,
unique: false,
createdAt: testTime,
...overrides,
}) as DBField;
const createTable = (overrides: Partial<DBTable>): DBTable =>
({
id: testId(),
name: 'table',
fields: [],
indexes: [],
createdAt: testTime,
x: 0,
y: 0,
width: 200,
...overrides,
}) as DBTable;
const createDiagram = (overrides: Partial<Diagram>): Diagram =>
({
id: testId(),
name: 'diagram',
databaseType: DatabaseType.GENERIC,
tables: [],
relationships: [],
createdAt: testTime,
updatedAt: testTime,
...overrides,
}) as Diagram;
const createTestDiagramWithPKIndex = (
databaseType: DatabaseType
): { diagram: Diagram; fieldId: string } => {
const fieldId = testId();
const diagram = createDiagram({
id: testId(),
name: 'PK Test',
databaseType,
tables: [
createTable({
id: testId(),
name: 'table_1',
schema: 'public',
fields: [
createField({
id: fieldId,
name: 'id',
type: { id: 'bigint', name: 'bigint' },
primaryKey: true,
nullable: false,
unique: false,
}),
],
indexes: [
{
id: testId(),
name: '', // Empty name indicates auto-generated PK index
unique: true,
fieldIds: [fieldId],
createdAt: testTime,
isPrimaryKey: true,
},
],
}),
],
relationships: [],
});
return { diagram, fieldId };
};
describe('Primary Key Index Export', () => {
describe('exportBaseSQL', () => {
it('should export PRIMARY KEY without CONSTRAINT for PostgreSQL', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.POSTGRESQL
);
const sql = exportBaseSQL({
diagram,
targetDatabaseType: DatabaseType.POSTGRESQL,
});
expect(sql).toContain('PRIMARY KEY ("id")');
expect(sql).not.toContain('CONSTRAINT');
});
it('should export PRIMARY KEY without CONSTRAINT for MySQL', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.MYSQL
);
const sql = exportBaseSQL({
diagram,
targetDatabaseType: DatabaseType.MYSQL,
});
expect(sql).toContain('PRIMARY KEY');
expect(sql).not.toContain('CONSTRAINT');
});
it('should export PRIMARY KEY without CONSTRAINT for SQL Server', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.SQL_SERVER
);
const sql = exportBaseSQL({
diagram,
targetDatabaseType: DatabaseType.SQL_SERVER,
});
expect(sql).toContain('PRIMARY KEY');
expect(sql).not.toContain('CONSTRAINT');
});
it('should export PRIMARY KEY without CONSTRAINT for SQLite', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.SQLITE
);
const sql = exportBaseSQL({
diagram,
targetDatabaseType: DatabaseType.SQLITE,
});
expect(sql).toContain('PRIMARY KEY');
expect(sql).not.toContain('CONSTRAINT');
});
});
describe('Database-specific exporters', () => {
it('exportPostgreSQL: should export PRIMARY KEY without CONSTRAINT', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.POSTGRESQL
);
const sql = exportPostgreSQL({ diagram });
expect(sql).toContain('PRIMARY KEY ("id")');
expect(sql).not.toContain('CONSTRAINT');
});
it('exportMySQL: should export PRIMARY KEY without CONSTRAINT', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.MYSQL
);
const sql = exportMySQL({ diagram });
expect(sql).toContain('PRIMARY KEY (`id`)');
expect(sql).not.toContain('CONSTRAINT');
});
it('exportMSSQL: should export PRIMARY KEY without CONSTRAINT', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.SQL_SERVER
);
const sql = exportMSSQL({ diagram });
expect(sql).toContain('PRIMARY KEY ([id])');
expect(sql).not.toContain('CONSTRAINT');
});
it('exportSQLite: should export PRIMARY KEY without CONSTRAINT', () => {
const { diagram } = createTestDiagramWithPKIndex(
DatabaseType.SQLITE
);
const sql = exportSQLite({ diagram });
// SQLite uses inline PRIMARY KEY for single integer columns
expect(sql).toContain('PRIMARY KEY');
expect(sql).not.toContain('CONSTRAINT');
});
});
});
});

6
src/lib/data/sql-export/export-per-type/mssql.ts
View File

@@ -180,6 +180,7 @@ export function exportMSSQL({
table.fields.filter((f) => f.primaryKey).length > 0
? `,\n ${(() => {
// Find PK index to get the constraint name
// Only use CONSTRAINT syntax if PK index has a non-empty name
const pkIndex = table.indexes.find(
(idx) => idx.isPrimaryKey
);
@@ -194,6 +195,11 @@ export function exportMSSQL({
}\n);\n${(() => {
const validIndexes = table.indexes
.map((index) => {
// Skip primary key indexes - they're already handled as constraints
if (index.isPrimaryKey) {
return '';
}
const indexName = table.schema
? `[${table.schema}_${index.name}]`
: `[${index.name}]`;

1
src/lib/data/sql-export/export-per-type/mysql.ts
View File

@@ -315,6 +315,7 @@ export function exportMySQL({
primaryKeyFields.length > 0
? `,\n ${(() => {
// Find PK index to get the constraint name
// Only use CONSTRAINT syntax if PK index has a non-empty name
const pkIndex = table.indexes.find(
(idx) => idx.isPrimaryKey
);

1
src/lib/data/sql-export/export-per-type/postgresql.ts
View File

@@ -331,6 +331,7 @@ export function exportPostgreSQL({
primaryKeyFields.length > 0
? `,\n ${(() => {
// Find PK index to get the constraint name
// Only use CONSTRAINT syntax if PK index has a non-empty name
const pkIndex = table.indexes.find(
(idx) => idx.isPrimaryKey
);

18
src/lib/data/sql-export/export-sql-script.ts
View File

@@ -469,9 +469,15 @@ export const exportBaseSQL = ({
}
}
// Handle PRIMARY KEY constraint - only add inline if no PK index with custom name
// Handle PRIMARY KEY constraint - only add inline if single PK without named constraint
const pkIndex = table.indexes.find((idx) => idx.isPrimaryKey);
if (field.primaryKey && !hasCompositePrimaryKey && !pkIndex?.name) {
// Only use CONSTRAINT syntax if PK index has a non-empty name
const useNamedConstraint = !!pkIndex?.name;
if (
field.primaryKey &&
!hasCompositePrimaryKey &&
!useNamedConstraint
) {
sqlScript += ' PRIMARY KEY';
// For SQLite with DBML flow, add AUTOINCREMENT after PRIMARY KEY
@@ -489,7 +495,7 @@ export const exportBaseSQL = ({
// Add a comma after each field except the last one (or before PK constraint)
const needsPKConstraint =
hasCompositePrimaryKey ||
(primaryKeyFields.length === 1 && pkIndex?.name);
(primaryKeyFields.length === 1 && useNamedConstraint);
if (index < table.fields.length - 1 || needsPKConstraint) {
sqlScript += ',\n';
}
@@ -497,14 +503,16 @@ export const exportBaseSQL = ({
// Add primary key constraint if needed (for composite PKs or single PK with custom name)
const pkIndex = table.indexes.find((idx) => idx.isPrimaryKey);
// Only use CONSTRAINT syntax if PK index has a non-empty name
const useNamedConstraint = !!pkIndex?.name;
if (
hasCompositePrimaryKey ||
(primaryKeyFields.length === 1 && pkIndex?.name)
(primaryKeyFields.length === 1 && useNamedConstraint)
) {
const pkFieldNames = primaryKeyFields
.map((f) => getQuotedFieldName(f.name, isDBMLFlow))
.join(', ');
if (pkIndex?.name) {
if (useNamedConstraint) {
sqlScript += `\n CONSTRAINT ${pkIndex.name} PRIMARY KEY (${pkFieldNames})`;
} else {
sqlScript += `\n PRIMARY KEY (${pkFieldNames})`;

22
src/lib/data/sql-import/index.ts
View File

@@ -10,6 +10,7 @@ import type { SQLParserResult } from './common';
import { convertToChartDBDiagram } from './common';
import { adjustTablePositions } from '@/lib/domain/db-table';
import { fromMySQL, isMySQLFormat } from './dialect-importers/mysql/mysql';
import { getTableIndexesWithPrimaryKey } from '@/lib/domain/db-index';
/**
* Detect if SQL content is from pg_dump format
@@ -235,14 +236,19 @@ export async function sqlImportToDiagram({
mode: 'perSchema',
});
const sortedTables = adjustedTables.sort((a, b) => {
if (a.isView === b.isView) {
// Both are either tables or views, so sort alphabetically by name
return a.name.localeCompare(b.name);
}
// If one is a view and the other is not, put tables first
return a.isView ? 1 : -1;
});
const sortedTables = adjustedTables
.map((table) => ({
...table,
indexes: getTableIndexesWithPrimaryKey({ table }),
}))
.sort((a, b) => {
if (a.isView === b.isView) {
// Both are either tables or views, so sort alphabetically by name
return a.name.localeCompare(b.name);
}
// If one is a view and the other is not, put tables first
return a.isView ? 1 : -1;
});
return {
...diagram,

7
src/lib/dbml/dbml-import/__tests__/composite-pk-name.test.ts
View File

@@ -177,14 +177,15 @@ Table "simple" {
expect(diagram.tables).toBeDefined();
const table = diagram.tables![0];
// PK index should not exist for composite PK without name
// PK index should exist but with empty name (auto-generated)
const pkIndex = table.indexes.find((idx) => idx.isPrimaryKey);
expect(pkIndex).toBeDefined();
expect(pkIndex!.name).toBe('');
const sqlScript = exportPostgreSQL({ diagram });
// Should have unnamed PRIMARY KEY
// Should have unnamed PRIMARY KEY (no CONSTRAINT for auto-generated PK index)
expect(sqlScript).toContain('PRIMARY KEY ("x", "y")');
expect(sqlScript).toContain('CONSTRAINT');
expect(sqlScript).not.toContain('CONSTRAINT');
});
});

3
src/lib/domain/db-index.ts
View File

@@ -66,9 +66,10 @@ export const getTablePrimaryKeyIndex = ({
};
} else {
// Create new PK index for primary key(s)
// Use empty name for auto-generated PK indexes to indicate no CONSTRAINT should be used
const pkIndex: DBIndex = {
id: generateId(),
name: `pk_${table.name}_${primaryKeyFields.map((f) => f.name).join('_')}`,
name: '',
fieldIds: pkFieldIds,
unique: true,
isPrimaryKey: true,