diff --git a/apps/desktop/src/components/diff/DataCompareDialog.vue b/apps/desktop/src/components/diff/DataCompareDialog.vue index d58e88bce9..5e96993d4f 100644 --- a/apps/desktop/src/components/diff/DataCompareDialog.vue +++ b/apps/desktop/src/components/diff/DataCompareDialog.vue @@ -14,6 +14,7 @@ import { databaseOptionsForConnection, fetchNamespaceOptionsForConnection } from import { isSchemaAware } from "@/lib/database/databaseCapabilities"; import { copyToClipboard } from "@/lib/common/clipboard"; import type { DataCompareCellValue, DataCompareModifiedRow, DataCompareResult, DataCompareRow, DataCompareSyncPlan, DataCompareSyncPlanTableOptions } from "@/lib/dataGrid/dataCompare"; +import { inferCompareKeyColumns } from "@/lib/dataGrid/dataCompare"; import type { ColumnInfo, DatabaseType } from "@/types/database"; import * as api from "@/lib/backend/api"; import { executeWithProductionSqlGuard } from "@/lib/database/productionExecutionGuard"; @@ -394,9 +395,7 @@ async function loadColumnsWithCache(cache: Map, connect async function inferKeyColumnsForTable(table: string, sourceColumnCache?: Map): Promise { if (!sourceConnectionId.value || !sourceDatabase.value || !sourceSchema.value || !table) return []; const columns = sourceColumnCache ? await loadColumnsWithCache(sourceColumnCache, sourceConnectionId.value, sourceDatabase.value, sourceSchema.value, table) : (((await api.getColumns(sourceConnectionId.value, sourceDatabase.value, sourceSchema.value, table)) as CompareColumn[]) ?? []); - const primaryKeys = columns.filter((column) => column.is_primary_key).map((column) => column.name); - if (primaryKeys.length > 0) return primaryKeys; - return columns.slice(0, 1).map((column) => column.name); + return inferCompareKeyColumns(columns); } async function inferKeyColumns() { diff --git a/apps/desktop/src/lib/dataGrid/__tests__/dataCompareKeyInference.spec.ts b/apps/desktop/src/lib/dataGrid/__tests__/dataCompareKeyInference.spec.ts new file mode 100644 index 0000000000..a06c53f443 --- /dev/null +++ b/apps/desktop/src/lib/dataGrid/__tests__/dataCompareKeyInference.spec.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { inferCompareKeyColumns } from "../dataCompare"; + +describe("inferCompareKeyColumns", () => { + it("returns all primary-key columns when a primary key exists", () => { + expect( + inferCompareKeyColumns([ + { name: "id", is_primary_key: true }, + { name: "name", is_primary_key: false }, + ]), + ).toEqual(["id"]); + }); + + it("returns a composite primary key as-is", () => { + expect( + inferCompareKeyColumns([ + { name: "tenant_id", is_primary_key: true }, + { name: "user_id", is_primary_key: true }, + { name: "name", is_primary_key: false }, + ]), + ).toEqual(["tenant_id", "user_id"]); + }); + + it("returns an empty array instead of falling back to the first column when there is no primary key", () => { + expect( + inferCompareKeyColumns([ + { name: "category", is_primary_key: false }, + { name: "name", is_primary_key: false }, + ]), + ).toEqual([]); + }); + + it("returns an empty array for an empty column list", () => { + expect(inferCompareKeyColumns([])).toEqual([]); + }); +}); diff --git a/apps/desktop/src/lib/dataGrid/dataCompare.ts b/apps/desktop/src/lib/dataGrid/dataCompare.ts index 24d66b9406..e154d2d325 100644 --- a/apps/desktop/src/lib/dataGrid/dataCompare.ts +++ b/apps/desktop/src/lib/dataGrid/dataCompare.ts @@ -118,3 +118,24 @@ export interface DataCompareSyncPlan { syncStatements: string[]; syncSql: string; } + +/** + * Safely infers the columns to use as comparison keys for a table. + * + * Only primary-key columns are trusted: a primary key is unique by + * definition, so comparing on it can never produce duplicate-key failures. + * Composite primary keys are returned as-is. + * + * When no primary key exists an empty array is returned so the caller can ask + * the user to pick matching columns explicitly. Falling back to the first + * column would silently select an arbitrary, possibly non-unique field + * (category, status, date, ...) and make the whole table comparison fail with + * an unhelpful duplicate-key error. + */ +export function inferCompareKeyColumns(columns: Pick[]): string[] { + const primaryKeys: string[] = []; + for (const column of columns) { + if (column.is_primary_key) primaryKeys.push(column.name); + } + return primaryKeys; +} diff --git a/crates/dbx-core/src/data_compare.rs b/crates/dbx-core/src/data_compare.rs index fe5dac23d4..a96d78e7d1 100644 --- a/crates/dbx-core/src/data_compare.rs +++ b/crates/dbx-core/src/data_compare.rs @@ -735,7 +735,7 @@ fn collect_compare_rows( for row in rows { let key = key_for_row(&row, key_columns, column_indexes); if items.contains_key(&key) { - return Err(format!("Duplicate {label} key: {key}")); + return Err(duplicate_key_error(label, key_columns, &key)); } order.push(key.clone()); items.insert(key, normalize_row_len(row, columns.len())); @@ -744,6 +744,22 @@ fn collect_compare_rows( Ok((items, order)) } +/// Formats a duplicate-key error so the user can tell which side (source or +/// target) failed, which columns make up the key and which key value repeated. +/// +/// Single-column keys render as `Duplicate source key for column(s) [id]: "1"`; +/// composite keys render as `Duplicate target key for column(s) [a, b]: ["1", "2"]`. +fn duplicate_key_error(label: &str, key_columns: &[String], key: &str) -> String { + let columns = key_columns.join(", "); + let key_display = if key_columns.len() > 1 { + let values = key.split('\u{001f}').collect::>(); + format!("[{}]", values.join(", ")) + } else { + key.to_string() + }; + format!("Duplicate {label} key for column(s) [{columns}]: {key_display}") +} + #[derive(Debug, Clone, Copy)] struct GenerateDataSyncSqlOptions<'a> { table_name: &'a str, @@ -2283,7 +2299,7 @@ mod tests { } #[test] - fn rejects_duplicate_row_keys() { + fn rejects_duplicate_row_keys_with_key_column_context() { let err = compare_data_rows(CompareDataRowsOptions { columns: vec!["id".to_string(), "name".to_string()], key_columns: vec!["id".to_string()], @@ -2292,7 +2308,38 @@ mod tests { }) .expect_err("duplicate source keys should fail"); - assert!(err.contains("Duplicate source key")); + assert!(err.contains("Duplicate source key for column(s) [id]: 1"), "{err}"); + } + + #[test] + fn rejects_duplicate_target_keys_with_key_column_context() { + let err = compare_data_rows(CompareDataRowsOptions { + columns: vec!["id".to_string(), "name".to_string()], + key_columns: vec!["id".to_string()], + source_rows: vec![vec![json!(1), json!("Ada")]], + target_rows: vec![vec![json!(1), json!("Ada")], vec![json!(1), json!("Ada Clone")]], + }) + .expect_err("duplicate target keys should fail"); + + assert!(err.contains("Duplicate target key for column(s) [id]: 1"), "{err}"); + } + + #[test] + fn rejects_duplicate_composite_keys_with_both_columns_named() { + let err = compare_data_rows(CompareDataRowsOptions { + columns: vec!["tenant_id".to_string(), "user_id".to_string(), "name".to_string()], + key_columns: vec!["tenant_id".to_string(), "user_id".to_string()], + source_rows: vec![ + vec![json!("A"), json!(1001), json!("Ada")], + vec![json!("A"), json!(1001), json!("Ada Clone")], + ], + target_rows: vec![vec![json!("A"), json!(1001), json!("Ada")]], + }) + .expect_err("duplicate composite source keys should fail"); + + assert!(err.contains("Duplicate source key"), "{err}"); + assert!(err.contains("[tenant_id, user_id]"), "{err}"); + assert!(err.contains("[\"A\", 1001]"), "{err}"); } #[test]