From c38bc05417b59d04053d6f2e09548a2df531ffe1 Mon Sep 17 00:00:00 2001 From: zipg Date: Tue, 30 Jun 2026 20:15:59 +0800 Subject: [PATCH] =?UTF-8?q?Fix=EF=BC=9A=E4=BF=AE=E5=A4=8D=20SQL=20?= =?UTF-8?q?=E5=AD=90=E6=9F=A5=E8=AF=A2=E8=AF=AD=E4=B9=89=E8=AF=8A=E6=96=AD?= =?UTF-8?q?=E8=AF=AF=E6=8A=A5=20(#2184)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix：修复 SQL 子查询语义诊断误报 * Improve：添加 SQL 语义诊断开关 * Improve：优化 SQL 语义诊断开关说明 * Fix：收紧 SQL 语义诊断飘红范围 * Improve：调整 SQL 语义诊断默认策略 --------- Co-authored-by: staff --- .../editor/EditorSettingsDialog.vue | 43 +++++-- .../src/components/editor/QueryEditor.vue | 37 +++++- apps/desktop/src/i18n/locales/en.ts | 2 + apps/desktop/src/i18n/locales/ja.ts | 2 + apps/desktop/src/i18n/locales/zh-CN.ts | 2 + .../desktop/src/lib/sqlSemanticDiagnostics.ts | 111 +++++++++++++++++- .../stores/__tests__/settingsStore.spec.ts | 16 +++ apps/desktop/src/stores/settingsStore.ts | 26 ++++ apps/desktop/src/types/database.ts | 8 ++ crates/dbx-core/src/sql_analysis.rs | 83 ++++++++++--- crates/dbx-core/tests/sql_analysis.rs | 20 ++++ .../app-tests/sqlSemanticDiagnostics.test.ts | 98 ++++++++++++++++ 12 files changed, 417 insertions(+), 31 deletions(-) diff --git a/apps/desktop/src/components/editor/EditorSettingsDialog.vue b/apps/desktop/src/components/editor/EditorSettingsDialog.vue index fc2141923..62f2ad1f2 100644 --- a/apps/desktop/src/components/editor/EditorSettingsDialog.vue +++ b/apps/desktop/src/components/editor/EditorSettingsDialog.vue @@ -34,6 +34,7 @@ import { type DesktopIconTheme, type InterfaceLayout, type DisconnectTabHandlingMode, + type SqlSemanticDiagnosticsMode, type UpdateDownloadSource, type CustomThemeColors, type CustomTheme, @@ -190,6 +191,8 @@ const editExecuteMode = ref(settingsStore.editorSettings.executeMode); const editShowExecutionTargetPicker = ref(settingsStore.editorSettings.showExecutionTargetPicker); const editAutoAliasTables = ref(settingsStore.editorSettings.autoAliasTables); const editWordWrap = ref(settingsStore.editorSettings.wordWrap); +const editSqlSemanticDiagnosticsMode = ref(settingsStore.editorSettings.sqlSemanticDiagnosticsMode); +const editSqlSemanticDiagnosticsEnabled = ref(settingsStore.editorSettings.sqlSemanticDiagnosticsEnabled); const editConfirmDangerousSqlExecution = ref(settingsStore.editorSettings.confirmDangerousSqlExecution); const editAppLayout = ref(settingsStore.editorSettings.appLayout); const editShowTrayIcon = ref(settingsStore.desktopSettings.show_tray_icon); @@ -468,6 +471,8 @@ watch( editShowExecutionTargetPicker.value = settingsStore.editorSettings.showExecutionTargetPicker; editAutoAliasTables.value = settingsStore.editorSettings.autoAliasTables; editWordWrap.value = settingsStore.editorSettings.wordWrap; + editSqlSemanticDiagnosticsMode.value = settingsStore.editorSettings.sqlSemanticDiagnosticsMode; + editSqlSemanticDiagnosticsEnabled.value = settingsStore.editorSettings.sqlSemanticDiagnosticsEnabled; editConfirmDangerousSqlExecution.value = settingsStore.editorSettings.confirmDangerousSqlExecution; editAppLayout.value = settingsStore.editorSettings.appLayout; editShowTrayIcon.value = settingsStore.desktopSettings.show_tray_icon; @@ -532,6 +537,8 @@ function hasChanges(): boolean { editShowExecutionTargetPicker.value !== settingsStore.editorSettings.showExecutionTargetPicker || editAutoAliasTables.value !== settingsStore.editorSettings.autoAliasTables || editWordWrap.value !== settingsStore.editorSettings.wordWrap || + editSqlSemanticDiagnosticsMode.value !== settingsStore.editorSettings.sqlSemanticDiagnosticsMode || + editSqlSemanticDiagnosticsEnabled.value !== settingsStore.editorSettings.sqlSemanticDiagnosticsEnabled || editConfirmDangerousSqlExecution.value !== settingsStore.editorSettings.confirmDangerousSqlExecution || editAppLayout.value !== settingsStore.editorSettings.appLayout || editShowTrayIcon.value !== settingsStore.desktopSettings.show_tray_icon || @@ -582,6 +589,7 @@ async function persistSettings() { showExecutionTargetPicker: editShowExecutionTargetPicker.value, autoAliasTables: editAutoAliasTables.value, wordWrap: editWordWrap.value, + sqlSemanticDiagnosticsMode: editSqlSemanticDiagnosticsMode.value, confirmDangerousSqlExecution: editConfirmDangerousSqlExecution.value, appLayout: editAppLayout.value, showColumnCommentsInHeader: editShowColumnCommentsInHeader.value, @@ -643,6 +651,8 @@ function resetDefaultsForTab(tab: SettingsCategory) { editShowExecutionTargetPicker.value = DEFAULT_EDITOR_SETTINGS.showExecutionTargetPicker; editAutoAliasTables.value = DEFAULT_EDITOR_SETTINGS.autoAliasTables; editWordWrap.value = DEFAULT_EDITOR_SETTINGS.wordWrap; + editSqlSemanticDiagnosticsMode.value = DEFAULT_EDITOR_SETTINGS.sqlSemanticDiagnosticsMode; + editSqlSemanticDiagnosticsEnabled.value = DEFAULT_EDITOR_SETTINGS.sqlSemanticDiagnosticsEnabled; editConfirmDangerousSqlExecution.value = DEFAULT_EDITOR_SETTINGS.confirmDangerousSqlExecution; } else if (tab === "formatter") { editSqlFormatter.value = normalizeSqlFormatterSettings(DEFAULT_EDITOR_SETTINGS.sqlFormatter); @@ -702,6 +712,8 @@ function resetAllDefaults() { editShowExecutionTargetPicker.value = DEFAULT_EDITOR_SETTINGS.showExecutionTargetPicker; editAutoAliasTables.value = DEFAULT_EDITOR_SETTINGS.autoAliasTables; editWordWrap.value = DEFAULT_EDITOR_SETTINGS.wordWrap; + editSqlSemanticDiagnosticsMode.value = DEFAULT_EDITOR_SETTINGS.sqlSemanticDiagnosticsMode; + editSqlSemanticDiagnosticsEnabled.value = DEFAULT_EDITOR_SETTINGS.sqlSemanticDiagnosticsEnabled; editConfirmDangerousSqlExecution.value = DEFAULT_EDITOR_SETTINGS.confirmDangerousSqlExecution; editAppLayout.value = DEFAULT_EDITOR_SETTINGS.appLayout; editShowTrayIcon.value = DEFAULT_DESKTOP_SETTINGS.show_tray_icon; @@ -866,6 +878,11 @@ function onExecuteModeChange(v: any) { if (v === "all" || v === "current") editExecuteMode.value = v; } +function onSqlSemanticDiagnosticsEnabledChange(value: boolean) { + editSqlSemanticDiagnosticsEnabled.value = value; + editSqlSemanticDiagnosticsMode.value = value ? "enabled" : "disabled"; +} + function onFontFamilyChange(v: any) { if (typeof v === "string") editFontFamily.value = v; } @@ -2005,14 +2022,26 @@ watch( -
-
- -

- {{ t("settings.confirmDangerousSqlExecutionDescription") }} -

+
+
+
+ +

+ {{ t("settings.sqlSemanticDiagnosticsEnabledDescription") }} +

+
+ +
+ +
+
+ +

+ {{ t("settings.confirmDangerousSqlExecutionDescription") }} +

+
+
-
diff --git a/apps/desktop/src/components/editor/QueryEditor.vue b/apps/desktop/src/components/editor/QueryEditor.vue index 9437a9860..758d62893 100644 --- a/apps/desktop/src/components/editor/QueryEditor.vue +++ b/apps/desktop/src/components/editor/QueryEditor.vue @@ -926,6 +926,17 @@ function setSemanticDiagnostics(next: SqlSemanticDiagnostic[]) { reconfigureDiagnostics(); } +function clearScheduledSemanticDiagnostics() { + semanticDiagnosticRunId++; + if (semanticDiagnosticTimer) clearTimeout(semanticDiagnosticTimer); + semanticDiagnosticTimer = null; + pendingSemanticDiagnosticPreserveOutsideRanges = false; +} + +function shouldSkipSqlSemanticDiagnostics() { + return props.databaseType !== "redis" && !settingsStore.editorSettings.sqlSemanticDiagnosticsEnabled; +} + function rangesOverlap(left: { from: number; to: number }, right: { from: number; to: number }): boolean { return left.from < right.to && right.from < left.to; } @@ -1046,6 +1057,10 @@ async function refreshSemanticDiagnostics(options: { preserveOutsideRanges?: boo setSemanticDiagnostics(buildRedisSyntaxDiagnostics(sql)); return; } + if (shouldSkipSqlSemanticDiagnostics()) { + setSemanticDiagnostics([]); + return; + } if (!shouldRunSqlSemanticDiagnostics(sql, currentView.state.selection.main.head, { databaseType: props.databaseType })) { scheduleSemanticDiagnostics(1200, { preserveOutsideRanges: options.preserveOutsideRanges }); return; @@ -1102,6 +1117,11 @@ async function refreshSemanticDiagnostics(options: { preserveOutsideRanges?: boo function scheduleSemanticDiagnostics(delay = 500, options: { preserveOutsideRanges?: boolean } = {}) { if (!editorIsActive) return; + if (shouldSkipSqlSemanticDiagnostics()) { + clearScheduledSemanticDiagnostics(); + setSemanticDiagnostics([]); + return; + } pendingSemanticDiagnosticPreserveOutsideRanges = !!options.preserveOutsideRanges; if (semanticDiagnosticTimer) clearTimeout(semanticDiagnosticTimer); semanticDiagnosticTimer = setTimeout(() => { @@ -2512,14 +2532,25 @@ watch( { deep: true }, ); +watch( + () => settingsStore.editorSettings.sqlSemanticDiagnosticsEnabled, + (enabled) => { + if (props.databaseType === "redis") return; + if (!shouldSkipSqlSemanticDiagnostics() && enabled) { + scheduleSemanticDiagnostics(0); + return; + } + clearScheduledSemanticDiagnostics(); + setSemanticDiagnostics([]); + }, +); + function pauseQueryEditorBackgroundWork() { flushEditorViewport(); flushEditorSelection(); clearTableNavigationHover(); editorIsActive = false; - semanticDiagnosticRunId++; - if (semanticDiagnosticTimer) clearTimeout(semanticDiagnosticTimer); - semanticDiagnosticTimer = null; + clearScheduledSemanticDiagnostics(); completionEpoch++; unregisterTableReferenceDropListener(); } diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index fa61a2106..65b59859e 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -2671,6 +2671,8 @@ export default { showExecutionTargetPickerDescription: "When enabled, running without a selection lets you choose between the current statement and all SQL.", wordWrap: "Word wrap", wordWrapDescription: "Wrap long SQL lines within the editor width", + sqlSemanticDiagnosticsEnabled: "SQL semantic diagnostics", + sqlSemanticDiagnosticsEnabledDescription: "When enabled, the editor reports semantic issues such as unknown tables and columns. Disable it to reduce parsing and metadata checks for large SQL.", confirmDangerousSqlExecution: "Confirm before dangerous SQL", confirmDangerousSqlExecutionDescription: "When disabled, ALTER, DROP, DELETE, TRUNCATE, and other dangerous SQL run without the warning dialog.", autoAliasTables: "Automatically add table aliases", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index ecebae175..bd6727bd2 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -2644,6 +2644,8 @@ export default withEnglishFallback({ showExecutionTargetPickerDescription: "有効にすると、選択なしで実行するときに現在の文とすべてのSQLを一時的に選べます。", wordWrap: "折り返し", wordWrapDescription: "長いSQL行をエディタ幅内で折り返します", + sqlSemanticDiagnosticsEnabled: "SQLセマンティック診断", + sqlSemanticDiagnosticsEnabledDescription: "有効時、エディタは不明なテーブルや列などの意味的な問題を表示します。大きなSQLの解析とメタデータ確認の負荷を減らすには無効にします。", confirmDangerousSqlExecution: "危険なSQLの前に確認", confirmDangerousSqlExecutionDescription: "無効時、ALTER、DROP、DELETE、TRUNCATEなどの危険なSQLが警告ダイアログなしで実行されます。", autoAliasTables: "テーブル別名を自動追加", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index 33a3c793b..dbc6aff28 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -2678,6 +2678,8 @@ export default withEnglishFallback({ showExecutionTargetPickerDescription: "开启后，无选区执行时可在当前语句和全部 SQL 之间临时选择。", wordWrap: "自动换行", wordWrapDescription: "长 SQL 在编辑器宽度内自动折行显示", + sqlSemanticDiagnosticsEnabled: "SQL 语义诊断", + sqlSemanticDiagnosticsEnabledDescription: "开启后，编辑器会提示未知表、字段等语义问题；关闭可减少 SQL 解析和元数据检查的性能开销。", confirmDangerousSqlExecution: "执行危险 SQL 前弹出确认", confirmDangerousSqlExecutionDescription: "关闭后，ALTER、DROP、DELETE、TRUNCATE 等危险 SQL 将直接执行。", autoAliasTables: "自动添加表别名", diff --git a/apps/desktop/src/lib/sqlSemanticDiagnostics.ts b/apps/desktop/src/lib/sqlSemanticDiagnostics.ts index d8cea1727..76e06da94 100644 --- a/apps/desktop/src/lib/sqlSemanticDiagnostics.ts +++ b/apps/desktop/src/lib/sqlSemanticDiagnostics.ts @@ -1,7 +1,7 @@ import type { SqlCompletionColumn, SqlCompletionTable } from "@/lib/sqlCompletion"; import { getSqlCompletionContext } from "@/lib/sqlCompletion"; import { executableStatementRanges, type SqlTextRange } from "@/lib/sqlStatementRanges"; -import type { DatabaseType, SqlColumnReference, SqlReferenceAnalysis, SqlTableReference, SqlTextSpan } from "@/types/database"; +import type { DatabaseType, SqlColumnReference, SqlReferenceAnalysis, SqlReferenceScope, SqlTableReference, SqlTextSpan } from "@/types/database"; export interface SqlSemanticDiagnostic { span: SqlTextSpan; @@ -42,6 +42,7 @@ export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, sche const diagnostics: SqlSemanticDiagnostic[] = []; const tables = analysis.tables.filter((table) => table.name.trim()); const knownTables = new Map(); + const scopesById = scopesByIdMap(analysis.scopes); for (const table of tables) { knownTables.set(normalizeName(table.name), table); @@ -52,14 +53,14 @@ export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, sche for (const table of tables) { if (!schema.missingTables?.has(tableReferenceKey(table))) continue; diagnostics.push({ - span: table.span, + span: trimSqlTextSpanWhitespace(schema.sql, table.span), message: `Unknown table ${displayTableName(table)}`, severity: "error", }); } for (const column of analysis.columns) { - const table = resolveColumnTable(column, tables, knownTables, schema.sql); + const table = resolveColumnTable(column, tables, knownTables, schema.sql, scopesById); if (!table) continue; if (schema.missingTables?.has(tableReferenceKey(table))) continue; @@ -71,7 +72,7 @@ export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, sche const displayName = column.qualifier ? `${column.qualifier}.${column.name}` : column.name; diagnostics.push({ - span: column.span, + span: trimSqlTextSpanWhitespace(schema.sql, column.span), message: `Unknown column ${displayName}`, severity: "error", }); @@ -80,6 +81,70 @@ export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, sche return diagnostics; } +function trimSqlTextSpanWhitespace(sql: string | undefined, span: SqlTextSpan): SqlTextSpan { + if (!sql) return span; + const range = sqlTextSpanToOffsetRange(sql, span); + if (!range) return span; + + let from = range.from; + let to = range.to; + while (from < to && /\s/.test(sql[from] ?? "")) from += 1; + while (to > from && /\s/.test(sql[to - 1] ?? "")) to -= 1; + if (from === range.from && to === range.to) return span; + + const start = offsetToSqlTextStartPosition(sql, from); + const end = offsetToSqlTextEndPosition(sql, to); + if (!start || !end) return span; + return { + start_line: start.line, + start_column: start.column, + end_line: end.line, + end_column: Math.max(end.column, start.column), + }; +} + +function sqlTextSpanToOffsetRange(sql: string, span: SqlTextSpan): { from: number; to: number } | null { + if (!span.start_line || !span.start_column) return null; + const from = sqlTextPositionToOffset(sql, span.start_line, span.start_column - 1); + const to = sqlTextPositionToOffset(sql, Math.max(span.end_line, span.start_line), Math.max(span.end_column, span.start_column)); + if (from == null || to == null || to <= from) return null; + return { from, to }; +} + +function sqlTextPositionToOffset(sql: string, line: number, column: number): number | null { + const lines = sql.split(/\r?\n/); + if (line < 1 || line > lines.length) return null; + let offset = 0; + for (let index = 0; index < line - 1; index += 1) { + offset += lines[index].length + 1; + } + return Math.min(offset + Math.max(column, 0), offset + lines[line - 1].length); +} + +function offsetToSqlTextStartPosition(sql: string, offset: number): { line: number; column: number } | null { + const position = offsetToLineColumn(sql, offset); + return position ? { line: position.line, column: position.column + 1 } : null; +} + +function offsetToSqlTextEndPosition(sql: string, offset: number): { line: number; column: number } | null { + return offsetToLineColumn(sql, offset); +} + +function offsetToLineColumn(sql: string, offset: number): { line: number; column: number } | null { + if (offset < 0 || offset > sql.length) return null; + const lines = sql.split(/\r?\n/); + let remaining = offset; + for (let index = 0; index < lines.length; index += 1) { + const lineLength = lines[index].length; + if (remaining <= lineLength) { + return { line: index + 1, column: remaining }; + } + remaining -= lineLength + 1; + } + const lastLine = lines[lines.length - 1] ?? ""; + return { line: lines.length, column: lastLine.length }; +} + export function buildSqlParserErrorDiagnostic(error: unknown, sql: string): SqlSemanticDiagnostic | null { const message = errorMessage(error); const location = /\bat Line:\s*(\d+),\s*Column:\s*(\d+)\b/i.exec(message); @@ -129,8 +194,8 @@ export function isSqlSemanticDiagnosticInputContext(sql: string, cursor: number, return context.exclusiveColumnSuggestions || !!context.qualifier || ((context.suggestTables || context.exclusiveTableSuggestions) && isCursorAfterTableTrigger(sql, cursor)); } -function resolveColumnTable(column: SqlColumnReference, tables: SqlTableReference[], knownTables: Map, sql?: string): SqlTableReference | null { - const candidateTables = sql ? tablesInSameStatement(tables, column, sql) : tables; +function resolveColumnTable(column: SqlColumnReference, tables: SqlTableReference[], knownTables: Map, sql?: string, scopesById?: Map): SqlTableReference | null { + const candidateTables = candidateTablesForColumn(tables, column, sql, scopesById); if (column.qualifier) { return tableLookupFor(candidateTables).get(normalizeName(column.qualifier)) ?? (sql ? null : (knownTables.get(normalizeName(column.qualifier)) ?? null)); } @@ -138,6 +203,34 @@ function resolveColumnTable(column: SqlColumnReference, tables: SqlTableReferenc return candidateTables[0]; } +function candidateTablesForColumn(tables: SqlTableReference[], column: SqlColumnReference, sql?: string, scopesById?: Map): SqlTableReference[] { + const scoped = tablesInVisibleScopes(tables, column, scopesById); + if (scoped) return scoped; + return sql ? tablesInSameStatement(tables, column, sql) : tables; +} + +function tablesInVisibleScopes(tables: SqlTableReference[], column: SqlColumnReference, scopesById?: Map): SqlTableReference[] | null { + if (column.scope_id == null || !scopesById || scopesById.size === 0) return null; + if (!column.qualifier) { + const currentScopeTables = tables.filter((table) => table.scope_id === column.scope_id); + if (currentScopeTables.length > 0) return currentScopeTables; + } + const visibleScopeIds = scopeAndParents(column.scope_id, scopesById); + if (visibleScopeIds.size === 0) return null; + return tables.filter((table) => table.scope_id != null && visibleScopeIds.has(table.scope_id)); +} + +function scopeAndParents(scopeId: number, scopesById: Map): Set { + const ids = new Set(); + let current: number | undefined = scopeId; + while (current != null && !ids.has(current)) { + ids.add(current); + const scope = scopesById.get(current); + current = scope?.parent_id ?? undefined; + } + return ids; +} + function tableLookupFor(tables: SqlTableReference[]): Map { const lookup = new Map(); for (const table of tables) { @@ -157,6 +250,12 @@ function tablesInSameStatement(tables: SqlTableReference[], column: SqlColumnRef }); } +function scopesByIdMap(scopes: readonly SqlReferenceScope[] | undefined): Map { + const map = new Map(); + for (const scope of scopes ?? []) map.set(scope.id, scope); + return map; +} + function spanStartOffset(sql: string, span: SqlTextSpan): number | null { if (!span.start_line || !span.start_column) return null; const lines = sql.split(/\r?\n/); diff --git a/apps/desktop/src/stores/__tests__/settingsStore.spec.ts b/apps/desktop/src/stores/__tests__/settingsStore.spec.ts index 7784aaa24..19782e107 100644 --- a/apps/desktop/src/stores/__tests__/settingsStore.spec.ts +++ b/apps/desktop/src/stores/__tests__/settingsStore.spec.ts @@ -10,6 +10,22 @@ describe("normalizeEditorSettings", () => { expect(normalizeEditorSettings({ autoAliasTables: false }).autoAliasTables).toBe(false); }); + it("keeps SQL semantic diagnostics in auto mode and disabled by default", () => { + const settings = normalizeEditorSettings({}); + expect(settings.sqlSemanticDiagnosticsMode).toBe("auto"); + expect(settings.sqlSemanticDiagnosticsEnabled).toBe(false); + }); + + it("preserves explicit SQL semantic diagnostics modes", () => { + expect(normalizeEditorSettings({ sqlSemanticDiagnosticsMode: "enabled" }).sqlSemanticDiagnosticsEnabled).toBe(true); + expect(normalizeEditorSettings({ sqlSemanticDiagnosticsMode: "disabled" }).sqlSemanticDiagnosticsEnabled).toBe(false); + }); + + it("migrates legacy SQL semantic diagnostics booleans to explicit modes", () => { + expect(normalizeEditorSettings({ sqlSemanticDiagnosticsEnabled: true } as any).sqlSemanticDiagnosticsMode).toBe("enabled"); + expect(normalizeEditorSettings({ sqlSemanticDiagnosticsEnabled: false } as any).sqlSemanticDiagnosticsMode).toBe("disabled"); + }); + it("defaults update downloads to the official source", () => { expect(normalizeEditorSettings({}).updateDownloadSource).toBe("official"); }); diff --git a/apps/desktop/src/stores/settingsStore.ts b/apps/desktop/src/stores/settingsStore.ts index add867f81..53b3d064a 100644 --- a/apps/desktop/src/stores/settingsStore.ts +++ b/apps/desktop/src/stores/settingsStore.ts @@ -59,8 +59,10 @@ export type DesktopIconTheme = "default" | "black"; export type InterfaceLayout = "separated" | "classic"; export type UpdateDownloadSource = "official" | "cnb"; +export type SqlSemanticDiagnosticsMode = "auto" | "enabled" | "disabled"; export const DEFAULT_SIDEBAR_TABLE_PAGE_SIZE = 1000; +const SQL_SEMANTIC_DIAGNOSTICS_AUTO_ENABLED = false; export const DEFAULT_DESKTOP_SETTINGS: DesktopSettings = { show_tray_icon: true, @@ -330,6 +332,8 @@ export interface EditorSettings { showExecutionTargetPicker: boolean; autoAliasTables: boolean; wordWrap: boolean; + sqlSemanticDiagnosticsMode: SqlSemanticDiagnosticsMode; + sqlSemanticDiagnosticsEnabled: boolean; confirmDangerousSqlExecution: boolean; compactTabTitle: boolean; appLayout: "separated" | "classic"; @@ -436,6 +440,8 @@ export const DEFAULT_EDITOR_SETTINGS: EditorSettings = { showExecutionTargetPicker: false, autoAliasTables: true, wordWrap: false, + sqlSemanticDiagnosticsMode: "auto", + sqlSemanticDiagnosticsEnabled: SQL_SEMANTIC_DIAGNOSTICS_AUTO_ENABLED, confirmDangerousSqlExecution: true, compactTabTitle: false, appLayout: "classic", @@ -514,6 +520,18 @@ function normalizeUpdateDownloadSource(value: unknown): UpdateDownloadSource { return value === "cnb" ? "cnb" : DEFAULT_EDITOR_SETTINGS.updateDownloadSource; } +function normalizeSqlSemanticDiagnosticsMode(value: unknown, legacyEnabled?: unknown): SqlSemanticDiagnosticsMode { + if (value === "auto" || value === "enabled" || value === "disabled") return value; + if (typeof legacyEnabled === "boolean") return legacyEnabled ? "enabled" : "disabled"; + return DEFAULT_EDITOR_SETTINGS.sqlSemanticDiagnosticsMode; +} + +function sqlSemanticDiagnosticsEnabledForMode(mode: SqlSemanticDiagnosticsMode): boolean { + if (mode === "enabled") return true; + if (mode === "disabled") return false; + return SQL_SEMANTIC_DIAGNOSTICS_AUTO_ENABLED; +} + function normalizeDisconnectTabHandlingMode(value: unknown, legacyCloseTabsOnDisconnect?: unknown): DisconnectTabHandlingMode { if (DISCONNECT_TAB_HANDLING_MODES.includes(value as DisconnectTabHandlingMode)) { return value as DisconnectTabHandlingMode; @@ -582,6 +600,7 @@ function normalizeToolbarItems(items: Partial | undefined): Toolba } export function normalizeEditorSettings(settings: Partial, existing?: EditorSettings): EditorSettings { + const sqlSemanticDiagnosticsMode = normalizeSqlSemanticDiagnosticsMode(settings.sqlSemanticDiagnosticsMode, settings.sqlSemanticDiagnosticsEnabled); return { fontFamily: settings.fontFamily ?? DEFAULT_EDITOR_SETTINGS.fontFamily, fontSize: settings.fontSize ?? DEFAULT_EDITOR_SETTINGS.fontSize, @@ -618,6 +637,8 @@ export function normalizeEditorSettings(settings: Partial, exist showExecutionTargetPicker: settings.showExecutionTargetPicker ?? DEFAULT_EDITOR_SETTINGS.showExecutionTargetPicker, autoAliasTables: settings.autoAliasTables ?? DEFAULT_EDITOR_SETTINGS.autoAliasTables, wordWrap: settings.wordWrap ?? DEFAULT_EDITOR_SETTINGS.wordWrap, + sqlSemanticDiagnosticsMode, + sqlSemanticDiagnosticsEnabled: sqlSemanticDiagnosticsEnabledForMode(sqlSemanticDiagnosticsMode), confirmDangerousSqlExecution: settings.confirmDangerousSqlExecution ?? DEFAULT_EDITOR_SETTINGS.confirmDangerousSqlExecution, compactTabTitle: settings.compactTabTitle ?? DEFAULT_EDITOR_SETTINGS.compactTabTitle, appLayout: settings.appLayout ?? DEFAULT_EDITOR_SETTINGS.appLayout, @@ -791,6 +812,11 @@ export const useSettingsStore = defineStore("settings", () => { if (partial.showExecutionTargetPicker !== undefined) editorSettings.value.showExecutionTargetPicker = partial.showExecutionTargetPicker; if (partial.autoAliasTables !== undefined) editorSettings.value.autoAliasTables = partial.autoAliasTables; if (partial.wordWrap !== undefined) editorSettings.value.wordWrap = partial.wordWrap; + if (partial.sqlSemanticDiagnosticsMode !== undefined || partial.sqlSemanticDiagnosticsEnabled !== undefined) { + const nextMode = normalizeSqlSemanticDiagnosticsMode(partial.sqlSemanticDiagnosticsMode, partial.sqlSemanticDiagnosticsEnabled); + editorSettings.value.sqlSemanticDiagnosticsMode = nextMode; + editorSettings.value.sqlSemanticDiagnosticsEnabled = sqlSemanticDiagnosticsEnabledForMode(nextMode); + } if (partial.confirmDangerousSqlExecution !== undefined) editorSettings.value.confirmDangerousSqlExecution = partial.confirmDangerousSqlExecution; if (partial.compactTabTitle !== undefined) editorSettings.value.compactTabTitle = partial.compactTabTitle; if (partial.appLayout !== undefined) editorSettings.value.appLayout = partial.appLayout; diff --git a/apps/desktop/src/types/database.ts b/apps/desktop/src/types/database.ts index ee6c83020..a9c59339d 100644 --- a/apps/desktop/src/types/database.ts +++ b/apps/desktop/src/types/database.ts @@ -448,17 +448,25 @@ export interface SqlTableReference { schema?: string | null; alias?: string | null; span: SqlTextSpan; + scope_id?: number; } export interface SqlColumnReference { name: string; qualifier?: string | null; span: SqlTextSpan; + scope_id?: number; +} + +export interface SqlReferenceScope { + id: number; + parent_id?: number | null; } export interface SqlReferenceAnalysis { tables: SqlTableReference[]; columns: SqlColumnReference[]; + scopes?: SqlReferenceScope[]; } export type TreeNodeType = diff --git a/crates/dbx-core/src/sql_analysis.rs b/crates/dbx-core/src/sql_analysis.rs index 533ce12dc..4a1067ae0 100644 --- a/crates/dbx-core/src/sql_analysis.rs +++ b/crates/dbx-core/src/sql_analysis.rs @@ -27,6 +27,7 @@ static POSTGRES_DEFAULT_PRIVILEGES_RE: LazyLock = LazyLock::new(|| { pub struct SqlReferenceAnalysis { pub tables: Vec, pub columns: Vec, + pub scopes: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -35,6 +36,7 @@ pub struct SqlTableReference { pub schema: Option, pub alias: Option, pub span: SqlTextSpan, + pub scope_id: usize, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -42,6 +44,13 @@ pub struct SqlColumnReference { pub name: String, pub qualifier: Option, pub span: SqlTextSpan, + pub scope_id: usize, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SqlReferenceScope { + pub id: usize, + pub parent_id: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -67,15 +76,18 @@ impl From for SqlTextSpan { struct Analyzer { tables: Vec, columns: Vec, + scopes: Vec, + scope_stack: Vec, + next_scope_id: usize, } pub fn analyze_sql_references(sql: &str, dialect: Option<&str>) -> Result { let normalized_dialect = normalize_dialect(dialect); if normalized_dialect == "duckdb" && starts_with_duckdb_parser_gap_sql(sql) { - return Ok(SqlReferenceAnalysis { tables: vec![], columns: vec![] }); + return Ok(SqlReferenceAnalysis { tables: vec![], columns: vec![], scopes: vec![] }); } if normalized_dialect == "postgres" && starts_with_postgres_parser_gap_sql(sql) { - return Ok(SqlReferenceAnalysis { tables: vec![], columns: vec![] }); + return Ok(SqlReferenceAnalysis { tables: vec![], columns: vec![], scopes: vec![] }); } let parser_sql = if normalized_dialect == "clickhouse" { normalize_clickhouse_join_order_for_parser(sql) @@ -99,7 +111,7 @@ pub fn analyze_sql_references(sql: &str, dialect: Option<&str>) -> Result bool { @@ -147,14 +159,31 @@ fn normalize_dialect(dialect: Option<&str>) -> String { impl Analyzer { fn visit_statement(&mut self, statement: &Statement) { if let Statement::Query(query) = statement { - self.visit_query(query); + self.visit_query_in_new_scope(query, None); } } + fn visit_query_in_new_scope(&mut self, query: &Query, parent_id: Option) { + let scope_id = self.next_scope_id; + self.next_scope_id += 1; + self.scopes.push(SqlReferenceScope { id: scope_id, parent_id }); + self.scope_stack.push(scope_id); + self.visit_query(query); + self.scope_stack.pop(); + } + + fn visit_child_query(&mut self, query: &Query) { + self.visit_query_in_new_scope(query, self.current_scope_id()); + } + + fn current_scope_id(&self) -> Option { + self.scope_stack.last().copied() + } + fn visit_query(&mut self, query: &Query) { if let Some(with) = &query.with { for cte in &with.cte_tables { - self.visit_query(&cte.query); + self.visit_child_query(&cte.query); } } self.visit_set_expr(&query.body); @@ -170,15 +199,24 @@ impl Analyzer { fn visit_set_expr(&mut self, set_expr: &SetExpr) { match set_expr { SetExpr::Select(select) => self.visit_select(select), - SetExpr::Query(query) => self.visit_query(query), + SetExpr::Query(query) => self.visit_child_query(query), SetExpr::SetOperation { left, right, .. } => { - self.visit_set_expr(left); - self.visit_set_expr(right); + self.visit_set_expr_in_child_scope(left); + self.visit_set_expr_in_child_scope(right); } _ => {} } } + fn visit_set_expr_in_child_scope(&mut self, set_expr: &SetExpr) { + let scope_id = self.next_scope_id; + self.next_scope_id += 1; + self.scopes.push(SqlReferenceScope { id: scope_id, parent_id: self.current_scope_id() }); + self.scope_stack.push(scope_id); + self.visit_set_expr(set_expr); + self.scope_stack.pop(); + } + fn visit_select(&mut self, select: &Select) { for table in &select.from { self.visit_table_with_joins(table); @@ -272,12 +310,16 @@ impl Analyzer { match factor { TableFactor::Table { name, alias, args, .. } => { if args.is_none() { - if let Some(table) = table_reference_from_name(name, alias.as_ref().map(|a| a.name.value.clone())) { + if let Some(table) = table_reference_from_name( + name, + alias.as_ref().map(|a| a.name.value.clone()), + self.current_scope_id(), + ) { self.tables.push(table); } } } - TableFactor::Derived { subquery, .. } => self.visit_query(subquery), + TableFactor::Derived { subquery, .. } => self.visit_child_query(subquery), TableFactor::NestedJoin { table_with_joins, .. } => self.visit_table_with_joins(table_with_joins), TableFactor::TableFunction { expr, .. } => self.visit_expr(expr), TableFactor::Function { args, .. } => { @@ -383,14 +425,14 @@ impl Analyzer { self.visit_expr(else_result); } } - Expr::Subquery(query) | Expr::Exists { subquery: query, .. } => self.visit_query(query), + Expr::Subquery(query) | Expr::Exists { subquery: query, .. } => self.visit_child_query(query), _ => {} } } fn visit_function_args(&mut self, args: &FunctionArguments) { match args { - FunctionArguments::Subquery(query) => self.visit_query(query), + FunctionArguments::Subquery(query) => self.visit_child_query(query), FunctionArguments::List(list) => { for arg in &list.args { self.visit_function_arg(arg); @@ -418,16 +460,27 @@ impl Analyzer { } fn push_column(&mut self, qualifier: Option, ident: &Ident) { - self.columns.push(SqlColumnReference { name: ident.value.clone(), qualifier, span: ident.span.into() }); + if let Some(scope_id) = self.current_scope_id() { + self.columns.push(SqlColumnReference { + name: ident.value.clone(), + qualifier, + span: ident.span.into(), + scope_id, + }); + } } } -fn table_reference_from_name(name: &ObjectName, alias: Option) -> Option { +fn table_reference_from_name( + name: &ObjectName, + alias: Option, + scope_id: Option, +) -> Option { let parts: Vec<&Ident> = name.0.iter().filter_map(ObjectNamePart::as_ident).collect(); let table = parts.last()?; let schema = if parts.len() >= 2 { parts.get(parts.len() - 2).map(|ident| ident.value.clone()) } else { None }; - Some(SqlTableReference { name: table.value.clone(), schema, alias, span: table.span.into() }) + Some(SqlTableReference { name: table.value.clone(), schema, alias, span: table.span.into(), scope_id: scope_id? }) } fn object_name_last_ident(name: &ObjectName) -> Option<&Ident> { diff --git a/crates/dbx-core/tests/sql_analysis.rs b/crates/dbx-core/tests/sql_analysis.rs index 8530fd025..f32acfd94 100644 --- a/crates/dbx-core/tests/sql_analysis.rs +++ b/crates/dbx-core/tests/sql_analysis.rs @@ -13,6 +13,26 @@ fn extracts_tables_aliases_and_qualified_columns() { assert_eq!(columns, vec![(Some("u"), "missing"), (Some("u"), "id")]); } +#[test] +fn extracts_nested_query_scopes_for_correlated_subqueries() { + let sql = "select aa.house_id from mds_base_house aa where exists (select 1 from mds_base_owner where HOUSE_ID = aa.HOUSE_ID)"; + let analysis = analyze_sql_references(sql, Some("mysql")).unwrap(); + + let tables: Vec<_> = + analysis.tables.iter().map(|table| (table.name.as_str(), table.alias.as_deref(), table.scope_id)).collect(); + assert_eq!(tables, vec![("mds_base_house", Some("aa"), 0), ("mds_base_owner", None, 1)]); + + let scopes: Vec<_> = analysis.scopes.iter().map(|scope| (scope.id, scope.parent_id)).collect(); + assert_eq!(scopes, vec![(0, None), (1, Some(0))]); + + let columns: Vec<_> = analysis + .columns + .iter() + .map(|column| (column.qualifier.as_deref(), column.name.as_str(), column.scope_id)) + .collect(); + assert_eq!(columns, vec![(Some("aa"), "house_id", 0), (None, "HOUSE_ID", 1), (Some("aa"), "HOUSE_ID", 1)]); +} + #[test] fn extracts_unqualified_columns_from_single_table_select() { let analysis = analyze_sql_references("select missing, id from users", Some("postgres")).unwrap(); diff --git a/packages/app-tests/sqlSemanticDiagnostics.test.ts b/packages/app-tests/sqlSemanticDiagnostics.test.ts index b948a493c..975b02e29 100644 --- a/packages/app-tests/sqlSemanticDiagnostics.test.ts +++ b/packages/app-tests/sqlSemanticDiagnostics.test.ts @@ -50,6 +50,22 @@ test("flags confirmed missing tables", () => { assert.equal(diagnostics[0]?.severity, "error"); }); +test("trims whitespace from missing table diagnostic spans", () => { + const analysis: SqlReferenceAnalysis = { + tables: [{ name: "t_00011", span: span(32, 39) }], + columns: [], + }; + + const diagnostics = buildSqlSemanticDiagnostics(analysis, { + tables: [], + columnsByTable: new Map(), + missingTables: new Set(["t_00011"]), + sql: "SELECT * FROM demo_2000_tables.t_00011 AS t0", + }); + + assert.deepEqual(diagnostics[0]?.span, span(32, 38)); +}); + test("flags missing columns when column metadata is cached with a schema key", () => { const analysis: SqlReferenceAnalysis = { tables: [{ name: "t_10001", span: span(24, 32) }], @@ -165,6 +181,88 @@ test("resolves qualified aliases against the table in the same statement", () => ); }); +test("resolves correlated subquery columns against the visible query scopes", () => { + const analysis: SqlReferenceAnalysis = { + tables: [ + { name: "mds_base_house", alias: "aa", span: span(1, 14), scope_id: 0 }, + { name: "mds_base_owner", span: span(1, 14), scope_id: 1 }, + { name: "FDS_PAY_ORDER", span: span(1, 13), scope_id: 2 }, + { name: "ac_fund_acct", span: span(1, 12), scope_id: 3 }, + ], + columns: [ + { name: "house_id", qualifier: "aa", span: span(8, 15), scope_id: 0 }, + { name: "hou_add", qualifier: "aa", span: span(18, 24), scope_id: 0 }, + { name: "contract_code", qualifier: "aa", span: span(27, 39), scope_id: 0 }, + { name: "pay_type", qualifier: "aa", span: span(42, 49), scope_id: 0 }, + { name: "HOU_PAY_AMT", qualifier: "aa", span: span(52, 62), scope_id: 0 }, + { name: "OWNER_NAME", span: span(8, 17), scope_id: 1 }, + { name: "HOUSE_ID", span: span(8, 15), scope_id: 1 }, + { name: "HOUSE_ID", qualifier: "aa", span: span(8, 15), scope_id: 1 }, + { name: "PAY_DATA", span: span(8, 15), scope_id: 2 }, + { name: "contract_code", qualifier: "aa", span: span(8, 20), scope_id: 2 }, + { name: "owner_id", span: span(8, 15), scope_id: 3 }, + { name: "house_id", qualifier: "aa", span: span(8, 15), scope_id: 3 }, + ], + scopes: [ + { id: 0, parent_id: null }, + { id: 1, parent_id: 0 }, + { id: 2, parent_id: 0 }, + { id: 3, parent_id: 0 }, + ], + }; + + const diagnostics = buildSqlSemanticDiagnostics(analysis, { + tables: [ + { name: "mds_base_house", type: "table" }, + { name: "mds_base_owner", type: "table" }, + { name: "FDS_PAY_ORDER", type: "table" }, + { name: "ac_fund_acct", type: "table" }, + ], + columnsByTable: new Map([ + ["mds_base_house", ["house_id", "hou_add", "contract_code", "pay_type", "HOU_PAY_AMT"].map((name) => ({ name, table: "mds_base_house" }))], + ["mds_base_owner", ["OWNER_NAME", "HOUSE_ID"].map((name) => ({ name, table: "mds_base_owner" }))], + ["FDS_PAY_ORDER", [{ name: "PAY_DATA", table: "FDS_PAY_ORDER" }]], + ["ac_fund_acct", [{ name: "owner_id", table: "ac_fund_acct" }]], + ]), + }); + + assert.deepEqual(diagnostics, []); +}); + +test("keeps missing-column diagnostics inside nested query scopes", () => { + const analysis: SqlReferenceAnalysis = { + tables: [ + { name: "parent_table", alias: "p", span: span(1, 12), scope_id: 0 }, + { name: "child_table", alias: "c", span: span(1, 11), scope_id: 1 }, + ], + columns: [ + { name: "id", qualifier: "p", span: span(8, 9), scope_id: 0 }, + { name: "missing", qualifier: "c", span: span(8, 15), scope_id: 1 }, + { name: "id", qualifier: "p", span: span(8, 9), scope_id: 1 }, + ], + scopes: [ + { id: 0, parent_id: null }, + { id: 1, parent_id: 0 }, + ], + }; + + const diagnostics = buildSqlSemanticDiagnostics(analysis, { + tables: [ + { name: "parent_table", type: "table" }, + { name: "child_table", type: "table" }, + ], + columnsByTable: new Map([ + ["parent_table", [{ name: "id", table: "parent_table" }]], + ["child_table", [{ name: "id", table: "child_table" }]], + ]), + }); + + assert.deepEqual( + diagnostics.map((diagnostic) => diagnostic.message), + ["Unknown column c.missing"], + ); +}); + test("does not flag unqualified columns when multiple tables make ownership ambiguous", () => { const analysis: SqlReferenceAnalysis = { tables: [