mirror/chartdb
1
0
Fork 0
mirror of https://github.com/chartdb/chartdb.git synced 2026-02-06 19:49:18 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: enforce NOT NULL for primary key fields (#1061)

Browse Source
This commit is contained in:
Jonathan Fishner
2026-01-13 17:53:00 +02:00
committed by GitHub
parent 5754538d7e
commit b21202e6c7
11 changed files with 59 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
src/hooks/use-update-table-field.ts
View File

@@ -103,6 +103,20 @@ export const useUpdateTableField = (
setLocalPrimaryKey(field.primaryKey);
}, [field.nullable, field.primaryKey]);
// Auto-correct: Primary key fields must be NOT NULL
// This fixes any existing data where PK columns were incorrectly set as nullable
useEffect(() => {
if (field.primaryKey && field.nullable) {
chartDBUpdateField(table.id, field.id, { nullable: false });
}
}, [
field.primaryKey,
field.nullable,
table.id,
field.id,
chartDBUpdateField,
]);
// Use custom updateField if provided, otherwise use the chartDB one
const updateField = useMemo(
() =>
@@ -296,6 +310,7 @@ export const useUpdateTableField = (
// When setting as primary key
const updates: Partial<DBField> = {
primaryKey: true,
nullable: false, // Primary keys must be NOT NULL
};
// Only auto-set unique if this will be the only primary key
if (primaryKeyCount === 0) {
@@ -318,6 +333,10 @@ export const useUpdateTableField = (
const handlePrimaryKeyToggle = useCallback(
(value: boolean) => {
setLocalPrimaryKey(value);
// Primary keys must be NOT NULL - update local state immediately for responsive UI
if (value) {
setLocalNullable(false);
}
debouncedPrimaryKeyUpdate(value, primaryKeyCount);
},
[primaryKeyCount, debouncedPrimaryKeyUpdate]

3
src/lib/clone.ts
View File

@@ -120,6 +120,9 @@ export const cloneTable = (
.map((id) => getNewId(id))
.filter((fieldId): fieldId is string => fieldId !== null),
id,
// Clear the name for primary key indexes to avoid duplicate constraint names
// when exporting SQL scripts (database will auto-generate unique names)
name: index.isPrimaryKey ? '' : index.name,
};
})
.filter((index): index is DBIndex => index !== null);

9
src/lib/data/sql-export/cross-dialect/postgresql/to-mssql.ts
View File

@@ -473,14 +473,7 @@ export function exportPostgreSQLToMSSQL({
}CREATE TABLE ${tableName} (\n${fieldDefinitions.join('\n')}${
// Add PRIMARY KEY as table constraint
primaryKeyFields.length > 0
? `\n ${(() => {
const pkIndex = table.indexes.find(
(idx) => idx.isPrimaryKey
);
return pkIndex?.name
? `CONSTRAINT [${pkIndex.name}] `
: '';
})()}PRIMARY KEY (${primaryKeyFields
? `\n PRIMARY KEY (${primaryKeyFields
.map((f) => `[${f.name}]`)
.join(
', '

10
src/lib/data/sql-export/cross-dialect/postgresql/to-mysql.ts
View File

@@ -397,15 +397,7 @@ export function exportPostgreSQLToMySQL({
}\nCREATE TABLE IF NOT EXISTS ${tableName} (\n${fieldDefinitions.join('\n')}${
// Add PRIMARY KEY as table constraint
primaryKeyFields.length > 0
? `\n ${(() => {
// Find PK index to get the constraint name
const pkIndex = table.indexes.find(
(idx) => idx.isPrimaryKey
);
return pkIndex?.name
? `CONSTRAINT \`${pkIndex.name}\` `
: '';
})()}PRIMARY KEY (${primaryKeyFields
? `\n PRIMARY KEY (${primaryKeyFields
.map((f) => `\`${f.name}\``)
.join(
', '

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

@@ -184,16 +184,7 @@ export function exportMSSQL({
})
.join(',\n')}${
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
);
return pkIndex?.name
? `CONSTRAINT [${pkIndex.name}] `
: '';
})()}PRIMARY KEY (${table.fields
? `,\n PRIMARY KEY (${table.fields
.filter((f) => f.primaryKey)
.map((f) => `[${f.name}]`)
.join(', ')})`

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

@@ -313,16 +313,7 @@ export function exportMySQL({
.join(',\n')}${
// Add PRIMARY KEY as table constraint
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
);
return pkIndex?.name
? `CONSTRAINT \`${pkIndex.name}\` `
: '';
})()}PRIMARY KEY (${primaryKeyFields
? `,\n PRIMARY KEY (${primaryKeyFields
.map((f) => `\`${f.name}\``)
.join(', ')})`
: ''

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

@@ -329,16 +329,7 @@ export function exportPostgreSQL({
})
.join(',\n')}${
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
);
return pkIndex?.name
? `CONSTRAINT "${pkIndex.name}" `
: '';
})()}PRIMARY KEY (${primaryKeyFields
? `,\n PRIMARY KEY (${primaryKeyFields
.map((f) => `"${f.name}"`)
.join(', ')})`
: ''

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

@@ -511,15 +511,9 @@ export const exportBaseSQL = ({
}
}
// Handle PRIMARY KEY constraint - only add inline if single PK without named constraint
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 (
field.primaryKey &&
!hasCompositePrimaryKey &&
!useNamedConstraint
) {
// Handle PRIMARY KEY constraint - add inline for single PK fields
// Never use named constraints to avoid duplicate constraint name issues
if (field.primaryKey && !hasCompositePrimaryKey) {
sqlScript += ' PRIMARY KEY';
// For SQLite with DBML flow, add AUTOINCREMENT after PRIMARY KEY
@@ -534,31 +528,19 @@ export const exportBaseSQL = ({
}
}
// Add a comma after each field except the last one (or before PK constraint)
const needsPKConstraint =
hasCompositePrimaryKey ||
(primaryKeyFields.length === 1 && useNamedConstraint);
if (index < table.fields.length - 1 || needsPKConstraint) {
// Add a comma after each field except the last one (or before composite PK constraint)
if (index < table.fields.length - 1 || hasCompositePrimaryKey) {
sqlScript += ',\n';
}
});
// 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;
const needsPKConstraint =
hasCompositePrimaryKey ||
(primaryKeyFields.length === 1 && useNamedConstraint);
if (needsPKConstraint) {
// Add primary key constraint for composite PKs only (single PKs are inline)
// Never use named constraints to avoid duplicate constraint name issues
if (hasCompositePrimaryKey) {
const pkFieldNames = primaryKeyFields
.map((f) => getQuotedFieldName(f.name, isDBMLFlow))
.join(', ');
if (useNamedConstraint) {
sqlScript += `\n CONSTRAINT ${pkIndex.name} PRIMARY KEY (${pkFieldNames})`;
} else {
sqlScript += `\n PRIMARY KEY (${pkFieldNames})`;
}
sqlScript += `\n PRIMARY KEY (${pkFieldNames})`;
}
// Add CHECK constraints (only for databases that support them, filter out empty)
@@ -568,10 +550,10 @@ export const exportBaseSQL = ({
);
if (validCheckConstraints.length > 0 && dbSupportsChecks) {
validCheckConstraints.forEach((checkConstraint, idx) => {
// Add comma if needed (after fields or PK constraint)
// Add comma if needed (after fields or composite PK constraint)
if (
idx === 0 &&
(table.fields.length > 0 || needsPKConstraint)
(table.fields.length > 0 || hasCompositePrimaryKey)
) {
sqlScript += ',';
} else if (idx > 0) {

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

@@ -53,12 +53,12 @@ Table "landlord"."users_master_table" {
expect(uniqueIndex!.unique).toBe(true);
});
it('should export composite primary key with CONSTRAINT name in PostgreSQL', async () => {
it('should export composite primary key without CONSTRAINT name in PostgreSQL (auto-generated)', async () => {
const dbmlContent = `
Table "users" {
"id" bigint [not null]
"tenant_id" bigint [not null]
Indexes {
(id, tenant_id) [pk, name: "pk_users_composite"]
}
@@ -71,19 +71,17 @@ Table "users" {
const sqlScript = exportPostgreSQL({ diagram });
// Check that the SQL contains the named constraint
expect(sqlScript).toContain(
'CONSTRAINT "pk_users_composite" PRIMARY KEY ("id", "tenant_id")'
);
expect(sqlScript).not.toContain('PRIMARY KEY ("id", "tenant_id"),'); // Should not have unnamed PK
// PK constraint names are auto-generated by the database to avoid duplicates
expect(sqlScript).toContain('PRIMARY KEY ("id", "tenant_id")');
expect(sqlScript).not.toContain('CONSTRAINT "pk_users_composite"');
});
it('should export composite primary key with CONSTRAINT name in MySQL', async () => {
it('should export composite primary key without CONSTRAINT name in MySQL (auto-generated)', async () => {
const dbmlContent = `
Table "orders" {
"order_id" int [not null]
"product_id" int [not null]
Indexes {
(order_id, product_id) [pk, name: "orders_order_product_pk"]
}
@@ -96,18 +94,17 @@ Table "orders" {
const sqlScript = exportMySQL({ diagram });
// Check that the SQL contains the named constraint
expect(sqlScript).toContain(
'CONSTRAINT `orders_order_product_pk` PRIMARY KEY (`order_id`, `product_id`)'
);
// PK constraint names are auto-generated by the database to avoid duplicates
expect(sqlScript).toContain('PRIMARY KEY (`order_id`, `product_id`)');
expect(sqlScript).not.toContain('CONSTRAINT `orders_order_product_pk`');
});
it('should export composite primary key with CONSTRAINT name in MSSQL', async () => {
it('should export composite primary key without CONSTRAINT name in MSSQL (auto-generated)', async () => {
const dbmlContent = `
Table "products" {
"category_id" int [not null]
"product_id" int [not null]
Indexes {
(category_id, product_id) [pk, name: "pk_products"]
}
@@ -120,10 +117,11 @@ Table "products" {
const sqlScript = exportMSSQL({ diagram });
// Check that the SQL contains the named constraint
// PK constraint names are auto-generated by the database to avoid duplicates
expect(sqlScript).toContain(
'CONSTRAINT [pk_products] PRIMARY KEY ([category_id], [product_id])'
'PRIMARY KEY ([category_id], [product_id])'
);
expect(sqlScript).not.toContain('CONSTRAINT [pk_products]');
});
it('should merge duplicate PK index with name', async () => {

2
src/pages/editor-page/canvas/table-node/table-edit-mode/table-edit-mode-field.tsx
View File

@@ -155,7 +155,7 @@ export const TableEditModeField: React.FC<TableEditModeFieldProps> = React.memo(
<TableFieldToggle
pressed={nullable}
onPressedChange={handleNullableToggle}
disabled={typeRequiresNotNull}
disabled={typeRequiresNotNull || primaryKey}
>
N
</TableFieldToggle>

6
src/pages/editor-page/side-panel/tables-section/table-list/table-list-item/table-list-item-content/table-field/table-field.tsx
View File

@@ -161,7 +161,11 @@ export const TableField: React.FC<TableFieldProps> = ({
<TableFieldToggle
pressed={nullable}
onPressedChange={handleNullableToggle}
disabled={readonly || typeRequiresNotNull}
disabled={
readonly ||
typeRequiresNotNull ||
primaryKey
}
>
N
</TableFieldToggle>