From cb6350bef60e0face8e7c5f3675690cc8a4c22e7 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Fri, 10 Jul 2026 13:19:28 +0800 Subject: [PATCH] fix(query): support editing distinct results --- apps/desktop/src/lib/sql/sqlAnalysis.ts | 16 +++- apps/desktop/src/stores/queryStore.ts | 2 +- crates/dbx-core/src/sql_editability.rs | 119 +++++++++++++----------- packages/app-tests/queryStore.test.ts | 20 ++-- packages/app-tests/sqlAnalysis.test.ts | 33 +++++++ 5 files changed, 119 insertions(+), 71 deletions(-) diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index ac4e963c79..23dd305fc9 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -23,6 +23,7 @@ export interface EditableQueryInfo { editableSourceKey?: string; multiSource?: boolean; allowInsertDelete?: boolean; + distinct?: boolean; } export interface EditableQueryColumn { @@ -80,8 +81,12 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability { const fromIndex = findTopLevelKeyword(normalized, "FROM", 0); if (fromIndex < 0) return { editable: false, reason: "no-table" }; - const selectBody = normalized.slice("SELECT".length, fromIndex).trim(); - if (/^DISTINCT\b/i.test(selectBody)) return { editable: false, reason: "aggregation" }; + const rawSelectBody = normalized.slice("SELECT".length, fromIndex).trim(); + const distinct = /^DISTINCT\b/i.test(rawSelectBody); + const selectBody = distinct ? rawSelectBody.replace(/^DISTINCT\b/i, "").trimStart() : rawSelectBody; + // DISTINCT ON selects a representative row using database-specific ordering, + // so it cannot be mapped to base rows like an ordinary DISTINCT projection. + if (distinct && /^ON\b/i.test(selectBody)) return { editable: false, reason: "aggregation" }; const groupIndex = findTopLevelKeyword(normalized, "GROUP", fromIndex + 4); const havingIndex = findTopLevelKeyword(normalized, "HAVING", fromIndex + 4); @@ -95,11 +100,11 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability { const source = sources[0]!; const selectStar = sources.length === 1 && isSelectStar(selectBody, source.alias); - if (sources.length > 1 && /(^|,)\s*(?:\*|(?:[A-Za-z_][\w$]*|"[^"]+"|`[^`]+`|\[[^\]]+\])\s*\.\s*\*)\s*(?:,|$)/.test(selectBody)) { - return { editable: false, reason: "complex-source" }; - } const columns = selectStar ? [] : parseSelectColumns(selectBody, sources); if (!selectStar && columns.length === 0) return { editable: false, reason: "computed-columns" }; + if (sources.length > 1 && columns.some((column) => column.star && !column.sourceKey)) { + return { editable: false, reason: "complex-source" }; + } const analysis: EditableQueryInfo = { catalog: source.catalog, @@ -111,6 +116,7 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability { tableAlias: source.alias, selectStar, columns, + ...(distinct ? { distinct: true, allowInsertDelete: false } : {}), }; if (sources.length > 1) { analysis.sources = sources; diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index dac69e456a..30c03cfef2 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -223,7 +223,7 @@ function cloneAnalysisForSource(analysis: EditableQueryInfo, source: EditableQue tableNameQuoted: source.tableNameQuoted, tableAlias: source.alias, editableSourceKey: source.key, - allowInsertDelete: analysis.sources?.length ? false : analysis.allowInsertDelete, + allowInsertDelete: analysis.sources?.length || analysis.distinct ? false : analysis.allowInsertDelete, }; } diff --git a/crates/dbx-core/src/sql_editability.rs b/crates/dbx-core/src/sql_editability.rs index 6a152df9a6..6a373e3fb7 100644 --- a/crates/dbx-core/src/sql_editability.rs +++ b/crates/dbx-core/src/sql_editability.rs @@ -24,6 +24,8 @@ pub struct EditableQueryInfo { pub multi_source: bool, #[serde(default, skip_serializing_if = "Option::is_none")] pub allow_insert_delete: Option, + #[serde(default, skip_serializing_if = "is_false")] + pub distinct: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -146,10 +148,18 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability { return not_editable(QueryEditabilityReason::NoTable); }; - let select_body = normalized["SELECT".len()..from_index].trim(); - if starts_with_keyword(select_body, "DISTINCT") { - return not_editable(QueryEditabilityReason::Aggregation); - } + let raw_select_body = normalized["SELECT".len()..from_index].trim(); + let (select_body, distinct) = if starts_with_keyword(raw_select_body, "DISTINCT") { + let body = raw_select_body["DISTINCT".len()..].trim_start(); + // DISTINCT ON has database-specific row-selection semantics and cannot be + // treated as a plain projection whose rows map directly to base records. + if starts_with_keyword(body, "ON") { + return not_editable(QueryEditabilityReason::Aggregation); + } + (body, true) + } else { + (raw_select_body, false) + }; let group_index = find_top_level_keyword(&normalized, "GROUP", from_index + "FROM".len()); let having_index = find_top_level_keyword(&normalized, "HAVING", from_index + "FROM".len()); @@ -172,13 +182,13 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability { let source = sources[0].clone(); let select_star = sources.len() == 1 && is_select_star(select_body, source.alias.as_deref()); - if sources.len() > 1 && select_body_contains_star_projection(select_body) { - return not_editable(QueryEditabilityReason::ComplexSource); - } let columns = if select_star { Vec::new() } else { parse_select_columns(select_body, &sources) }; if !select_star && columns.is_empty() { return not_editable(QueryEditabilityReason::ComputedColumns); } + if sources.len() > 1 && columns.iter().any(|column| column.star && column.source_key.is_none()) { + return not_editable(QueryEditabilityReason::ComplexSource); + } let mut analysis = EditableQueryInfo { catalog: source.catalog, @@ -193,7 +203,10 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability { sources: None, editable_source_key: None, multi_source: false, - allow_insert_delete: None, + // Updating an identified base row is safe, but insert/delete semantics are + // ambiguous when the displayed set is de-duplicated by the query. + allow_insert_delete: distinct.then_some(false), + distinct, }; if sources.len() > 1 { analysis.sources = Some(sources.into_iter().map(EditableQuerySource::from).collect()); @@ -491,52 +504,6 @@ fn is_external_from_source(body: &str) -> bool { is_single_quoted_source_with_optional_alias(trimmed) || starts_with_table_function(trimmed) } -fn select_body_contains_star_projection(body: &str) -> bool { - split_top_level_commas(body).iter().any(|item| { - let trimmed = item.trim(); - if trimmed == "*" { - return true; - } - let Some((_, suffix)) = trimmed.split_once('.') else { - return false; - }; - suffix.trim() == "*" - }) -} - -fn split_top_level_commas(body: &str) -> Vec { - let mut items = Vec::new(); - let mut depth = 0i32; - let mut current = String::new(); - let mut quote: Option = None; - for ch in body.chars() { - if let Some(close) = quote { - current.push(ch); - if ch == close { - quote = None; - } - continue; - } - match ch { - '\'' | '"' | '`' => quote = Some(ch), - '[' => quote = Some(']'), - '(' => depth += 1, - ')' => depth = 0.max(depth - 1), - ',' if depth == 0 => { - items.push(current); - current = String::new(); - continue; - } - _ => {} - } - current.push(ch); - } - if !current.is_empty() { - items.push(current); - } - items -} - fn table_source_terminator_at(text: &str, pos: usize) -> bool { let pos = skip_whitespace(text, pos); if pos >= text.len() || text[pos..].starts_with(',') { @@ -855,6 +822,50 @@ mod tests { ); } + #[test] + fn recognizes_distinct_single_table_projection_as_update_only() { + let result = analyze_editable_query_editability("select distinct id, name from users"); + + assert!(result.editable); + let analysis = result.analysis.unwrap(); + assert!(analysis.distinct); + assert_eq!(analysis.allow_insert_delete, Some(false)); + assert_eq!(analysis.columns.len(), 2); + } + + #[test] + fn recognizes_distinct_qualified_star_from_join() { + let result = analyze_editable_query_editability( + "select distinct u.* from users u left join orders o on o.user_id = u.id", + ); + + assert!(result.editable); + let analysis = result.analysis.unwrap(); + assert!(analysis.distinct); + assert!(analysis.multi_source); + assert_eq!(analysis.allow_insert_delete, Some(false)); + assert_eq!(analysis.columns, vec![star_column(Some("u"), Some("u:0"), "u.*")]); + } + + #[test] + fn rejects_unqualified_star_from_join() { + let result = + analyze_editable_query_editability("select distinct * from users u join orders o on o.user_id = u.id"); + + assert!(!result.editable); + assert_eq!(result.reason, Some(QueryEditabilityReason::ComplexSource)); + } + + #[test] + fn rejects_distinct_on_projection() { + let result = analyze_editable_query_editability( + "select distinct on (user_id) id, user_id from orders order by user_id, id desc", + ); + + assert!(!result.editable); + assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation)); + } + #[test] fn maps_joined_query_source_columns() { let result = analyze_editable_query_editability( diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index 2499cc092c..e20de12531 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -1229,7 +1229,7 @@ test("normalizes unquoted Oracle query identifiers before loading editable metad } }); -test("binds joined query edits to the single safe source table", async () => { +test("binds DISTINCT qualified-star edits to the single safe joined source", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); const connectionStore = useConnectionStore(); @@ -1245,8 +1245,8 @@ test("binds joined query edits to the single safe source table", async () => { return new Response( JSON.stringify([ { - columns: ["user_id", "name", "total"], - rows: [[1, "Ada", 42]], + columns: ["id", "name"], + rows: [[1, "Ada"]], affected_rows: 0, execution_time_ms: 1, }, @@ -1256,7 +1256,7 @@ test("binds joined query edits to the single safe source table", async () => { } if (url === "/api/query/analyze-editability") { const body = JSON.parse(String(init?.body ?? "{}")); - assert.equal(body.sql, "select u.id as user_id, u.name, o.total from users u join orders o on o.user_id = u.id"); + assert.equal(body.sql, "select distinct u.* from users u join orders o on o.user_id = u.id"); return new Response( JSON.stringify({ editable: true, @@ -1267,17 +1267,14 @@ test("binds joined query edits to the single safe source table", async () => { tableNameQuoted: false, tableAlias: "u", selectStar: false, + distinct: true, multiSource: true, allowInsertDelete: false, sources: [ { key: "u:0", tableName: "users", tableNameQuoted: false, schemaQuoted: false, alias: "u" }, { key: "o:1", tableName: "orders", tableNameQuoted: false, schemaQuoted: false, alias: "o" }, ], - columns: [ - { sourceName: "id", sourceNameQuoted: false, sourceQualifier: "u", sourceKey: "u:0", resultName: "user_id", expression: "u.id" }, - { sourceName: "name", sourceNameQuoted: false, sourceQualifier: "u", sourceKey: "u:0", resultName: "name", expression: "u.name" }, - { sourceName: "total", sourceNameQuoted: false, sourceQualifier: "o", sourceKey: "o:1", resultName: "total", expression: "o.total" }, - ], + columns: [{ star: true, sourceQualifier: "u", sourceKey: "u:0", resultName: "*", expression: "u.*" }], }, }), { status: 200, headers: { "Content-Type": "application/json" } }, @@ -1311,7 +1308,7 @@ test("binds joined query edits to the single safe source table", async () => { }); try { - const sql = "select u.id as user_id, u.name, o.total from users u join orders o on o.user_id = u.id"; + const sql = "select distinct u.* from users u join orders o on o.user_id = u.id"; const tabId = store.createTab("pg-join-1", "appdb", "Query 1", "query"); await store.executeTabSql(tabId, sql); @@ -1323,9 +1320,10 @@ test("binds joined query edits to the single safe source table", async () => { ]); assert.equal(tab?.queryEditabilityReason, undefined); assert.equal(tab?.queryAnalysis?.multiSource, true); + assert.equal(tab?.queryAnalysis?.distinct, true); assert.equal(tab?.queryAnalysis?.allowInsertDelete, false); assert.equal(tab?.tableMeta?.tableName, "users"); - assert.deepEqual(tab?.querySourceColumns, ["id", "name", undefined]); + assert.deepEqual(tab?.querySourceColumns, ["id", "name"]); } finally { globalThis.fetch = originalFetch; restoreStorage(); diff --git a/packages/app-tests/sqlAnalysis.test.ts b/packages/app-tests/sqlAnalysis.test.ts index 81af508b98..0bcd68aead 100644 --- a/packages/app-tests/sqlAnalysis.test.ts +++ b/packages/app-tests/sqlAnalysis.test.ts @@ -70,6 +70,39 @@ test("recognizes single-table explicit columns mixed with alias star", () => { ]); }); +test("treats DISTINCT single-table projections as update-only", () => { + const result = analyzeEditableQueryEditability("select distinct id, name from users"); + + assert.equal(result.editable, true); + assert.equal(result.analysis.distinct, true); + assert.equal(result.analysis.allowInsertDelete, false); + assert.deepEqual(result.analysis.columns, [ + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { sourceName: "name", sourceNameQuoted: false, resultName: "name", expression: "name" }, + ]); +}); + +test("maps a DISTINCT qualified star to one joined source", () => { + const result = analyzeEditableQueryEditability("select distinct u.* from users u left join orders o on o.user_id = u.id"); + + assert.equal(result.editable, true); + assert.equal(result.analysis.distinct, true); + assert.equal(result.analysis.multiSource, true); + assert.equal(result.analysis.allowInsertDelete, false); + assert.deepEqual(result.analysis.columns, [{ star: true, sourceQualifier: "u", sourceKey: "u:0", resultName: "*", expression: "u.*" }]); +}); + +test("keeps ambiguous DISTINCT projections read-only", () => { + assert.deepEqual(analyzeEditableQueryEditability("select distinct * from users u join orders o on o.user_id = u.id"), { + editable: false, + reason: "complex-source", + }); + assert.deepEqual(analyzeEditableQueryEditability("select distinct on (user_id) id, user_id from orders order by user_id, id desc"), { + editable: false, + reason: "aggregation", + }); +}); + test("reports DuckDB external file scans as read-only external sources", () => { const result = analyzeEditableQueryEditability("SELECT * FROM '/tmp/duckdb_excel_extension_test.xlsx'");