diff --git a/apps/desktop/src/lib/sqlAnalysis.ts b/apps/desktop/src/lib/sqlAnalysis.ts index ee6fb8bec..341ed6ded 100644 --- a/apps/desktop/src/lib/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sqlAnalysis.ts @@ -23,7 +23,9 @@ export function isBinaryType(dataType: string): boolean { export interface EditableQueryInfo { schema: string | undefined; + schemaQuoted?: boolean; tableName: string; + tableNameQuoted?: boolean; tableAlias?: string; selectStar: boolean; columns: EditableQueryColumn[]; // empty array if SELECT * @@ -31,6 +33,7 @@ export interface EditableQueryInfo { export interface EditableQueryColumn { sourceName?: string; + sourceNameQuoted?: boolean; resultName: string; expression: string; } @@ -101,7 +104,9 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability { editable: true, analysis: { schema: source.schema, + schemaQuoted: source.schemaQuoted, tableName: source.tableName, + tableNameQuoted: source.tableNameQuoted, tableAlias: source.alias, selectStar, columns, @@ -166,6 +171,7 @@ function parseSelectColumn(col: string): EditableQueryColumn | null { if (alias === null) return parseComputedSelectColumn(col); return { sourceName: source.parts[source.parts.length - 1], + sourceNameQuoted: source.quoted[source.quoted.length - 1], resultName: alias ?? source.parts[source.parts.length - 1], expression: col.slice(0, source.end).trim(), }; @@ -176,6 +182,7 @@ function parseComputedSelectColumn(col: string): EditableQueryColumn | null { if (!alias) return null; return { sourceName: undefined, + sourceNameQuoted: false, resultName: alias.resultName, expression: alias.expression, }; @@ -207,7 +214,9 @@ function isSelectStar(body: string, alias: string | undefined): boolean { return new RegExp(`^${escapeRegExp(alias)}\\s*\\.\\s*\\*$`, "i").test(trimmed); } -function parseFromSource(body: string): { schema?: string; tableName: string; alias?: string } | null { +function parseFromSource( + body: string, +): { schema?: string; schemaQuoted?: boolean; tableName: string; tableNameQuoted?: boolean; alias?: string } | null { if (!body || /[,()]/.test(body) || /\bJOIN\b/i.test(body)) return null; const ident = parseQualifiedIdentifier(body); if (!ident || ident.parts.length < 1 || ident.parts.length > 2) return null; @@ -220,8 +229,10 @@ function parseFromSource(body: string): { schema?: string; tableName: string; al alias = aliasIdent.value; } const tableName = ident.parts[ident.parts.length - 1]; + const tableNameQuoted = ident.quoted[ident.quoted.length - 1]; const schema = ident.parts.length === 2 ? ident.parts[0] : undefined; - return { schema, tableName, alias }; + const schemaQuoted = ident.parts.length === 2 ? ident.quoted[0] : false; + return { schema, schemaQuoted, tableName, tableNameQuoted, alias }; } function isExternalFromSource(body: string): boolean { @@ -229,36 +240,40 @@ function isExternalFromSource(body: string): boolean { return /^'(?:''|[^'])*'(?:\s+(?:AS\s+)?[A-Za-z_][\w$]*)?$/i.test(trimmed) || /^[A-Za-z_][\w$]*\s*\(/.test(trimmed); } -function parseQualifiedIdentifier(text: string): { parts: string[]; end: number; rest: string; done: boolean } | null { +function parseQualifiedIdentifier( + text: string, +): { parts: string[]; quoted: boolean[]; end: number; rest: string; done: boolean } | null { const parts: string[] = []; + const quoted: boolean[] = []; let pos = 0; while (pos < text.length) { pos = skipWhitespace(text, pos); const ident = readIdentifier(text, pos); if (!ident) break; parts.push(ident.value); + quoted.push(ident.quoted); pos = skipWhitespace(text, ident.end); if (text[pos] !== ".") break; pos++; } if (parts.length === 0) return null; - return { parts, end: pos, rest: text.slice(pos), done: text.slice(pos).trim() === "" }; + return { parts, quoted, end: pos, rest: text.slice(pos), done: text.slice(pos).trim() === "" }; } -function readIdentifier(text: string, start: number): { value: string; end: number } | null { +function readIdentifier(text: string, start: number): { value: string; quoted: boolean; end: number } | null { const pos = skipWhitespace(text, start); const quote = text[pos]; if (quote === '"' || quote === "`" || quote === "[") { const close = quote === "[" ? "]" : quote; let value = ""; for (let i = pos + 1; i < text.length; i++) { - if (text[i] === close) return { value, end: i + 1 }; + if (text[i] === close) return { value, quoted: true, end: i + 1 }; value += text[i]; } return null; } const match = text.slice(pos).match(/^[A-Za-z_][\w$]*/); - return match ? { value: match[0], end: pos + match[0].length } : null; + return match ? { value: match[0], quoted: false, end: pos + match[0].length } : null; } function skipWhitespace(text: string, pos: number): number { diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 16dea4f0f..c2f1e27ac 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -7,7 +7,12 @@ import { orderPinnedFirst } from "@/lib/pinnedItems"; import { canCancelQueryExecution } from "@/lib/queryExecutionState"; import { closeAllTabsState, closeOtherTabsState } from "@/lib/tabCloseActions"; import { buildExplainSql, parseExplainResult } from "@/lib/explainPlan"; -import { allEditableColumnsWriteable, allPrimaryKeysPresent, sourceColumnsForResult } from "@/lib/sqlAnalysis"; +import { + allEditableColumnsWriteable, + allPrimaryKeysPresent, + sourceColumnsForResult, + type EditableQueryInfo, +} from "@/lib/sqlAnalysis"; import { restoreOpenTabsState, serializeOpenTabs } from "@/lib/openTabsPersistence"; import { evaluateMongoAggregateSafety, @@ -31,6 +36,30 @@ import type { SavedSqlFile } from "@/types/database"; const STORAGE_KEY = "dbx-open-tabs"; const ACTIVE_TAB_KEY = "dbx-active-tab"; +const ORACLE_LIKE_METADATA_TYPES = new Set(["oracle", "dameng", "oceanbase-oracle"]); + +function normalizeOracleLikeMetadataIdentifier(dbType: string, identifier: string | undefined, quoted?: boolean) { + if (!identifier || quoted || !ORACLE_LIKE_METADATA_TYPES.has(dbType)) return identifier; + return identifier.toUpperCase(); +} + +function normalizeOracleLikeQueryAnalysis( + dbType: string, + analysis: EditableQueryInfo, + schema: string | undefined, + tableName: string, +): EditableQueryInfo { + if (!ORACLE_LIKE_METADATA_TYPES.has(dbType)) return analysis; + return { + ...analysis, + schema, + tableName, + columns: analysis.columns.map((column) => ({ + ...column, + sourceName: normalizeOracleLikeMetadataIdentifier(dbType, column.sourceName, column.sourceNameQuoted), + })), + }; +} function saveTabs(tabs: QueryTab[], activeTabId: string | null) { try { @@ -570,15 +599,32 @@ export const useQueryStore = defineStore("query", () => { if (dbType === "postgres") schema = "public"; else schema = ""; } + const metadataSchema = + normalizeOracleLikeMetadataIdentifier( + dbType, + schema || undefined, + analysis.schema ? analysis.schemaQuoted : false, + ) || ""; + const metadataTableName = normalizeOracleLikeMetadataIdentifier( + dbType, + analysis.tableName, + analysis.tableNameQuoted, + )!; + const metadataAnalysis = normalizeOracleLikeQueryAnalysis( + dbType, + analysis, + metadataSchema || undefined, + metadataTableName, + ); try { console.info("[DBX][executeTabSql:metadata:get-columns:start]", { traceId, - schema, - table: analysis.tableName, + schema: metadataSchema, + table: metadataTableName, elapsed: elapsed?.(), }); - const columns = await api.getColumns(tab.connectionId, tab.database, schema, analysis.tableName); + const columns = await api.getColumns(tab.connectionId, tab.database, metadataSchema, metadataTableName); console.info("[DBX][executeTabSql:metadata:get-columns:done]", { traceId, columnCount: columns.length, @@ -586,8 +632,8 @@ export const useQueryStore = defineStore("query", () => { }); const primaryKeys = editablePrimaryKeys(dbType as DatabaseType, columns); const tableMeta = { - schema: schema || undefined, - tableName: analysis.tableName, + schema: metadataSchema || undefined, + tableName: metadataTableName, columns, primaryKeys, }; @@ -601,7 +647,7 @@ export const useQueryStore = defineStore("query", () => { }; } - if (!allPrimaryKeysPresent(primaryKeys, tab.result.columns, analysis)) { + if (!allPrimaryKeysPresent(primaryKeys, tab.result.columns, metadataAnalysis)) { return { queryAnalysis: undefined, querySourceColumns: undefined, @@ -610,7 +656,7 @@ export const useQueryStore = defineStore("query", () => { }; } - if (!allEditableColumnsWriteable(analysis, tab.result.columns)) { + if (!allEditableColumnsWriteable(metadataAnalysis, tab.result.columns)) { return { queryAnalysis: undefined, querySourceColumns: undefined, @@ -620,8 +666,8 @@ export const useQueryStore = defineStore("query", () => { } return { - queryAnalysis: analysis, - querySourceColumns: sourceColumnsForResult(analysis, tab.result.columns), + queryAnalysis: metadataAnalysis, + querySourceColumns: sourceColumnsForResult(metadataAnalysis, tab.result.columns), queryEditabilityReason: undefined, tableMeta, }; diff --git a/apps/desktop/src/types/database.ts b/apps/desktop/src/types/database.ts index e56d9d223..9e6c1a545 100644 --- a/apps/desktop/src/types/database.ts +++ b/apps/desktop/src/types/database.ts @@ -392,11 +392,14 @@ export interface QueryTab { }; queryAnalysis?: { schema?: string; + schemaQuoted?: boolean; tableName: string; + tableNameQuoted?: boolean; tableAlias?: string; selectStar: boolean; columns: { sourceName?: string; + sourceNameQuoted?: boolean; resultName: string; expression: string; }[]; diff --git a/crates/dbx-core/src/sql_editability.rs b/crates/dbx-core/src/sql_editability.rs index 7f8ad0cd6..245bf28be 100644 --- a/crates/dbx-core/src/sql_editability.rs +++ b/crates/dbx-core/src/sql_editability.rs @@ -5,7 +5,9 @@ use serde::{Deserialize, Serialize}; pub struct EditableQueryInfo { #[serde(skip_serializing_if = "Option::is_none")] pub schema: Option, + pub schema_quoted: bool, pub table_name: String, + pub table_name_quoted: bool, #[serde(skip_serializing_if = "Option::is_none")] pub table_alias: Option, pub select_star: bool, @@ -17,6 +19,7 @@ pub struct EditableQueryInfo { pub struct EditableQueryColumn { #[serde(skip_serializing_if = "Option::is_none")] pub source_name: Option, + pub source_name_quoted: bool, pub result_name: String, pub expression: String, } @@ -51,19 +54,22 @@ pub struct QueryEditability { #[derive(Debug, Clone, PartialEq, Eq)] struct FromSource { schema: Option, + schema_quoted: bool, table_name: String, + table_name_quoted: bool, alias: Option, } #[derive(Debug, Clone, PartialEq, Eq)] struct QualifiedIdentifier { - parts: Vec, + parts: Vec, end: usize, } #[derive(Debug, Clone, PartialEq, Eq)] struct Identifier { value: String, + quoted: bool, end: usize, } @@ -131,7 +137,9 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability { editable: true, analysis: Some(EditableQueryInfo { schema: source.schema, + schema_quoted: source.schema_quoted, table_name: source.table_name, + table_name_quoted: source.table_name_quoted, table_alias: source.alias, select_star, columns, @@ -195,9 +203,11 @@ fn parse_select_column(column: &str) -> Option { let Some(alias) = parse_column_alias(rest) else { return parse_computed_select_column(column); }; - let source_name = source.parts.last()?.clone(); + let source = source.parts.last()?; + let source_name = source.value.clone(); Some(EditableQueryColumn { source_name: Some(source_name.clone()), + source_name_quoted: source.quoted, result_name: alias.unwrap_or(source_name), expression: column[..source.end].trim().to_string(), }) @@ -205,7 +215,12 @@ fn parse_select_column(column: &str) -> Option { fn parse_computed_select_column(column: &str) -> Option { let alias = parse_expression_alias(column)?; - Some(EditableQueryColumn { source_name: None, result_name: alias.result_name, expression: alias.expression }) + Some(EditableQueryColumn { + source_name: None, + source_name_quoted: false, + result_name: alias.result_name, + expression: alias.expression, + }) } #[derive(Debug, Clone, PartialEq, Eq)] @@ -307,9 +322,16 @@ fn parse_from_source(body: &str) -> Option { } Some(alias_ident.value) }; - let table_name = ident.parts.last()?.clone(); - let schema = if ident.parts.len() == 2 { Some(ident.parts[0].clone()) } else { None }; - Some(FromSource { schema, table_name, alias }) + let table = ident.parts.last()?; + let table_name = table.value.clone(); + let table_name_quoted = table.quoted; + let (schema, schema_quoted) = if ident.parts.len() == 2 { + let schema = &ident.parts[0]; + (Some(schema.value.clone()), schema.quoted) + } else { + (None, false) + }; + Some(FromSource { schema, schema_quoted, table_name, table_name_quoted, alias }) } fn is_external_from_source(body: &str) -> bool { @@ -354,8 +376,9 @@ fn parse_qualified_identifier(text: &str) -> Option { let Some(ident) = read_identifier(text, pos) else { break; }; - parts.push(ident.value); - pos = skip_whitespace(text, ident.end); + let ident_end = ident.end; + parts.push(ident); + pos = skip_whitespace(text, ident_end); if !text[pos..].starts_with('.') { break; } @@ -376,7 +399,7 @@ fn read_identifier(text: &str, start: usize) -> Option { let mut value = String::new(); for (offset, ch) in chars { if ch == close { - return Some(Identifier { value, end: pos + offset + ch.len_utf8() }); + return Some(Identifier { value, quoted: true, end: pos + offset + ch.len_utf8() }); } value.push(ch); } @@ -389,11 +412,11 @@ fn read_identifier(text: &str, start: usize) -> Option { let mut end = pos + first.len_utf8(); for (offset, ch) in text[end..].char_indices() { if !(ch.is_ascii_alphanumeric() || ch == '_' || ch == '$') { - return Some(Identifier { value: text[pos..end + offset].to_string(), end: end + offset }); + return Some(Identifier { value: text[pos..end + offset].to_string(), quoted: false, end: end + offset }); } } end = text.len(); - Some(Identifier { value: text[pos..end].to_string(), end }) + Some(Identifier { value: text[pos..end].to_string(), quoted: false, end }) } fn skip_whitespace(text: &str, pos: usize) -> usize { @@ -532,17 +555,21 @@ mod tests { editable: true, analysis: Some(EditableQueryInfo { schema: Some("public".to_string()), + schema_quoted: false, table_name: "users".to_string(), + table_name_quoted: false, table_alias: None, select_star: false, columns: vec![ EditableQueryColumn { source_name: Some("id".to_string()), + source_name_quoted: false, result_name: "id".to_string(), expression: "id".to_string(), }, EditableQueryColumn { source_name: Some("name".to_string()), + source_name_quoted: false, result_name: "name".to_string(), expression: "name".to_string(), }, @@ -562,17 +589,21 @@ mod tests { result.analysis.unwrap(), EditableQueryInfo { schema: Some("app schema".to_string()), + schema_quoted: true, table_name: "user table".to_string(), + table_name_quoted: true, table_alias: Some("u".to_string()), select_star: false, columns: vec![ EditableQueryColumn { source_name: Some("id".to_string()), + source_name_quoted: true, result_name: "id".to_string(), expression: r#"u."id""#.to_string(), }, EditableQueryColumn { source_name: Some("full name".to_string()), + source_name_quoted: true, result_name: "full name".to_string(), expression: r#"u."full name""#.to_string(), }, @@ -587,7 +618,9 @@ mod tests { analyze_editable_query("select * from users").unwrap(), EditableQueryInfo { schema: None, + schema_quoted: false, table_name: "users".to_string(), + table_name_quoted: false, table_alias: None, select_star: true, columns: Vec::new(), @@ -631,21 +664,25 @@ mod tests { vec![ EditableQueryColumn { source_name: Some("iso3".to_string()), + source_name_quoted: false, result_name: "iso3".to_string(), expression: "iso3".to_string(), }, EditableQueryColumn { source_name: Some("year".to_string()), + source_name_quoted: false, result_name: "year".to_string(), expression: "year".to_string(), }, EditableQueryColumn { source_name: Some("country_name".to_string()), + source_name_quoted: false, result_name: "country_name".to_string(), expression: "country_name".to_string(), }, EditableQueryColumn { source_name: None, + source_name_quoted: false, result_name: "score".to_string(), expression: "ihli / gdp_pc".to_string(), }, diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index 7e5609c58..368e72353 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -36,6 +36,22 @@ function conn(id: string): ConnectionConfig { }; } +function oracleConn(id: string): ConnectionConfig { + return { + ...conn(id), + db_type: "oracle", + port: 1521, + }; +} + +async function waitFor(predicate: () => boolean, timeoutMs = 1000) { + const started = Date.now(); + while (!predicate()) { + if (Date.now() - started > timeoutMs) throw new Error("timed out waiting for condition"); + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + test("setErrorResult stops loading and shows the error result", () => { setActivePinia(createPinia()); const store = useQueryStore(); @@ -115,6 +131,107 @@ test("editing query sql preserves the displayed result editability state", () => assert.equal(tab.tableMeta?.tableName, "users"); }); +test("normalizes unquoted Oracle query identifiers before loading editable metadata", async () => { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + const columnRequests: Array<{ schema: string | null; table: string | null }> = []; + + connectionStore.addEphemeralConnection(oracleConn("oracle-1")); + + globalThis.fetch = (async (input, init) => { + const url = String(input); + if (url === "/api/query/execute-multi") { + return new Response( + JSON.stringify([ + { + columns: ["ID", "NAME"], + rows: [[1, "Ada"]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url === "/api/query/analyze-editability") { + const body = JSON.parse(String(init?.body ?? "{}")); + assert.equal(body.sql, "select id, name from users"); + return new Response( + JSON.stringify({ + editable: true, + analysis: { + schema: undefined, + schemaQuoted: false, + tableName: "users", + tableNameQuoted: false, + tableAlias: undefined, + selectStar: false, + columns: [ + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { sourceName: "name", sourceNameQuoted: false, resultName: "name", expression: "name" }, + ], + }, + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url.startsWith("/api/schema/columns?")) { + const params = new URL(url, "http://localhost").searchParams; + columnRequests.push({ schema: params.get("schema"), table: params.get("table") }); + return new Response( + JSON.stringify([ + { + name: "ID", + data_type: "NUMBER", + is_nullable: false, + column_default: null, + is_primary_key: true, + extra: null, + comment: "identifier", + }, + { + name: "NAME", + data_type: "VARCHAR2", + is_nullable: true, + column_default: null, + is_primary_key: false, + extra: null, + comment: "display name", + }, + ]), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url === "/api/query/prepare-pagination-plan") { + const body = JSON.parse(String(init?.body ?? "{}")); + return new Response(JSON.stringify({ sqlToExecute: body.options.sql, useAgentResultSession: false }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response("unexpected request", { status: 500 }); + }) as typeof fetch; + + try { + const tabId = store.createTab("oracle-1", "ORCL", "Query 1", "query", "app"); + await store.executeTabSql(tabId, "select id, name from users"); + + const tab = store.tabs.find((item) => item.id === tabId); + await waitFor(() => columnRequests.length > 0 && tab?.tableMeta?.tableName === "USERS"); + assert.deepEqual(columnRequests, [{ schema: "APP", table: "USERS" }]); + assert.equal(tab?.tableMeta?.schema, "APP"); + assert.equal(tab?.tableMeta?.tableName, "USERS"); + assert.deepEqual(tab?.querySourceColumns, ["ID", "NAME"]); + assert.equal(tab?.queryEditabilityReason, undefined); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } +}); + test("evicting cached tab results releases multi-result payloads and sessions", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); diff --git a/packages/app-tests/sqlAnalysis.test.ts b/packages/app-tests/sqlAnalysis.test.ts index 423d433be..b5decc1c3 100644 --- a/packages/app-tests/sqlAnalysis.test.ts +++ b/packages/app-tests/sqlAnalysis.test.ts @@ -15,12 +15,14 @@ test("recognizes a simple single-table SELECT as editable", () => { assert.equal(result.editable, true); assert.deepEqual(result.analysis, { schema: "public", + schemaQuoted: false, tableName: "users", + tableNameQuoted: false, tableAlias: undefined, selectStar: false, columns: [ - { sourceName: "id", resultName: "id", expression: "id" }, - { sourceName: "name", resultName: "name", expression: "name" }, + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { sourceName: "name", sourceNameQuoted: false, resultName: "name", expression: "name" }, ], }); }); @@ -31,12 +33,14 @@ test("recognizes quoted table names and table aliases", () => { assert.equal(result.editable, true); assert.deepEqual(result.analysis, { schema: "app schema", + schemaQuoted: true, tableName: "user table", + tableNameQuoted: true, tableAlias: "u", selectStar: false, columns: [ - { sourceName: "id", resultName: "id", expression: 'u."id"' }, - { sourceName: "full name", resultName: "full name", expression: 'u."full name"' }, + { sourceName: "id", sourceNameQuoted: true, resultName: "id", expression: 'u."id"' }, + { sourceName: "full name", sourceNameQuoted: true, resultName: "full name", expression: 'u."full name"' }, ], }); }); @@ -44,7 +48,9 @@ test("recognizes quoted table names and table aliases", () => { test("keeps the legacy analyzer API for editable SELECT queries", () => { assert.deepEqual(analyzeEditableQuery("select * from users"), { schema: undefined, + schemaQuoted: false, tableName: "users", + tableNameQuoted: false, tableAlias: undefined, selectStar: true, columns: [], @@ -87,10 +93,10 @@ test("keeps single-table expression columns while mapping writable source column assert.equal(result.editable, true); assert.deepEqual(result.analysis.columns, [ - { sourceName: "iso3", resultName: "iso3", expression: "iso3" }, - { sourceName: "year", resultName: "year", expression: "year" }, - { sourceName: "country_name", resultName: "country_name", expression: "country_name" }, - { sourceName: undefined, resultName: "score", expression: "ihli / gdp_pc" }, + { sourceName: "iso3", sourceNameQuoted: false, resultName: "iso3", expression: "iso3" }, + { sourceName: "year", sourceNameQuoted: false, resultName: "year", expression: "year" }, + { sourceName: "country_name", sourceNameQuoted: false, resultName: "country_name", expression: "country_name" }, + { sourceName: undefined, sourceNameQuoted: false, resultName: "score", expression: "ihli / gdp_pc" }, ]); assert.equal( allPrimaryKeysPresent(["iso3", "year"], ["iso3", "year", "country_name", "score"], result.analysis),