From d79dc05cf53f5f097d3044834e1f22ce3edc2dd4 Mon Sep 17 00:00:00 2001 From: onenewcode Date: Tue, 15 Sep 2026 14:51:44 +0800 Subject: [PATCH] fix(query): keep Oracle NUM column results editable --- .../sql/editableQueryHiddenKeys.spec.ts | 14 +++++ apps/desktop/src/lib/sql/sqlAnalysis.ts | 58 ++++++++++++++++--- apps/desktop/src/stores/queryStore.ts | 2 +- crates/dbx-core/src/sql_editability.rs | 12 ++++ packages/app-tests/sqlAnalysis.test.ts | 47 +++++++++++++++ 5 files changed, 124 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts index 772ec5685c..daed7b4c5a 100644 --- a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts +++ b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts @@ -216,6 +216,20 @@ describe("editable query hidden primary keys", () => { expect(analyzeEditableQueryEditability("select id from jobs union select id from archived_jobs")).toEqual({ editable: false, reason: "set-operation" }); }); + it("keeps Oracle NUM columns and aliases mapped when result labels disagree", () => { + const named = analyzeEditableQueryEditability("select id, num from users"); + expect(named.editable).toBe(true); + if (!named.editable) return; + expect(sourceColumnsForResult(named.analysis, ["ID", "NUM"], undefined, "oracle")).toEqual(["id", "num"]); + expect(sourceColumnsForResult(named.analysis, ["ID", "ROWNUM"], undefined, "oracle")).toEqual(["id", undefined]); + + const aliased = analyzeEditableQueryEditability("select id, amount as num from users"); + expect(aliased.editable).toBe(true); + if (!aliased.editable) return; + expect(sourceColumnsForResult(aliased.analysis, ["ID", "AMOUNT"], undefined, "oracle")).toEqual(["id", "amount"]); + expect(sourceColumnsForResult(aliased.analysis, ["ID", "NUM", "ROWNUM"], undefined, "oracle")).toEqual(["id", "amount", undefined]); + }); + it("maps a qualified Oracle ROWID projection to the synthetic row key", () => { const analysis: EditableQueryInfo = { schema: "APP", diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index fe5f35343d..4f3030f12b 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -861,13 +861,32 @@ export function allPrimaryKeysPresent(primaryKeys: string[], resultColumns: stri return primaryKeys.every((pk) => colSet.has(pk)); } -function matchColumnsForResult(analysis: EditableQueryInfo, resultColumns: string[]): EditableQueryColumn[] | undefined { - // Preserve projection order before searching by label: folded duplicate names are not unique. - if (analysis.columns.length === resultColumns.length && analysis.columns.every((column, index) => column.resultName.toLowerCase() === resultColumns[index]!.toLowerCase())) return analysis.columns; +const SYNTHETIC_RESULT_ROW_NUMBER_LABELS = new Set(["__dbx_row_num", "dbx_rn"]); +function isOracleFamilyDatabase(databaseType?: DatabaseType | string): boolean { + return !!databaseType && ORACLE_FOLDED_IDENTIFIER_TYPES.has(databaseType); +} + +function isTrailingPaginationResultLabel(label: string, analysisResultNames: Set): boolean { + const normalized = label.toLowerCase(); + if (SYNTHETIC_RESULT_ROW_NUMBER_LABELS.has(normalized)) return true; + return normalized === "rownum" && !analysisResultNames.has("rownum"); +} + +function resultPrefixLengthForEditMatching(analysis: EditableQueryInfo, resultColumns: string[]): number { + const analysisNames = new Set(analysis.columns.map((column) => column.resultName.toLowerCase())); + let end = resultColumns.length; + while (end > analysis.columns.length) { + if (!isTrailingPaginationResultLabel(resultColumns[end - 1]!, analysisNames)) break; + end -= 1; + } + return end; +} + +function matchResultPrefixByLabel(analysis: EditableQueryInfo, prefix: string[]): EditableQueryColumn[] | undefined { const matches: EditableQueryColumn[] = []; let searchFrom = 0; - for (const resultColumn of resultColumns) { + for (const resultColumn of prefix) { let matchIndex = analysis.columns.findIndex((column, index) => index >= searchFrom && column.resultName === resultColumn); if (matchIndex < 0) { const normalized = resultColumn.toLowerCase(); @@ -881,17 +900,40 @@ function matchColumnsForResult(analysis: EditableQueryInfo, resultColumns: strin return matches; } -export function allEditableColumnsWriteable(analysis: EditableQueryInfo, resultColumns: string[], sourceKey?: string): boolean { +function isRownumLabelSubstitution(resultLabel: string, column: EditableQueryColumn): boolean { + if (resultLabel.toLowerCase() !== "rownum") return false; + return column.resultName.toLowerCase() !== "rownum" && column.sourceName?.toLowerCase() !== "rownum"; +} + +function padMatchedColumnsToResultLength(matches: Array, resultColumnCount: number): Array { + if (matches.length === resultColumnCount) return matches; + return [...matches, ...Array.from({ length: resultColumnCount - matches.length }, () => undefined)]; +} + +function matchColumnsForResult(analysis: EditableQueryInfo, resultColumns: string[], databaseType?: DatabaseType | string): Array | undefined { + const prefixLength = resultPrefixLengthForEditMatching(analysis, resultColumns); + const prefix = resultColumns.slice(0, prefixLength); + const labeled = matchResultPrefixByLabel(analysis, prefix); + if (labeled) return padMatchedColumnsToResultLength(labeled, resultColumns.length); + + if (prefix.length !== analysis.columns.length || !isOracleFamilyDatabase(databaseType)) return undefined; + + const ordinal = analysis.columns.map((column, index) => (isRownumLabelSubstitution(prefix[index]!, column) ? undefined : column)); + return padMatchedColumnsToResultLength(ordinal, resultColumns.length); +} + +export function allEditableColumnsWriteable(analysis: EditableQueryInfo, resultColumns: string[], sourceKey?: string, databaseType?: DatabaseType | string): boolean { if (analysis.selectStar) return true; - const matchedColumns = matchColumnsForResult(analysis, resultColumns); - return !!matchedColumns && matchedColumns.every((source) => !sourceKey || !source.sourceName || source.sourceKey === sourceKey); + const matchedColumns = matchColumnsForResult(analysis, resultColumns, databaseType); + return !!matchedColumns && matchedColumns.every((source) => !source || !sourceKey || !source.sourceName || source.sourceKey === sourceKey); } export function sourceColumnsForResult(analysis: EditableQueryInfo, resultColumns: string[], sourceKey?: string, databaseType?: DatabaseType, primaryKeys?: readonly string[]): Array | undefined { if (analysis.selectStar) return undefined; - const matchedColumns = matchColumnsForResult(analysis, resultColumns); + const matchedColumns = matchColumnsForResult(analysis, resultColumns, databaseType); if (!matchedColumns) return undefined; return matchedColumns.map((column) => { + if (!column) return undefined; if (sourceKey && column.sourceKey !== sourceKey) return undefined; if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) { return primaryKeys?.length === 1 && primaryKeys[0] === DBX_ROWID_COLUMN ? DBX_ROWID_COLUMN : undefined; diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index e557f137fb..1d17d89a88 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -5764,7 +5764,7 @@ export const useQueryStore = defineStore("query", () => { }; } - if (!allEditableColumnsWriteable(metadataAnalysis, tab.result.columns)) { + if (!allEditableColumnsWriteable(metadataAnalysis, tab.result.columns, undefined, dbType)) { return { queryAnalysis: undefined, querySourceColumns: undefined, diff --git a/crates/dbx-core/src/sql_editability.rs b/crates/dbx-core/src/sql_editability.rs index da2f05c593..d384a29cd3 100644 --- a/crates/dbx-core/src/sql_editability.rs +++ b/crates/dbx-core/src/sql_editability.rs @@ -1103,6 +1103,18 @@ mod tests { ); } + #[test] + fn recognizes_oracle_num_column_and_alias_as_editable() { + for sql in ["select id, num from users", "select id, amount as num from users"] { + let result = analyze_editable_query_editability(sql); + assert!(result.editable, "{sql}"); + assert_eq!(result.reason, None, "{sql}"); + let analysis = result.analysis.unwrap(); + assert_eq!(analysis.table_name, "users", "{sql}"); + assert_eq!(analysis.columns.last().unwrap().result_name, "num", "{sql}"); + } + } + #[test] fn recognizes_sql_server_top_selects_as_editable() { for sql in [ diff --git a/packages/app-tests/sqlAnalysis.test.ts b/packages/app-tests/sqlAnalysis.test.ts index 0feca9a8f9..b240c0bc93 100644 --- a/packages/app-tests/sqlAnalysis.test.ts +++ b/packages/app-tests/sqlAnalysis.test.ts @@ -342,6 +342,53 @@ test("rejects ambiguous case-only result column mapping", () => { assert.equal(sourceColumnsForResult(analysis, ["Id"]), undefined); }); +test("does not bind non-Oracle same-arity label mismatches by projection order", () => { + const analysis = analyzeEditableQuery("select id, amount as num from users"); + assert.ok(analysis); + assert.equal(sourceColumnsForResult(analysis, ["id", "amount"]), undefined); + assert.equal(allEditableColumnsWriteable(analysis, ["id", "amount"]), false); + assert.equal(sourceColumnsForResult(analysis, ["id", "amount"], undefined, "mysql"), undefined); + assert.equal(allEditableColumnsWriteable(analysis, ["id", "amount"], undefined, "mysql"), false); +}); + +test("keeps Oracle queries with a NUM column or alias editable when result labels disagree", () => { + const byName = analyzeEditableQuery("select id, num from users"); + assert.ok(byName); + assert.deepEqual( + byName.columns, + [ + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { sourceName: "num", sourceNameQuoted: false, resultName: "num", expression: "num" }, + ], + ); + assert.deepEqual(sourceColumnsForResult(byName, ["ID", "NUM"], undefined, "oracle"), ["id", "num"]); + assert.equal(allEditableColumnsWriteable(byName, ["ID", "NUM"], undefined, "oracle"), true); + assert.deepEqual(sourceColumnsForResult(byName, ["ID", "ROWNUM"], undefined, "oracle"), ["id", undefined]); + assert.equal(sourceColumnsForResult(byName, ["ID", "ROWNUM"], undefined, "oracle")?.[1], undefined); + + const byAlias = analyzeEditableQuery("select id, amount as num from users"); + assert.ok(byAlias); + assert.deepEqual(sourceColumnsForResult(byAlias, ["ID", "AMOUNT"], undefined, "oracle"), ["id", "amount"]); + assert.equal(allEditableColumnsWriteable(byAlias, ["ID", "AMOUNT"], undefined, "oracle"), true); + assert.deepEqual(sourceColumnsForResult(byAlias, ["ID", "NUM", "ROWNUM"], undefined, "oracle"), ["id", "amount", undefined]); + assert.equal(allEditableColumnsWriteable(byAlias, ["ID", "NUM", "ROWNUM"], undefined, "oracle"), true); + assert.deepEqual(sourceColumnsForResult(byAlias, ["ID", "AMOUNT", "__dbx_row_num"], undefined, "oracle"), ["id", "amount", undefined]); + assert.deepEqual(sourceColumnsForResult(byAlias, ["ID", "AMOUNT", "dbx_rn"], undefined, "oracle"), ["id", "amount", undefined]); + assert.equal(allEditableColumnsWriteable(byAlias, ["ID", "AMOUNT", "__dbx_row_num"], undefined, "oracle"), true); +}); + +test("tolerates trailing pagination labels for non-Oracle engines", () => { + const analysis = analyzeEditableQuery("select id, amount as num from users"); + assert.ok(analysis); + assert.deepEqual(sourceColumnsForResult(analysis, ["id", "num", "__dbx_row_num"], undefined, "mysql"), ["id", "amount", undefined]); + assert.equal(allEditableColumnsWriteable(analysis, ["id", "num", "__dbx_row_num"], undefined, "mysql"), true); + assert.deepEqual(sourceColumnsForResult(analysis, ["id", "num", "dbx_rn"], undefined, "mysql"), ["id", "amount", undefined]); + assert.equal(allEditableColumnsWriteable(analysis, ["id", "num", "dbx_rn"], undefined, "mysql"), true); + assert.deepEqual(sourceColumnsForResult(analysis, ["id", "num", "ROWNUM"], undefined, "mysql"), ["id", "amount", undefined]); + assert.equal(allEditableColumnsWriteable(analysis, ["id", "num", "ROWNUM"], undefined, "mysql"), true); + assert.deepEqual(sourceColumnsForResult(analysis, ["id", "num", "rownum"], undefined, "mysql"), ["id", "amount", undefined]); +}); + test("maps ClickHouse simple query results when identifier columns are returned", () => { const analysis = analyzeEditableQuery("SELECT id, name, score + 1 AS next_score FROM default.people");