From 14e1d4f25b7f352a0ed50edf019e698e80bcf5d9 Mon Sep 17 00:00:00 2001 From: zipg Date: Mon, 7 Sep 2026 00:54:59 +0800 Subject: [PATCH] fix(query): enable editing multi-table JOIN result sets --- apps/desktop/src/components/grid/DataGrid.vue | 2 + .../src/components/layout/ContentArea.vue | 1 + .../__tests__/useDataGridEditor.spec.ts | 53 +++++++++++ .../src/composables/useDataGridEditor.ts | 34 ++++++- .../dataGrid/joinedQueryAnalysis.spec.ts | 8 ++ .../dataGrid/joinedRowChanges.spec.ts | 81 +++++++++++++++++ .../src/lib/dataGrid/joinedRowChanges.ts | 89 +++++++++++++++++++ apps/desktop/src/lib/sql/sqlAnalysis.ts | 2 +- apps/desktop/src/lib/tabs/tabResultCache.ts | 2 + ...eryStore.multiSourceColumnComments.spec.ts | 13 ++- apps/desktop/src/stores/queryStore.ts | 29 ++++-- apps/desktop/src/types/database.ts | 2 + crates/dbx-core/src/db/postgres.rs | 4 +- crates/dbx-core/src/query_result_sql.rs | 5 +- crates/dbx-core/src/sql_editability.rs | 27 +++++- packages/app-tests/queryStore.test.ts | 15 ++-- 16 files changed, 345 insertions(+), 22 deletions(-) create mode 100644 apps/desktop/src/lib/__tests__/dataGrid/joinedQueryAnalysis.spec.ts create mode 100644 apps/desktop/src/lib/__tests__/dataGrid/joinedRowChanges.spec.ts create mode 100644 apps/desktop/src/lib/dataGrid/joinedRowChanges.ts diff --git a/apps/desktop/src/components/grid/DataGrid.vue b/apps/desktop/src/components/grid/DataGrid.vue index 84bcdda58e..92a1a1d21e 100644 --- a/apps/desktop/src/components/grid/DataGrid.vue +++ b/apps/desktop/src/components/grid/DataGrid.vue @@ -461,6 +461,7 @@ interface DataGridProps { pageSizePreference?: DataGridPageSizePreference; autoTransposeSingleRow?: boolean; sourceColumns?: Array; + joinedWriteTargets?: import("@/types/database").QueryTab["queryWriteTargets"]; readonlyColumnIndexes?: number[]; /** * Column comments for a multi-source query result (e.g. JOIN), indexed by @@ -4383,6 +4384,7 @@ const editor = useDataGridEditor({ database: computed(() => props.executionDatabase ?? props.database), tableMeta: computed(() => props.tableMeta), sourceColumns: computed(() => props.sourceColumns), + joinedWriteTargets: computed(() => props.joinedWriteTargets), readonlyColumnIndexes: computed(() => (props.readonlyColumnIndexes ? new Set(props.readonlyColumnIndexes) : undefined)), canEditExistingRows, onExecuteSql: computed(() => props.onExecuteSql), diff --git a/apps/desktop/src/components/layout/ContentArea.vue b/apps/desktop/src/components/layout/ContentArea.vue index eb7b926471..89d351acaf 100644 --- a/apps/desktop/src/components/layout/ContentArea.vue +++ b/apps/desktop/src/components/layout/ContentArea.vue @@ -1852,6 +1852,7 @@ defineExpose({ :loading="activeResultIsLoading" :editable="!!activeTab.queryAnalysis || !!mongoQueryResultSaveHandler" :source-columns="activeTab.querySourceColumns" + :joined-write-targets="activeTab.queryWriteTargets" :readonly-column-indexes="groupedQueryReadonlyColumnIndexes(activeTab)" :result-column-comments="activeTab.resultColumnComments" :query-display-source-columns="activeTab.queryDisplaySourceColumns" diff --git a/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts b/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts index c9b8c1f4f0..f97d7f594c 100644 --- a/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts +++ b/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts @@ -627,6 +627,8 @@ describe("useDataGridEditor saveChanges reload", () => { function createSaveTestEditor( options: { + joinedWriteTargets?: import("@/types/database").QueryTab["queryWriteTargets"]; + queryResult?: { columns: string[]; rows: CellValue[][] }; currentPage?: Ref; prepareFullReload?: () => void; customSaveHandler?: { save: ReturnType }; @@ -644,6 +646,7 @@ describe("useDataGridEditor saveChanges reload", () => { [2, "pending"], ], }); + if (options.queryResult) result.value = options.queryResult; const editor = useDataGridEditor({ result: computed(() => result.value), editable: computed(() => true), @@ -659,6 +662,7 @@ describe("useDataGridEditor saveChanges reload", () => { primaryKeys: ["id"], })), sourceColumns: computed(() => undefined), + joinedWriteTargets: computed(() => options.joinedWriteTargets), onExecuteSql: computed(() => undefined), customSaveHandler: computed(() => options.customSaveHandler), manualTransactionSessionId: computed(() => options.manualTransactionSessionId), @@ -694,6 +698,55 @@ describe("useDataGridEditor saveChanges reload", () => { expect(emit).toHaveBeenCalledWith("reload", undefined, "", undefined, undefined, 100, 0); }); + it("previews and saves edits to both joined tables in a single transaction", async () => { + const refreshSavedRows = vi.fn(); + const targets = [ + { tableMeta: { tableName: "users", primaryKeys: ["id"], columns: [] }, sourceColumns: ["id", "name", undefined, undefined] }, + { tableMeta: { tableName: "papers", primaryKeys: ["id"], columns: [] }, sourceColumns: [undefined, undefined, "id", "title"] }, + ]; + mocks.prepareDataGridSave.mockImplementation(async (options) => ({ statements: ["update " + options.tableMeta.tableName], rollbackStatements: ["undo " + options.tableMeta.tableName] })); + mocks.executeInTransaction.mockResolvedValue({ affected_rows: 2 }); + const { editor, emit } = createSaveTestEditor({ joinedWriteTargets: targets, queryResult: { columns: ["id", "name", "paper_id", "title"], rows: [[1, "old", 20, "old"]] }, refreshSavedRows }); + editor.dirtyRows.value.set( + 0, + new Map([ + [1, "new name"], + [3, "new title"], + ]), + ); + expect(await editor.previewChanges()).toEqual(["update users", "update papers"]); + expect(mocks.executeInTransaction).not.toHaveBeenCalled(); + await editor.saveChanges(); + expect(mocks.executeInTransaction).toHaveBeenCalledWith("connection-1", "app", ["update users", "update papers"], undefined); + expect(mocks.executeBatch).not.toHaveBeenCalled(); + expect(refreshSavedRows).not.toHaveBeenCalled(); + expect(emit).toHaveBeenCalledWith("reload", undefined, "", undefined, undefined, 100, 0); + expect(mocks.prepareDataGridSave.mock.calls[0]![0].dirtyRows).toEqual([[0, [[1, "new name"]]]]); + expect(mocks.prepareDataGridSave.mock.calls[1]![0].dirtyRows).toEqual([[0, [[3, "new title"]]]]); + }); + + it("keeps both joined edits pending if the transaction fails", async () => { + const targets = [ + { tableMeta: { tableName: "users", primaryKeys: ["id"], columns: [] }, sourceColumns: ["id", "name", undefined, undefined] }, + { tableMeta: { tableName: "papers", primaryKeys: ["id"], columns: [] }, sourceColumns: [undefined, undefined, "id", "title"] }, + ]; + mocks.prepareDataGridSave.mockImplementation(async (options) => ({ statements: ["update " + options.tableMeta.tableName], rollbackStatements: [] })); + mocks.executeInTransaction.mockRejectedValue(new Error("second update failed")); + const { editor, emit } = createSaveTestEditor({ joinedWriteTargets: targets, queryResult: { columns: ["id", "name", "paper_id", "title"], rows: [[1, "old", 20, "old"]] } }); + editor.dirtyRows.value.set( + 0, + new Map([ + [1, "new name"], + [3, "new title"], + ]), + ); + await editor.saveChanges(); + expect(editor.dirtyRows.value.get(0)?.size).toBe(2); + expect(editor.saveError.value).toContain("second update failed"); + expect(mocks.executeBatch).not.toHaveBeenCalled(); + expect(emit.mock.calls.some(([event]) => event === "reload")).toBe(false); + }); + it("saves query-result edits through the active manual transaction session", async () => { const statement = "UPDATE orders_test SET status='shipped' WHERE id=1"; const refreshSavedRows = vi.fn().mockResolvedValue(true); diff --git a/apps/desktop/src/composables/useDataGridEditor.ts b/apps/desktop/src/composables/useDataGridEditor.ts index 8659a81fc9..dd7b643c43 100644 --- a/apps/desktop/src/composables/useDataGridEditor.ts +++ b/apps/desktop/src/composables/useDataGridEditor.ts @@ -1,3 +1,4 @@ +import { joinedSaveOptions } from "@/lib/dataGrid/joinedRowChanges"; import { ref, shallowRef, triggerRef, computed, nextTick, watch, getCurrentInstance, onActivated, onBeforeUnmount, onDeactivated, onMounted, toRaw, type ComputedRef, type Ref } from "vue"; import * as api from "@/lib/backend/api"; import type { CellValue } from "@/lib/dataGrid/cellValue"; @@ -99,6 +100,7 @@ export interface UseDataGridEditorOptions { | undefined >; sourceColumns?: ComputedRef | undefined>; + joinedWriteTargets?: ComputedRef; readonlyColumnIndexes?: ComputedRef | undefined>; canEditExistingRows?: ComputedRef; onExecuteSql: ComputedRef<((sql: string) => Promise) | undefined>; @@ -217,6 +219,7 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { database, tableMeta, sourceColumns = computed(() => undefined), + joinedWriteTargets = computed(() => undefined), readonlyColumnIndexes = computed(() => undefined), canEditExistingRows = computed(() => true), onExecuteSql, @@ -666,7 +669,9 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { } } const columnName = sourceColumns.value?.[columnIndex] ?? result.value.columns[columnIndex]; - const info = columnName ? tableMeta.value?.columns.find((column) => column.name.toLowerCase() === columnName.toLowerCase()) : undefined; + const sourceTarget = joinedWriteTargets.value?.find((target) => target.sourceColumns[columnIndex] !== undefined); + const metadata = sourceTarget?.tableMeta ?? tableMeta.value; + const info = columnName ? metadata?.columns.find((column) => column.name.toLowerCase() === columnName.toLowerCase()) : undefined; if (isBatching && batchColumnInfoCache) { batchColumnInfoCache.set(columnIndex, info); } @@ -1494,12 +1499,33 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { }; } + async function prepareSaveStatements(stmtOptions: NonNullable>) { + const targets = joinedWriteTargets.value; + if (!targets?.length) return api.prepareDataGridSave(stmtOptions, saveDriverProfile()); + const groups = joinedSaveOptions(targets, { + ...stmtOptions, + dirtyRows: new Map(stmtOptions.dirtyRows.map(([index, changes]) => [index, new Map(changes)])), + deletedRows: new Set(stmtOptions.deletedRows), + }); + const prepared: Awaited> = { statements: [], rollbackStatements: [] }; + for (const group of groups) { + const part = await api.prepareDataGridSave(group, saveDriverProfile()); + if (part.validationError) return part; + prepared.statements.push(...part.statements); + prepared.rollbackStatements.unshift(...part.rollbackStatements); + if (prepared.executionSchema && part.executionSchema && prepared.executionSchema !== part.executionSchema) throw new Error("Joined source tables require incompatible execution schemas."); + prepared.executionSchema ??= part.executionSchema; + } + return prepared; + } + function saveDriverProfile() { const id = connectionId.value; return id ? connectionStore.getConfig(id)?.driver_profile : undefined; } function tableHistoryTarget() { + if (joinedWriteTargets.value?.length) return [...new Set(joinedWriteTargets.value.map(({ tableMeta: target }) => [target.database, target.schema, target.tableName].filter(Boolean).join(".")))].join(", "); if (!tableMeta.value) return ""; return [tableMeta.value.schema, tableMeta.value.tableName].filter(Boolean).join("."); } @@ -1799,7 +1825,7 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { let preparedSave: Awaited> | undefined; if (stmtOptions) { try { - preparedSave = await api.prepareDataGridSave(stmtOptions, saveDriverProfile()); + preparedSave = await prepareSaveStatements(stmtOptions); } catch (e: any) { saveError.value = normalizeDataGridSaveError(databaseType.value, e); await finishInterruptedSaveChanges(snapshot); @@ -1892,7 +1918,7 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { applyDirtyRowsToResult(snapshot); options.onResultPayloadMutated?.(); let savedRowsRefreshed = false; - if (!manualTransactionSessionId.value && !shouldReloadAfterSave && snapshot.dirtyRows.size > 0 && options.refreshSavedRows) { + if (!joinedWriteTargets.value?.length && !manualTransactionSessionId.value && !shouldReloadAfterSave && snapshot.dirtyRows.size > 0 && options.refreshSavedRows) { try { savedRowsRefreshed = await options.refreshSavedRows({ dirtyRows: snapshot.dirtyRows, @@ -2029,7 +2055,7 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) { } const stmtOptions = saveStatementOptions(); if (!stmtOptions) return []; - const prepared = await api.prepareDataGridSave(stmtOptions, saveDriverProfile()); + const prepared = await prepareSaveStatements(stmtOptions); if (prepared?.validationError) { saveError.value = prepared.validationError; return []; diff --git a/apps/desktop/src/lib/__tests__/dataGrid/joinedQueryAnalysis.spec.ts b/apps/desktop/src/lib/__tests__/dataGrid/joinedQueryAnalysis.spec.ts new file mode 100644 index 0000000000..a15e3a22c3 --- /dev/null +++ b/apps/desktop/src/lib/__tests__/dataGrid/joinedQueryAnalysis.spec.ts @@ -0,0 +1,8 @@ +import { expect, it } from "vitest"; +import { analyzeEditableQueryEditability } from "../../sql/sqlAnalysis"; +it("allows parentheses in JOIN predicates but not derived or function sources", () => { + expect(analyzeEditableQueryEditability("SELECT u.id,u.name,p.id,p.title FROM users u JOIN papers p ON (u.id=p.user_id)").editable).toBe(true); + for (const sql of ["SELECT u.id,p.id FROM users u JOIN (SELECT id FROM papers) p ON u.id=p.id", "SELECT u.id,p.id FROM users u JOIN generate_series(1,2) p ON u.id=p.id"]) { + expect(analyzeEditableQueryEditability(sql).editable).toBe(false); + } +}); diff --git a/apps/desktop/src/lib/__tests__/dataGrid/joinedRowChanges.spec.ts b/apps/desktop/src/lib/__tests__/dataGrid/joinedRowChanges.spec.ts new file mode 100644 index 0000000000..7451c30fae --- /dev/null +++ b/apps/desktop/src/lib/__tests__/dataGrid/joinedRowChanges.spec.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import { joinedRowChanges, type JoinedWriteSource } from "../../dataGrid/joinedRowChanges"; + +const sources: JoinedWriteSource[] = [ + { tableKey: "users", primaryKeys: ["id"], sourceColumns: ["id", "name", undefined, undefined] }, + { tableKey: "papers", primaryKeys: ["id"], sourceColumns: [undefined, undefined, "id", "title"] }, +]; + +describe("joined row updates", () => { + it("routes edits on both tables to their own primary key row", () => { + const updates = joinedRowChanges( + sources, + [[1, "user", 20, "paper"]], + new Map([ + [ + 0, + new Map([ + [1, "renamed"], + [3, "retitled"], + ]), + ], + ]), + ); + expect(updates).toEqual([ + { sourceIndex: 0, rowIndex: 0, changes: new Map([[1, "renamed"]]) }, + { sourceIndex: 1, rowIndex: 0, changes: new Map([[3, "retitled"]]) }, + ]); + }); + + it("coalesces the same user repeated by a one-to-many join", () => { + const rows = [ + [1, "user", 20, "one"], + [1, "user", 21, "two"], + ]; + const updates = joinedRowChanges( + sources, + rows, + new Map([ + [0, new Map([[1, "same"]])], + [1, new Map([[1, "same"]])], + ]), + ); + expect(updates).toHaveLength(1); + expect(() => + joinedRowChanges( + sources, + rows, + new Map([ + [0, new Map([[1, "first"]])], + [1, new Map([[1, "second"]])], + ]), + ), + ).toThrow("Conflicting edits"); + }); + + it("rejects an outer join row without an existing target record", () => { + expect(() => joinedRowChanges(sources, [[1, "user", null, null]], new Map([[0, new Map([[3, "new"]])]]))).toThrow("unmatched outer-join"); + }); + + it("never treats an unbound expression or incomplete key as writable", () => { + expect(() => joinedRowChanges(sources, [[1, "user", 20, "paper", 5]], new Map([[0, new Map([[4, "value"]])]]))).toThrow("unique source"); + expect(() => joinedRowChanges([{ ...sources[0]!, primaryKeys: ["missing"] }], [[1, "user"]], new Map([[0, new Map([[1, "new"]])]]))).toThrow("complete primary key"); + }); + + it("distinguishes composite keys and physical tables", () => { + const composite = [{ tableKey: "users", primaryKeys: ["tenant", "id"], sourceColumns: ["tenant", "id", "name"] }]; + expect( + joinedRowChanges( + composite, + [ + [1, 2, "a"], + [2, 2, "b"], + ], + new Map([ + [0, new Map([[2, "x"]])], + [1, new Map([[2, "y"]])], + ]), + ), + ).toHaveLength(2); + }); +}); diff --git a/apps/desktop/src/lib/dataGrid/joinedRowChanges.ts b/apps/desktop/src/lib/dataGrid/joinedRowChanges.ts new file mode 100644 index 0000000000..84f2d7cff3 --- /dev/null +++ b/apps/desktop/src/lib/dataGrid/joinedRowChanges.ts @@ -0,0 +1,89 @@ +import type { CellValue } from "./cellValue"; +import type { DataGridSaveStatementOptions, DataGridTableMeta } from "./dataGridSql"; + +export interface JoinedSaveTarget { + tableMeta: DataGridTableMeta; + sourceColumns: Array; +} + +/** Feed per-source changes through the existing SQL builder, including undo SQL. */ +export function joinedSaveOptions( + targets: JoinedSaveTarget[], + options: Pick & { + rows: CellValue[][]; + dirtyRows: Map>; + newRows: CellValue[][]; + deletedRows: Set; + }, +): DataGridSaveStatementOptions[] { + if (options.newRows.length || options.deletedRows.size) throw new Error("Joined results support updates only."); + const sources = targets.map(({ tableMeta, sourceColumns }) => ({ + tableKey: JSON.stringify([tableMeta.catalog, tableMeta.database, tableMeta.schema, tableMeta.tableName]), + primaryKeys: tableMeta.primaryKeys, + sourceColumns, + })); + return joinedRowChanges(sources, options.rows, options.dirtyRows).map((update) => ({ + databaseType: options.databaseType, + identifierQuote: options.identifierQuote, + columns: options.columns, + tableMeta: targets[update.sourceIndex]!.tableMeta, + sourceColumns: targets[update.sourceIndex]!.sourceColumns, + rows: options.rows, + dirtyRows: [[update.rowIndex, [...update.changes]]], + newRows: [], + deletedRows: [], + })); +} + +export interface JoinedWriteSource { + /** Physical table identity; aliases of the same table must share this key. */ + tableKey: string; + primaryKeys: string[]; + sourceColumns: Array; +} + +export interface JoinedRowChange { + sourceIndex: number; + rowIndex: number; + changes: Map; +} + +/** Route each edited cell to its physical row, coalescing repeated JOIN rows. */ +export function joinedRowChanges(sources: JoinedWriteSource[], rows: CellValue[][], dirtyRows: Map>): JoinedRowChange[] { + const updates = new Map(); + for (const [rowIndex, changes] of dirtyRows) { + const row = rows[rowIndex]; + if (!row) throw new Error("The edited query row is no longer available."); + for (const [columnIndex, value] of changes) { + const owners = sources.flatMap((source, index) => (source.sourceColumns[columnIndex] === undefined ? [] : [index])); + if (owners.length !== 1) throw new Error("The edited column does not have a unique source table."); + const sourceIndex = owners[0]!; + const source = sources[sourceIndex]!; + if (!source.primaryKeys.length) throw new Error("A joined row requires a complete primary key."); + const keys = source.primaryKeys.map((key) => { + const positions = source.sourceColumns.flatMap((name, index) => (name === key ? [index] : [])); + if (!positions.length) throw new Error("A joined row requires a complete primary key."); + const original = row[positions[0]!]; + if (original === null || original === undefined) throw new Error("Cannot update an unmatched outer-join row."); + if (positions.some((position) => JSON.stringify(row[position]) !== JSON.stringify(original))) throw new Error("Conflicting primary key projections in the query result."); + return original; + }); + const identity = JSON.stringify([source.tableKey, keys]); + let update = updates.get(identity); + if (!update) { + update = { sourceIndex, rowIndex, changes: new Map() }; + updates.set(identity, update); + } + // A self-join can project the same record through different aliases. + const target = sources[update.sourceIndex]!; + const columnName = source.sourceColumns[columnIndex]!; + const targetIndex = target.sourceColumns.indexOf(columnName); + if (targetIndex < 0) throw new Error("Repeated source rows have incompatible column projections."); + if (update.changes.has(targetIndex) && JSON.stringify(update.changes.get(targetIndex)) !== JSON.stringify(value)) { + throw new Error("Conflicting edits refer to the same source row and column."); + } + update.changes.set(targetIndex, value); + } + } + return [...updates.values()]; +} diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index c8a4259d0b..d43a2e9187 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -613,7 +613,7 @@ function isSelectStar(body: string, alias: string | undefined): boolean { } function parseFromSources(body: string): EditableQuerySource[] { - if (!body || /[()]/.test(body)) return []; + if (!body) return []; const sources: EditableQuerySource[] = []; let pos = 0; const first = parseTableSourceAt(body, pos, sources.length); diff --git a/apps/desktop/src/lib/tabs/tabResultCache.ts b/apps/desktop/src/lib/tabs/tabResultCache.ts index e4f8addddd..bde99a867d 100644 --- a/apps/desktop/src/lib/tabs/tabResultCache.ts +++ b/apps/desktop/src/lib/tabs/tabResultCache.ts @@ -34,6 +34,7 @@ export interface TabResultSnapshot { activeResultRunId?: string; queryAnalysis?: QueryTab["queryAnalysis"]; querySourceColumns?: QueryTab["querySourceColumns"]; + queryWriteTargets?: QueryTab["queryWriteTargets"]; resultColumnComments?: QueryTab["resultColumnComments"]; queryDisplaySourceColumns?: QueryTab["queryDisplaySourceColumns"]; queryEditabilityReason?: QueryTab["queryEditabilityReason"]; @@ -748,6 +749,7 @@ export function buildTabResultSnapshot(tab: QueryTab): TabResultSnapshot | undef activeResultRunId: tab.activeResultRunId, queryAnalysis: tab.queryAnalysis ? clonePlain(tab.queryAnalysis) : undefined, querySourceColumns: tab.querySourceColumns ? [...tab.querySourceColumns] : undefined, + queryWriteTargets: tab.queryWriteTargets?.map((target) => ({ ...target, sourceColumns: [...target.sourceColumns] })), resultColumnComments: tab.resultColumnComments ? clonePlain(tab.resultColumnComments) : undefined, queryDisplaySourceColumns: tab.queryDisplaySourceColumns ? [...tab.queryDisplaySourceColumns] : undefined, queryEditabilityReason: tab.queryEditabilityReason, diff --git a/apps/desktop/src/stores/__tests__/queryStore.multiSourceColumnComments.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.multiSourceColumnComments.spec.ts index 2f238532dd..3ab93cf94f 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.multiSourceColumnComments.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.multiSourceColumnComments.spec.ts @@ -128,10 +128,15 @@ describe("queryStore multi-source result column comments", () => { const tab = store.tabs.find((item) => item.id === tabId)!; await vi.waitFor(() => expect(tab.resultColumnComments).toBeDefined()); - // Multi-source results stay non-editable: no single tableMeta. - expect(tab.tableMeta).toBeUndefined(); - expect(tab.queryEditabilityReason).toBe("complex-source"); - expect(tab.querySourceColumns).toBeUndefined(); + // Both sources return their primary key, so each has its own write target. + expect(tab.queryEditabilityReason).toBeUndefined(); + expect(tab.queryAnalysis).toMatchObject({ multiSource: true, allowInsert: false, allowDelete: false }); + expect(tab.queryWriteTargets?.map((target) => target.tableMeta.tableName)).toEqual(["orders", "users"]); + expect(tab.queryWriteTargets?.map((target) => target.tableMeta.primaryKeys)).toEqual([["id"], ["id"]]); + expect(tab.queryWriteTargets?.map((target) => target.sourceColumns)).toEqual([ + ["id", "user_id", undefined, undefined], + [undefined, undefined, "id", "name"], + ]); // Comments are indexed by result ordinal, so the second `id` (users.id) // keeps its own comment instead of first-source-wins on the name. diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 6662c5b1a5..e4fc238e8c 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -1462,6 +1462,7 @@ export const useQueryStore = defineStore("query", () => { tab.resultEstimatedBytes = undefined; tab.queryAnalysis = undefined; tab.querySourceColumns = undefined; + tab.queryWriteTargets = undefined; tab.resultColumnComments = undefined; tab.queryDisplaySourceColumns = undefined; tab.queryEditabilityReason = undefined; @@ -1534,6 +1535,7 @@ export const useQueryStore = defineStore("query", () => { run.resultEstimatedBytes = undefined; run.queryAnalysis = undefined; run.querySourceColumns = undefined; + run.queryWriteTargets = undefined; run.resultColumnComments = undefined; run.queryDisplaySourceColumns = undefined; run.queryEditabilityReason = undefined; @@ -1578,6 +1580,7 @@ export const useQueryStore = defineStore("query", () => { tab.resultEvicted = run.resultEvicted; tab.queryAnalysis = run.queryAnalysis; tab.querySourceColumns = run.querySourceColumns; + tab.queryWriteTargets = run.queryWriteTargets; tab.resultColumnComments = run.resultColumnComments; tab.queryDisplaySourceColumns = run.queryDisplaySourceColumns; tab.queryEditabilityReason = run.queryEditabilityReason; @@ -1818,6 +1821,7 @@ export const useQueryStore = defineStore("query", () => { activeResultRunId: run.id, queryAnalysis: run.queryAnalysis, querySourceColumns: run.querySourceColumns, + queryWriteTargets: run.queryWriteTargets, resultColumnComments: run.resultColumnComments, queryDisplaySourceColumns: run.queryDisplaySourceColumns, queryEditabilityReason: run.queryEditabilityReason, @@ -1899,6 +1903,7 @@ export const useQueryStore = defineStore("query", () => { resultEvicted: tab.resultEvicted, queryAnalysis: tab.queryAnalysis, querySourceColumns: tab.querySourceColumns, + queryWriteTargets: tab.queryWriteTargets, resultColumnComments: tab.resultColumnComments, queryDisplaySourceColumns: tab.queryDisplaySourceColumns, queryEditabilityReason: tab.queryEditabilityReason, @@ -1999,6 +2004,7 @@ export const useQueryStore = defineStore("query", () => { resultEvicted: tab.resultEvicted, queryAnalysis: tab.queryAnalysis, querySourceColumns: tab.querySourceColumns, + queryWriteTargets: tab.queryWriteTargets, resultColumnComments: tab.resultColumnComments, queryDisplaySourceColumns: tab.queryDisplaySourceColumns, queryEditabilityReason: tab.queryEditabilityReason, @@ -3699,6 +3705,7 @@ export const useQueryStore = defineStore("query", () => { tableMeta: original.tableMeta ? { ...original.tableMeta, columns: [...original.tableMeta.columns], primaryKeys: [...original.tableMeta.primaryKeys] } : undefined, queryAnalysis: original.queryAnalysis ? { ...original.queryAnalysis, sources: original.queryAnalysis.sources?.map((source) => ({ ...source })), columns: original.queryAnalysis.columns.map((c) => ({ ...c })) } : undefined, querySourceColumns: original.querySourceColumns ? [...original.querySourceColumns] : undefined, + queryWriteTargets: original.queryWriteTargets?.map((target) => ({ ...target, sourceColumns: [...target.sourceColumns] })), resultColumnComments: original.resultColumnComments ? [...original.resultColumnComments] : undefined, queryDisplaySourceColumns: original.queryDisplaySourceColumns ? [...original.queryDisplaySourceColumns] : undefined, queryEditabilityReason: original.queryEditabilityReason, @@ -4569,7 +4576,7 @@ export const useQueryStore = defineStore("query", () => { return producedResult; } - type QueryMetadataPatch = Pick; + type QueryMetadataPatch = Pick; type LoadedEditableSource = { source: EditableQuerySource; @@ -4658,6 +4665,7 @@ export const useQueryStore = defineStore("query", () => { function applyQueryMetadataPatch(tab: QueryTab, patch: QueryMetadataPatch) { tab.queryAnalysis = patch.queryAnalysis; tab.querySourceColumns = patch.querySourceColumns; + tab.queryWriteTargets = patch.queryWriteTargets; tab.queryEditabilityReason = patch.queryEditabilityReason; tab.mongoEditTarget = undefined; tab.tableMeta = patch.tableMeta; @@ -5173,11 +5181,16 @@ export const useQueryStore = defineStore("query", () => { } if (candidates.length > 1) { + const target = candidates[0]!; return { - queryAnalysis: undefined, - querySourceColumns: undefined, - queryEditabilityReason: "complex-source", - tableMeta: undefined, + queryAnalysis: { ...target.analysis, multiSource: true, allowInsert: false, allowDelete: false, allowInsertDelete: false }, + querySourceColumns: tab.result.columns.map((_, index) => { + const owners = candidates.filter((candidate) => candidate.sourceColumns?.[index] !== undefined); + return owners.length === 1 ? owners[0]!.sourceColumns![index] : undefined; + }), + queryWriteTargets: candidates.map((candidate) => ({ tableMeta: candidate.tableMeta, sourceColumns: candidate.sourceColumns! })), + queryEditabilityReason: undefined, + tableMeta: target.tableMeta, resultColumnComments: multiSourceInfo?.comments, queryDisplaySourceColumns: multiSourceInfo?.mapping, }; @@ -5614,6 +5627,7 @@ export const useQueryStore = defineStore("query", () => { touchResult(current); current.queryAnalysis = undefined; current.querySourceColumns = undefined; + current.queryWriteTargets = undefined; current.resultColumnComments = undefined; current.queryDisplaySourceColumns = undefined; current.queryEditabilityReason = undefined; @@ -5998,6 +6012,7 @@ export const useQueryStore = defineStore("query", () => { touchResult(current); current.queryAnalysis = undefined; current.querySourceColumns = undefined; + current.queryWriteTargets = undefined; current.resultColumnComments = undefined; current.queryDisplaySourceColumns = undefined; current.queryEditabilityReason = undefined; @@ -6065,6 +6080,7 @@ export const useQueryStore = defineStore("query", () => { touchResult(current); current.queryAnalysis = undefined; current.querySourceColumns = undefined; + current.queryWriteTargets = undefined; current.resultColumnComments = undefined; current.queryDisplaySourceColumns = undefined; current.queryEditabilityReason = undefined; @@ -6559,6 +6575,7 @@ export const useQueryStore = defineStore("query", () => { } current.queryAnalysis = undefined; current.querySourceColumns = undefined; + current.queryWriteTargets = undefined; current.resultColumnComments = undefined; current.queryDisplaySourceColumns = undefined; current.queryEditabilityReason = undefined; @@ -7076,6 +7093,7 @@ export const useQueryStore = defineStore("query", () => { touchResult(tab, Date.now(), { reuseEstimatedBytes: true }); tab.queryAnalysis = undefined; tab.querySourceColumns = undefined; + tab.queryWriteTargets = undefined; tab.resultColumnComments = undefined; tab.queryDisplaySourceColumns = undefined; tab.queryEditabilityReason = undefined; @@ -7202,6 +7220,7 @@ export const useQueryStore = defineStore("query", () => { tab.queryAnalysis = snapshot.queryAnalysis; tab.querySourceColumns = snapshot.querySourceColumns; + tab.queryWriteTargets = snapshot.queryWriteTargets; tab.resultColumnComments = snapshot.resultColumnComments; tab.queryDisplaySourceColumns = snapshot.queryDisplaySourceColumns; tab.queryEditabilityReason = snapshot.queryEditabilityReason; diff --git a/apps/desktop/src/types/database.ts b/apps/desktop/src/types/database.ts index e2483c042a..364028115d 100644 --- a/apps/desktop/src/types/database.ts +++ b/apps/desktop/src/types/database.ts @@ -831,6 +831,7 @@ export interface QueryResultRun { resultEvicted?: boolean; queryAnalysis?: QueryTab["queryAnalysis"]; querySourceColumns?: QueryTab["querySourceColumns"]; + queryWriteTargets?: QueryTab["queryWriteTargets"]; resultColumnComments?: QueryTab["resultColumnComments"]; queryDisplaySourceColumns?: QueryTab["queryDisplaySourceColumns"]; queryEditabilityReason?: QueryTab["queryEditabilityReason"]; @@ -1370,6 +1371,7 @@ export interface QueryTab { }[]; }; querySourceColumns?: Array; + queryWriteTargets?: Array<{ tableMeta: NonNullable; sourceColumns: Array }>; /** * Column comments for a multi-source query result (e.g. JOIN), indexed by * result-column ordinal (projection order). Each entry is the comment of the diff --git a/crates/dbx-core/src/db/postgres.rs b/crates/dbx-core/src/db/postgres.rs index 9e447c079d..5b4862f877 100644 --- a/crates/dbx-core/src/db/postgres.rs +++ b/crates/dbx-core/src/db/postgres.rs @@ -10323,7 +10323,9 @@ mod tests { // guarded ordinal so composite keys stay aligned. for compat_sql in [postgres_foreign_keys_compat_sql(), postgres_foreign_keys_for_relations_compat_sql()] { assert!(!compat_sql.contains("generate_series(1, array_length")); - assert!(compat_sql.contains("JOIN generate_series(1, 32) AS fk(ord) ON fk.ord <= array_length(con.conkey, 1)")); + assert!( + compat_sql.contains("JOIN generate_series(1, 32) AS fk(ord) ON fk.ord <= array_length(con.conkey, 1)") + ); assert!(compat_sql.contains("(con.conkey)[fk.ord]")); assert!(compat_sql.contains("(con.confkey)[fk.ord]")); assert!(!compat_sql.contains("LATERAL")); diff --git a/crates/dbx-core/src/query_result_sql.rs b/crates/dbx-core/src/query_result_sql.rs index 20f07d6c45..63bc1d6e86 100644 --- a/crates/dbx-core/src/query_result_sql.rs +++ b/crates/dbx-core/src/query_result_sql.rs @@ -2360,7 +2360,10 @@ mod tests { let page_sql = plan.page_sql.expect("pagination plan provides page SQL"); assert!(page_sql.to_uppercase().contains("GROUP BY")); - assert!(page_sql.contains("ORDER BY 1, 2, 3 LIMIT 500;"), "dedup pagination must be appended after the GROUP BY, got: {page_sql}"); + assert!( + page_sql.contains("ORDER BY 1, 2, 3 LIMIT 500;"), + "dedup pagination must be appended after the GROUP BY, got: {page_sql}" + ); let count_sql = plan.count_sql.expect("pagination plan provides count SQL"); assert!(count_sql.starts_with("SELECT COUNT(*) AS dbx_total_rows FROM (")); diff --git a/crates/dbx-core/src/sql_editability.rs b/crates/dbx-core/src/sql_editability.rs index 0e487520c8..7b65077744 100644 --- a/crates/dbx-core/src/sql_editability.rs +++ b/crates/dbx-core/src/sql_editability.rs @@ -694,7 +694,7 @@ fn is_select_star(body: &str, alias: Option<&str>) -> bool { } fn parse_from_sources(body: &str) -> Vec { - if body.is_empty() || body.contains('(') || body.contains(')') { + if body.is_empty() { return Vec::new(); } @@ -742,6 +742,9 @@ fn parse_table_source_at(text: &str, start: usize, index: usize) -> Option<(From } let mut end = pos + ident.end; let tail_pos = skip_whitespace(text, end); + if text[tail_pos..].starts_with('(') { + return None; + } let alias = if starts_with_keyword_at(text, tail_pos, "AS") { let alias_ident = read_identifier(text, tail_pos + 2)?; end = alias_ident.end; @@ -1585,3 +1588,25 @@ mod tests { } } } + +#[cfg(test)] +mod joined_predicate_regression { + use super::analyze_editable_query_editability; + + #[test] + fn parenthesized_join_predicate_preserves_both_sources() { + let result = analyze_editable_query_editability("SELECT u.id,u.nickname,p.paper_id,p.paper_name FROM users u JOIN papers p ON (u.id=p.user_id) WHERE u.id=1"); + assert!(result.editable); + assert_eq!(result.analysis.unwrap().sources.unwrap().len(), 2); + } + + #[test] + fn joined_derived_and_function_sources_remain_read_only() { + for sql in [ + "SELECT u.id,p.id FROM users u JOIN (SELECT id FROM papers) p ON u.id=p.id", + "SELECT u.id,p.id FROM users u JOIN generate_series(1,2) p ON u.id=p.id", + ] { + assert!(!analyze_editable_query_editability(sql).editable, "{sql}"); + } + } +} diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index ad7d3f271a..ff40b6ba88 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -3624,7 +3624,7 @@ test("expands single-table alias star projections for editable query metadata", } }); -test("keeps joined query read-only when multiple source tables are writable candidates", async () => { +test("keeps per-table write targets when multiple source tables are writable candidates", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); const connectionStore = useConnectionStore(); @@ -3711,10 +3711,15 @@ test("keeps joined query read-only when multiple source tables are writable cand await store.executeTabSql(tabId, sql); const tab = store.tabs.find((item) => item.id === tabId); - await waitFor(() => columnRequests.length === 2 && tab?.queryEditabilityReason === "complex-source"); - assert.equal(tab?.queryAnalysis, undefined); - assert.equal(tab?.tableMeta, undefined); - assert.equal(tab?.querySourceColumns, undefined); + await waitFor(() => columnRequests.length === 2 && tab?.queryWriteTargets?.length === 2); + assert.equal(tab?.queryEditabilityReason, undefined); + assert.equal(tab?.queryAnalysis?.allowDelete, false); + assert.equal(tab?.queryAnalysis?.allowInsert, false); + assert.deepEqual( + tab?.queryWriteTargets?.map((target) => target.tableMeta.tableName), + ["users", "orders"], + ); + assert.deepEqual(tab?.querySourceColumns, ["id", "name", "id", "total"]); } finally { globalThis.fetch = originalFetch; restoreStorage();