github/dbx
4
0 Fork 0
mirror of https://github.com/t8y2/dbx.git synced 2026-11-15 14:52:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(schema-diff): disambiguate duplicate child IDs

Browse Source 
Closes #8067
This commit is contained in:
t8y2 committed 2026-09-04 00:44:10 +08:00
1 parent 0279a0903c
commit 81587313eb
2 files changed
+121 -48
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

apps/desktop/src/lib/__tests__/schema/schemaDiffSelection.spec.ts
Vendored
+54 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { convertToSchemaDiffObjects, normalizeSchemaDiffDependencyGraph, selectSchemaDiffInput, selectSchemaDiffInputForObject, setSchemaDiffObjectSelected, type SchemaDiffPreparation } from "../../schema/schemaDiff";
import { convertToSchemaDiffObjects, normalizeSchemaDiffDependencyGraph, schemaDiffObjectSelectionState, selectSchemaDiffInput, selectSchemaDiffInputForObject, setSchemaDiffObjectSelected, type SchemaDiffPreparation } from "../../schema/schemaDiff";
function createPreparation(): SchemaDiffPreparation {
return {
@@ -24,6 +24,31 @@ function createPreparation(): SchemaDiffPreparation {
};
}
function createDuplicateForeignKeyPreparation(): SchemaDiffPreparation {
return {
diffs: [
{
type: "modified",
objectType: "table",
name: "orders",
foreignKeys: [
{
type: "removed",
name: "fk_customer",
target: { name: "fk_customer", column: "legacy_customer_id", ref_table: "customers", ref_column: "id" },
},
{
type: "added",
name: "fk_customer",
source: { name: "fk_customer", column: "customer_id", ref_table: "customers", ref_column: "id" },
},
],
},
],
syncSql: "-- orders",
};
}
describe("schema diff SQL selection projections", () => {
it("keeps the focused table SQL separate from the full selected SQL", () => {
const result = createPreparation();
@@ -62,6 +87,34 @@ describe("schema diff SQL selection projections", () => {
expect(focusedInput.diffs[0]?.columns?.map((column) => column.name)).toEqual(["a_col", "a_other_col"]);
});
it("selects duplicate-name foreign keys independently and projects only the selected one", () => {
const result = createDuplicateForeignKeyPreparation();
const objects = convertToSchemaDiffObjects(result.diffs);
const tableObject = objects[0]!;
const [legacyForeignKey, currentForeignKey] = tableObject.children ?? [];
const foreignKeyIds = tableObject.children?.map((child) => child.id) ?? [];
expect(foreignKeyIds).toHaveLength(2);
expect(new Set(foreignKeyIds).size).toBe(2);
expect(schemaDiffObjectSelectionState(tableObject)).toEqual({ checked: true, indeterminate: false });
expect(legacyForeignKey && setSchemaDiffObjectSelected(objects, legacyForeignKey.id, false)).toBe(true);
expect(legacyForeignKey?.selected).toBe(false);
expect(currentForeignKey?.selected).toBe(true);
expect(tableObject.selected).toBe(false);
expect(schemaDiffObjectSelectionState(tableObject)).toEqual({ checked: false, indeterminate: true });
const selectedInput = selectSchemaDiffInput(result, objects);
expect(selectedInput.diffs[0]?.foreignKeys?.map((foreignKey) => foreignKey.source?.column)).toEqual(["customer_id"]);
expect(currentForeignKey && setSchemaDiffObjectSelected(objects, currentForeignKey.id, false)).toBe(true);
expect(schemaDiffObjectSelectionState(tableObject)).toEqual({ checked: false, indeterminate: false });
expect(selectSchemaDiffInput(result, objects).diffs).toEqual([]);
const rebuiltObjects = convertToSchemaDiffObjects(result.diffs);
expect(rebuiltObjects[0]?.children?.map((child) => child.id)).toEqual(foreignKeyIds);
});
it("returns an empty input for an object that is no longer present", () => {
const result = createPreparation();
const objects = convertToSchemaDiffObjects(result.diffs);
apps/desktop/src/lib/schema/schemaDiff.ts
+67 -47
View File
@@ -391,20 +391,26 @@ function tableObjectId(tableName: string): string {
return `table-${tableName}`;
}
function columnObjectId(tableName: string, columnName: string): string {
return `col-${tableName}-${columnName}`;
type SchemaDiffChildKind = "column" | "index" | "foreignKey" | "trigger";
const SCHEMA_DIFF_CHILD_ID_PREFIX: Record<SchemaDiffChildKind, string> = {
column: "col",
index: "idx",
foreignKey: "fk",
trigger: "trg",
};
function diffChildObjectId(kind: SchemaDiffChildKind, tableName: string, childName: string, siblings: readonly { name: string }[], index: number): string {
const baseId = `${SCHEMA_DIFF_CHILD_ID_PREFIX[kind]}-${tableName}-${childName}`;
const duplicateCount = siblings.filter((sibling) => sibling.name === childName).length;
if (duplicateCount < 2) return baseId;
const occurrence = siblings.slice(0, index).filter((sibling) => sibling.name === childName).length;
return `${baseId}-${occurrence}`;
}
function indexObjectId(tableName: string, indexName: string): string {
return `idx-${tableName}-${indexName}`;
}
function foreignKeyObjectId(tableName: string, foreignKeyName: string): string {
return `fk-${tableName}-${foreignKeyName}`;
}
function triggerObjectId(tableName: string, triggerName: string): string {
return `trg-${tableName}-${triggerName}`;
function findDiffChildIndex(kind: SchemaDiffChildKind, tableName: string, siblings: readonly { name: string }[] | undefined, objectId: string): number {
return (siblings ?? []).findIndex((sibling, index) => diffChildObjectId(kind, tableName, sibling.name, siblings ?? [], index) === objectId);
}
function tableOptionObjectId(tableName: string): string {
@@ -613,11 +619,15 @@ export function convertToSchemaDiffObjects(tableDiffs: TableDiff[], functionDiff
const isRenamed = diff.type === "renamed";
const newName = isRenamed && renameCandidates ? (renameCandidates.find((rc) => rc.sourceName === diff.name)?.targetName ?? diff.name) : undefined;
const tableId = tableObjectId(diff.name);
const columns = diff.columns ?? [];
const indexes = diff.indexes ?? [];
const foreignKeys = diff.foreignKeys ?? [];
const triggers = diff.triggers ?? [];
const children: SchemaDiffObject[] =
opType === "modify"
? [
...(diff.columns?.map((column) => ({
id: columnObjectId(diff.name, column.name),
...columns.map((column, index) => ({
id: diffChildObjectId("column", diff.name, column.name, columns, index),
operationType: getOperationType(column.type),
objectKind: "column" as DiffObjectKind,
name: column.name,
@@ -627,9 +637,9 @@ export function convertToSchemaDiffObjects(tableDiffs: TableDiff[], functionDiff
changes: column.changes,
parentId: tableId,
parentName: diff.name,
})) || []),
...(diff.indexes?.map((index) => ({
id: indexObjectId(diff.name, index.name),
})),
...indexes.map((index, childIndex) => ({
id: diffChildObjectId("index", diff.name, index.name, indexes, childIndex),
// A modified index is one DROP + CREATE deploy unit.
operationType: index.type === "modified" ? ("delete" as DiffOperationType) : getOperationType(index.type),
objectKind: "index" as DiffObjectKind,
@@ -640,9 +650,9 @@ export function convertToSchemaDiffObjects(tableDiffs: TableDiff[], functionDiff
changes: index.changes,
parentId: tableId,
parentName: diff.name,
})) || []),
...(diff.foreignKeys?.map((foreignKey) => ({
id: foreignKeyObjectId(diff.name, foreignKey.name),
})),
...foreignKeys.map((foreignKey, index) => ({
id: diffChildObjectId("foreignKey", diff.name, foreignKey.name, foreignKeys, index),
// Modified foreign keys are also dropped before they are recreated.
operationType: foreignKey.type === "modified" ? ("delete" as DiffOperationType) : getOperationType(foreignKey.type),
objectKind: "foreignKey" as DiffObjectKind,
@@ -653,9 +663,9 @@ export function convertToSchemaDiffObjects(tableDiffs: TableDiff[], functionDiff
changes: foreignKey.changes,
parentId: tableId,
parentName: diff.name,
})) || []),
...(diff.triggers?.map((trigger) => ({
id: triggerObjectId(diff.name, trigger.name),
})),
...triggers.map((trigger, index) => ({
id: diffChildObjectId("trigger", diff.name, trigger.name, triggers, index),
operationType: getOperationType(trigger.type),
objectKind: "trigger" as DiffObjectKind,
name: trigger.name,
@@ -665,7 +675,7 @@ export function convertToSchemaDiffObjects(tableDiffs: TableDiff[], functionDiff
changes: trigger.changes,
parentId: tableId,
parentName: diff.name,
})) || []),
})),
...(diff.sourceTableComment !== diff.targetTableComment
? [
{
@@ -805,10 +815,10 @@ export function selectSchemaDiffInput(result: SchemaDiffPreparation, objects: Sc
return tableObject.selected ? [{ ...diff, syncSql: undefined }] : [];
}
const columns = diff.columns?.filter((column) => selectedIds.has(columnObjectId(diff.name, column.name))) ?? [];
const indexes = diff.indexes?.filter((index) => selectedIds.has(indexObjectId(diff.name, index.name))) ?? [];
const foreignKeys = diff.foreignKeys?.filter((foreignKey) => selectedIds.has(foreignKeyObjectId(diff.name, foreignKey.name))) ?? [];
const triggers = diff.triggers?.filter((trigger) => selectedIds.has(triggerObjectId(diff.name, trigger.name))) ?? [];
const columns = diff.columns?.filter((column, index) => selectedIds.has(diffChildObjectId("column", diff.name, column.name, diff.columns ?? [], index))) ?? [];
const indexes = diff.indexes?.filter((index, childIndex) => selectedIds.has(diffChildObjectId("index", diff.name, index.name, diff.indexes ?? [], childIndex))) ?? [];
const foreignKeys = diff.foreignKeys?.filter((foreignKey, index) => selectedIds.has(diffChildObjectId("foreignKey", diff.name, foreignKey.name, diff.foreignKeys ?? [], index))) ?? [];
const triggers = diff.triggers?.filter((trigger, index) => selectedIds.has(diffChildObjectId("trigger", diff.name, trigger.name, diff.triggers ?? [], index))) ?? [];
const includeTableOptions = selectedIds.has(tableOptionObjectId(diff.name));
if (columns.length === 0 && indexes.length === 0 && foreignKeys.length === 0 && triggers.length === 0 && !includeTableOptions) {
@@ -1126,17 +1136,25 @@ export function setSchemaDiffObjectSelectedWithDependencies(objects: SchemaDiffO
if (!tableObject || !tableDiff) return;
setSchemaDiffObjectSelected(objects, id, value);
const child = (kind: DiffObjectKind, name: string) => tableObject.children?.find((candidate) => candidate.objectKind === kind && candidate.name === name);
const child = <Child extends { name: string }>(kind: SchemaDiffChildKind, collection: readonly Child[] | undefined, index: number) => {
const candidate = collection?.[index];
if (!candidate) return undefined;
const childId = diffChildObjectId(kind, tableObject.name, candidate.name, collection, index);
return tableObject.children?.find((candidateObject) => candidateObject.id === childId);
};
const applyColumn = (name: string, value: boolean) => {
const columnDiff = tableDiff.columns?.find((column) => column.name === name);
const columnObject = child("column", name);
const columnIndex = tableDiff.columns?.findIndex((column) => column.name === name) ?? -1;
const columnDiff = columnIndex < 0 ? undefined : tableDiff.columns?.[columnIndex];
const columnObject = columnIndex < 0 ? undefined : child("column", tableDiff.columns, columnIndex);
if (!columnDiff || !columnObject) return;
if (value && !["added", "renamed"].includes(columnDiff.type)) return;
apply(columnObject.id, value);
};
if (object.objectKind === "column") {
const columnDiff = tableDiff.columns?.find((column) => column.name === object.name);
const columnIndex = findDiffChildIndex("column", tableObject.name, tableDiff.columns, object.id);
const columnDiff = columnIndex < 0 ? undefined : tableDiff.columns?.[columnIndex];
if (!columnDiff) return;
if (value && columnDiff.type === "added" && columnDiff.addPosition && typeof columnDiff.addPosition === "object") {
@@ -1144,47 +1162,49 @@ export function setSchemaDiffObjectSelectedWithDependencies(objects: SchemaDiffO
}
if (value && columnDiff.type === "removed") {
for (const index of tableDiff.indexes ?? []) {
if (!["removed", "modified"].includes(index.type) || !index.target?.columns.includes(columnDiff.name)) continue;
const indexObject = child("index", index.name);
for (const [index, indexDiff] of (tableDiff.indexes ?? []).entries()) {
if (!["removed", "modified"].includes(indexDiff.type) || !indexDiff.target?.columns.includes(columnDiff.name)) continue;
const indexObject = child("index", tableDiff.indexes, index);
if (indexObject) apply(indexObject.id, true);
}
for (const foreignKey of tableDiff.foreignKeys ?? []) {
for (const [index, foreignKey] of (tableDiff.foreignKeys ?? []).entries()) {
if (!["removed", "modified"].includes(foreignKey.type) || foreignKey.target?.column !== columnDiff.name) continue;
const foreignKeyObject = child("foreignKey", foreignKey.name);
const foreignKeyObject = child("foreignKey", tableDiff.foreignKeys, index);
if (foreignKeyObject) apply(foreignKeyObject.id, true);
}
}
if (!value && ["added", "renamed"].includes(columnDiff.type)) {
for (const dependentColumn of tableDiff.columns ?? []) {
for (const [index, dependentColumn] of (tableDiff.columns ?? []).entries()) {
if (dependentColumn.type !== "added" || !dependentColumn.addPosition || typeof dependentColumn.addPosition !== "object" || dependentColumn.addPosition.after !== columnDiff.name) continue;
const dependentObject = child("column", dependentColumn.name);
const dependentObject = child("column", tableDiff.columns, index);
if (dependentObject) apply(dependentObject.id, false);
}
for (const index of tableDiff.indexes ?? []) {
const sourceColumns = [...(index.source?.columns ?? []), ...(index.source?.included_columns ?? [])];
if (!["added", "modified"].includes(index.type) || !sourceColumns.includes(columnDiff.name)) continue;
const indexObject = child("index", index.name);
for (const [index, indexDiff] of (tableDiff.indexes ?? []).entries()) {
const sourceColumns = [...(indexDiff.source?.columns ?? []), ...(indexDiff.source?.included_columns ?? [])];
if (!["added", "modified"].includes(indexDiff.type) || !sourceColumns.includes(columnDiff.name)) continue;
const indexObject = child("index", tableDiff.indexes, index);
if (indexObject) apply(indexObject.id, false);
}
for (const foreignKey of tableDiff.foreignKeys ?? []) {
for (const [index, foreignKey] of (tableDiff.foreignKeys ?? []).entries()) {
if (!["added", "modified"].includes(foreignKey.type) || foreignKey.source?.column !== columnDiff.name) continue;
const foreignKeyObject = child("foreignKey", foreignKey.name);
const foreignKeyObject = child("foreignKey", tableDiff.foreignKeys, index);
if (foreignKeyObject) apply(foreignKeyObject.id, false);
}
}
}
if (value && object.objectKind === "index") {
const index = tableDiff.indexes?.find((candidate) => candidate.name === object.name);
const indexIndex = findDiffChildIndex("index", tableObject.name, tableDiff.indexes, object.id);
const index = indexIndex < 0 ? undefined : tableDiff.indexes?.[indexIndex];
for (const columnName of [...(index?.source?.columns ?? []), ...(index?.source?.included_columns ?? [])]) {
applyColumn(columnName, true);
}
}
if (value && object.objectKind === "foreignKey") {
const foreignKey = tableDiff.foreignKeys?.find((candidate) => candidate.name === object.name);
const foreignKeyIndex = findDiffChildIndex("foreignKey", tableObject.name, tableDiff.foreignKeys, object.id);
const foreignKey = foreignKeyIndex < 0 ? undefined : tableDiff.foreignKeys?.[foreignKeyIndex];
if (foreignKey?.source?.column) applyColumn(foreignKey.source.column, true);
}
};