From 970f8ce605edd5f49cb7b690a8926b21c4c3c998 Mon Sep 17 00:00:00 2001 From: Matheus Tonon Date: Thu, 28 May 2026 14:17:06 -0300 Subject: [PATCH 1/8] fix(autocomplete): resolve aliased PostgreSQL quoted table columns correctly --- src/utils/autocomplete.ts | 18 +++++++---- src/utils/sqlAnalysis.ts | 54 ++++++++++++++++++++++----------- tests/utils/sqlAnalysis.test.ts | 16 ++++++++++ 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/utils/autocomplete.ts b/src/utils/autocomplete.ts index 40807522..a24e9985 100644 --- a/src/utils/autocomplete.ts +++ b/src/utils/autocomplete.ts @@ -98,6 +98,10 @@ export const clearAutocompleteCache = (connectionId?: string) => { } }; +// Find a table by name in the list of tables +const findTableByName = (name: string, tables: TableInfo[]) => + tables.find((t) => t.name.toLowerCase() === name.toLowerCase())?.name; + export const registerSqlAutocomplete = ( monaco: Monaco, connectionId: string | null, @@ -141,13 +145,13 @@ export const registerSqlAutocomplete = ( // Check if it's an alias or table name let actualTableName = tableAliases?.get(typedName); - + if (!actualTableName) { - // Try direct table name match - const foundTable = tables.find(t => t.name.toLowerCase() === typedName); - actualTableName = foundTable?.name; + actualTableName = findTableByName(typedName, tables); + } else { + actualTableName = findTableByName(actualTableName, tables) ?? actualTableName; } - + if (actualTableName) { const columns = await getTableColumns(connectionId, actualTableName, schema); @@ -170,6 +174,8 @@ export const registerSqlAutocomplete = ( return { suggestions }; } + + return { suggestions: [] }; } // ============================================ @@ -188,7 +194,7 @@ export const registerSqlAutocomplete = ( // User is inside a query with FROM/JOIN - suggest columns from those tables const tableNames = Array.from(new Set(tableAliases.values())); const matchingTables = tableNames - .map(name => tables.find(t => t.name.toLowerCase() === name.toLowerCase())) + .map((name) => tables.find((t) => t.name.toLowerCase() === name.toLowerCase())) .filter(Boolean) as TableInfo[]; // Limit parallel fetches to prevent memory spikes diff --git a/src/utils/sqlAnalysis.ts b/src/utils/sqlAnalysis.ts index d38a5ccb..4da6e684 100644 --- a/src/utils/sqlAnalysis.ts +++ b/src/utils/sqlAnalysis.ts @@ -1,49 +1,69 @@ // SQL Analysis Utilities - Pure logic functions for parsing and analyzing SQL +// Removes wrapping SQL identifier quotes/backticks. +// Unquoted identifiers are normalized to lowercase. +function stripIdentifierQuotes(token: string): string { + const q = token[0]; + if (q === '"' || q === '`') return token.slice(1, -1); + return token.toLowerCase(); +} + // Optimized table parser - early exit and minimal allocations export const parseTablesFromQuery = (sql: string): Map | null => { if (!sql || sql.length === 0) return null; - + const lowerSql = sql.toLowerCase(); - + // Quick check if query contains FROM/JOIN keywords if (!lowerSql.includes('from') && !lowerSql.includes('join')) { return null; } - + + // Only scan FROM clause onward (avoids SELECT-list commas; keeps quoted case) + const fromAt = lowerSql.search(/\bfrom\b/); + const scan = fromAt >= 0 ? sql.slice(fromAt) : sql; + const tableMap = new Map(); - const fromPattern = /(?:from|join)\s+(?:`)?([a-z_][a-z0-9_]*)(?:`)?(?:\s+(?:as\s+)?(?:`)?([a-z_][a-z0-9_]*)(?:`)?)?/gi; - + const fromPattern = + /(?:from|join|,)\s+("(?:[^"]|"")*"|`[^`]+`|[a-zA-Z_][a-zA-Z0-9_]*)(?:\.("(?:[^"]|"")*"|`[^`]+`|[a-zA-Z_][a-zA-Z0-9_]*))?(?:\s+(?:as\s+)?("(?:[^"]|"")*"|`[^`]+`|[a-zA-Z_][a-zA-Z0-9_]*))?/gi; + let match; let matchCount = 0; const MAX_MATCHES = 10; // Prevent regex catastrophic backtracking - - while ((match = fromPattern.exec(lowerSql)) !== null && matchCount++ < MAX_MATCHES) { - const tableName = match[1]; - const alias = match[2] || tableName; - tableMap.set(alias, tableName); + + while ((match = fromPattern.exec(scan)) !== null && matchCount++ < MAX_MATCHES) { + const tableToken = match[2] ?? match[1]; + + if (!tableToken) continue; + + const tableName = stripIdentifierQuotes(tableToken); + const aliasToken = match[3]; + const alias = aliasToken ? stripIdentifierQuotes(aliasToken) : tableName; + + tableMap.set(alias.toLowerCase(), tableName); } - + return tableMap.size > 0 ? tableMap : null; }; // Optimized statement extractor - avoid full text scan when possible export const getCurrentStatement = (model: { getValue: () => string; getOffsetAt: (position: { lineNumber: number; column: number }) => number }, position: { lineNumber: number; column: number }): string => { const fullText = model.getValue(); - + // For small files, just return full text if (fullText.length < 500) { return fullText; } - + const offset = model.getOffsetAt(position); let start = 0; let end = fullText.length; - + + // Search within reasonable bounds (±2000 chars from cursor) const searchStart = Math.max(0, offset - 2000); const searchEnd = Math.min(fullText.length, offset + 2000); - + // Find previous semicolon for (let i = offset - 1; i >= searchStart; i--) { if (fullText[i] === ';') { @@ -51,7 +71,7 @@ export const getCurrentStatement = (model: { getValue: () => string; getOffsetAt break; } } - + // Find next semicolon for (let i = offset; i < searchEnd; i++) { if (fullText[i] === ';') { @@ -59,6 +79,6 @@ export const getCurrentStatement = (model: { getValue: () => string; getOffsetAt break; } } - + return fullText.substring(start, end).trim(); }; diff --git a/tests/utils/sqlAnalysis.test.ts b/tests/utils/sqlAnalysis.test.ts index 150689d9..889c81a9 100644 --- a/tests/utils/sqlAnalysis.test.ts +++ b/tests/utils/sqlAnalysis.test.ts @@ -48,6 +48,22 @@ describe('sqlAnalysis utils', () => { expect(result?.get('u')).toBe('users'); expect(result?.get('p')).toBe('products'); }); + + it('should extract PostgreSQL double-quoted table with alias', () => { + const result = parseTablesFromQuery('SELECT ael. FROM "AccountEventLog" ael'); + expect(result?.get('ael')).toBe('AccountEventLog'); + }); + + it('should extract schema-qualified table with alias', () => { + const result = parseTablesFromQuery('SELECT u. FROM public.users u'); + expect(result?.get('u')).toBe('users'); + }); + + it('should extract comma-separated FROM tables', () => { + const result = parseTablesFromQuery('SELECT * FROM users u, orders o'); + expect(result?.get('u')).toBe('users'); + expect(result?.get('o')).toBe('orders'); + }); }); describe('getCurrentStatement', () => { From c38bd3ea2092cc444c8572a97e2ce59ac27aaade Mon Sep 17 00:00:00 2001 From: Matheus Tonon Date: Thu, 28 May 2026 14:30:41 -0300 Subject: [PATCH 2/8] feat(sql-autocomplete): integrate SQL autocomplete registration into NotebookView and Editor components --- src/components/notebook/NotebookView.tsx | 7 ++ src/contexts/DatabaseProvider.tsx | 4 +- src/hooks/useSqlAutocompleteRegistration.ts | 89 +++++++++++++++++++++ src/pages/Editor.tsx | 28 ++----- src/utils/autocomplete.ts | 26 +++++- tests/utils/autocomplete.test.ts | 71 ++++++++++++++++ 6 files changed, 200 insertions(+), 25 deletions(-) create mode 100644 src/hooks/useSqlAutocompleteRegistration.ts diff --git a/src/components/notebook/NotebookView.tsx b/src/components/notebook/NotebookView.tsx index 0336a8cb..f1f53c8b 100644 --- a/src/components/notebook/NotebookView.tsx +++ b/src/components/notebook/NotebookView.tsx @@ -44,6 +44,7 @@ import { createNotebookFromState, } from "../../utils/notebookStore"; import { useDatabase } from "../../hooks/useDatabase"; +import { useSqlAutocompleteRegistration } from "../../hooks/useSqlAutocompleteRegistration"; import { isMultiDatabaseCapable } from "../../utils/database"; import { useSettings } from "../../hooks/useSettings"; import { useAlert } from "../../hooks/useAlert"; @@ -59,12 +60,14 @@ interface NotebookViewProps { tab: Tab; updateTab: (id: string, partial: Partial) => void; connectionId: string; + isActive: boolean; } export function NotebookView({ tab, updateTab, connectionId, + isActive, }: NotebookViewProps) { const { t } = useTranslation(); const { activeSchema, activeCapabilities, selectedDatabases } = useDatabase(); @@ -72,6 +75,10 @@ export function NotebookView({ isMultiDatabaseCapable(activeCapabilities) && selectedDatabases.length > 1; const effectiveSchema = tab.schema || activeSchema || (isMultiDb ? selectedDatabases[0] : null); + useSqlAutocompleteRegistration(connectionId, { + schema: effectiveSchema, + enabled: isActive, + }); const { settings } = useSettings(); const { showAlert } = useAlert(); const { matchesShortcut } = useKeybindings(); diff --git a/src/contexts/DatabaseProvider.tsx b/src/contexts/DatabaseProvider.tsx index a6620839..eb3fcf20 100644 --- a/src/contexts/DatabaseProvider.tsx +++ b/src/contexts/DatabaseProvider.tsx @@ -14,7 +14,7 @@ import { } from './DatabaseContext'; import type { ReactNode } from 'react'; import type { PluginManifest } from '../types/plugins'; -import { clearAutocompleteCache } from '../utils/autocomplete'; +import { clearAutocompleteCache, disposeSqlAutocomplete } from '../utils/autocomplete'; import { toErrorMessage } from '../utils/errors'; import { useSettings } from '../hooks/useSettings'; import { findConnectionsForDrivers } from '../utils/connectionManager'; @@ -691,6 +691,7 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { if (!targetId) return; clearAutocompleteCache(targetId); + disposeSqlAutocomplete(); try { await invoke('disconnect_connection', { connectionId: targetId }); @@ -782,6 +783,7 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { console.warn(`[DatabaseProvider] Connection health check failed for ${connectionId}: ${event.payload.error}`); clearAutocompleteCache(connectionId); + disposeSqlAutocomplete(); setOpenConnectionIds(prev => prev.filter(id => id !== connectionId)); setConnectionDataMap(prev => { diff --git a/src/hooks/useSqlAutocompleteRegistration.ts b/src/hooks/useSqlAutocompleteRegistration.ts new file mode 100644 index 00000000..1f3e476f --- /dev/null +++ b/src/hooks/useSqlAutocompleteRegistration.ts @@ -0,0 +1,89 @@ +import { useEffect } from "react"; +import type { Monaco } from "@monaco-editor/react"; +import { loader } from "@monaco-editor/react"; +import { useDatabase } from "./useDatabase"; +import { isMultiDatabaseCapable } from "../utils/database"; +import { registerSqlAutocomplete } from "../utils/autocomplete"; + +type Options = { + monaco?: Monaco | null; + schema?: string | null; + /** When false, skips registration (e.g. inactive notebook tabs). Defaults to true. */ + enabled?: boolean; +}; + +/** + * Keeps the global SQL completion provider in sync with the active connection. + * Pass `monaco` from the main editor when available; otherwise Monaco is loaded via loader.init (notebook). + */ +export function useSqlAutocompleteRegistration( + connectionId: string | null, + options?: Options, +) { + const { + tables, + activeDriver, + activeSchema, + activeCapabilities, + schemaDataMap, + databaseDataMap, + selectedDatabases, + } = useDatabase(); + + const schema = options?.schema ?? activeSchema; + const isMultiDb = + isMultiDatabaseCapable(activeCapabilities) && selectedDatabases.length > 1; + + const enabled = options?.enabled ?? true; + + useEffect(() => { + if (!connectionId || !enabled) return; + + let cancelled = false; + + const register = (monaco: Monaco) => { + if (cancelled) return; + + let effectiveTables = tables; + if (activeCapabilities?.schemas && schema) { + effectiveTables = schemaDataMap[schema]?.tables ?? tables; + } else if (isMultiDb) { + effectiveTables = selectedDatabases.flatMap( + (db) => databaseDataMap[db]?.tables ?? [], + ); + } + + registerSqlAutocomplete( + monaco, + connectionId, + effectiveTables, + schema, + activeDriver ?? null, + ); + }; + + if (options?.monaco) { + register(options.monaco); + return () => { + cancelled = true; + }; + } + + loader.init().then((monaco) => register(monaco)); + return () => { + cancelled = true; + }; + }, [ + connectionId, + enabled, + options?.monaco, + schema, + tables, + activeDriver, + activeCapabilities, + schemaDataMap, + databaseDataMap, + isMultiDb, + selectedDatabases, + ]); +} diff --git a/src/pages/Editor.tsx b/src/pages/Editor.tsx index 6f226067..8dfda8bc 100644 --- a/src/pages/Editor.tsx +++ b/src/pages/Editor.tsx @@ -82,7 +82,7 @@ import { SqlEditorWrapper } from "../components/ui/SqlEditorWrapper"; import { NotebookView } from "../components/notebook/NotebookView"; import { extractSqlFromCells } from "../utils/notebook"; import { createNotebook } from "../utils/notebookStore"; -import { registerSqlAutocomplete } from "../utils/autocomplete"; +import { useSqlAutocompleteRegistration } from "../hooks/useSqlAutocompleteRegistration"; import { type OnMount, type Monaco } from "@monaco-editor/react"; import { save } from "@tauri-apps/plugin-dialog"; import { useAlert } from "../hooks/useAlert"; @@ -137,7 +137,6 @@ export const Editor = () => { const { t } = useTranslation(); const { activeConnectionId, - tables, views, activeDriver, activeSchema, @@ -145,8 +144,6 @@ export const Editor = () => { selectedDatabases, activeConnectionName, activeDatabaseName, - schemaDataMap, - databaseDataMap, } = useDatabase(); const { explorerConnectionId } = useConnectionLayoutContext(); const { settings } = useSettings(); @@ -2136,23 +2133,11 @@ export const Editor = () => { }); }; - useEffect(() => { - if (monacoInstance && activeConnectionId) { - let effectiveTables = tables; - if (activeCapabilities?.schemas && activeSchema) { - effectiveTables = schemaDataMap[activeSchema]?.tables ?? tables; - } else if (isMultiDb) { - effectiveTables = selectedDatabases.flatMap(db => databaseDataMap[db]?.tables ?? []); - } - const disposable = registerSqlAutocomplete( - monacoInstance, - activeConnectionId, - effectiveTables, - activeSchema, - ); - return () => disposable.dispose(); - } - }, [monacoInstance, activeConnectionId, tables, activeSchema, activeCapabilities, schemaDataMap, databaseDataMap, isMultiDb, selectedDatabases]); + useSqlAutocompleteRegistration(activeConnectionId, { + monaco: monacoInstance, + schema: activeSchema, + enabled: !isNotebookTab, + }); useEffect(() => { const state = location.state as EditorState; @@ -2742,6 +2727,7 @@ export const Editor = () => { tab={tab} updateTab={updateTab} connectionId={activeConnectionId || ""} + isActive={isActive} /> ); diff --git a/src/utils/autocomplete.ts b/src/utils/autocomplete.ts index a24e9985..34c5f31b 100644 --- a/src/utils/autocomplete.ts +++ b/src/utils/autocomplete.ts @@ -1,6 +1,7 @@ import type { Monaco } from "@monaco-editor/react"; import { invoke } from "@tauri-apps/api/core"; import type { TableInfo } from "../contexts/DatabaseContext"; +import { formatSqlIdentifier, quoteTableRef } from "./identifiers"; import { getCurrentStatement, parseTablesFromQuery } from "./sqlAnalysis"; // Lightweight column cache with TTL and size limits @@ -102,11 +103,28 @@ export const clearAutocompleteCache = (connectionId?: string) => { const findTableByName = (name: string, tables: TableInfo[]) => tables.find((t) => t.name.toLowerCase() === name.toLowerCase())?.name; +const tableInsertText = ( + tableName: string, + driver?: string | null, + schema?: string | null, +) => + schema + ? quoteTableRef(tableName, driver, schema) + : formatSqlIdentifier(tableName, driver); + +let sqlCompletionProvider: { dispose: () => void } | null = null; + +export const disposeSqlAutocomplete = (): void => { + sqlCompletionProvider?.dispose(); + sqlCompletionProvider = null; +}; + export const registerSqlAutocomplete = ( monaco: Monaco, connectionId: string | null, tables: TableInfo[], schema?: string | null, + driver?: string | null, ) => { const provider = monaco.languages.registerCompletionItemProvider("sql", { triggerCharacters: [".", " "], @@ -167,7 +185,7 @@ export const registerSqlAutocomplete = ( label: c.label, kind: monaco.languages.CompletionItemKind.Field, detail: c.detail, - insertText: c.label, + insertText: formatSqlIdentifier(c.label, driver), range: columnRange, sortText: `0_${c.label}`, })); @@ -229,7 +247,7 @@ export const registerSqlAutocomplete = ( label: col.label, kind: monaco.languages.CompletionItemKind.Field, detail: `${col.detail} — ${table.name}${aliasHint}`, - insertText: col.label, + insertText: formatSqlIdentifier(col.label, driver), range, sortText: `0_${col.label}`, }); @@ -265,7 +283,7 @@ export const registerSqlAutocomplete = ( label: t.name, kind: monaco.languages.CompletionItemKind.Class, detail: "Table", - insertText: t.name, + insertText: tableInsertText(t.name, driver, schema), range, sortText: `1_${t.name}` })); @@ -280,5 +298,7 @@ export const registerSqlAutocomplete = ( }, }); + sqlCompletionProvider?.dispose(); + sqlCompletionProvider = provider; return provider; }; diff --git a/tests/utils/autocomplete.test.ts b/tests/utils/autocomplete.test.ts index 2d85e7d6..647f917c 100644 --- a/tests/utils/autocomplete.test.ts +++ b/tests/utils/autocomplete.test.ts @@ -151,6 +151,50 @@ describe('autocomplete', () => { expect(tableSuggestions[1].label).toBe('orders'); }); + it('inserts double-quoted table names for postgres', async () => { + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'AccountEventLog' }], + null, + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + const result = await provider.provideCompletionItems( + createMockModel('SELECT * FROM '), + { lineNumber: 1, column: 15 }, + ); + + const tableSuggestions = result.suggestions.filter((s: { sortText?: string }) => + s.sortText?.startsWith('1_'), + ); + expect(tableSuggestions[0]?.insertText).toBe('"AccountEventLog"'); + }); + + it('inserts schema-qualified table names for postgres when schema is set', async () => { + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'AccountEventLog' }], + 'public', + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + const result = await provider.provideCompletionItems( + createMockModel('SELECT * FROM '), + { lineNumber: 1, column: 15 }, + ); + + const tableSuggestions = result.suggestions.filter((s: { sortText?: string }) => + s.sortText?.startsWith('1_'), + ); + expect(tableSuggestions[0]?.insertText).toBe('"public"."AccountEventLog"'); + }); + it('should include all table suggestions regardless of count', async () => { const monaco = createMockMonaco(); const tables: TableInfo[] = Array.from({ length: 60 }, (_, i) => ({ @@ -321,6 +365,33 @@ describe('autocomplete', () => { // Should include column suggestions expect(result.suggestions.length).toBeGreaterThan(0); }); + + it('inserts double-quoted column names for postgres', async () => { + const mockInvoke = invoke as unknown as ReturnType; + mockInvoke.mockResolvedValue([{ name: 'CreatedAt', data_type: 'timestamp' }]); + + const { parseTablesFromQuery } = await import('../../src/utils/sqlAnalysis'); + (parseTablesFromQuery as ReturnType).mockReturnValue( + new Map([['ael', 'AccountEventLog']]), + ); + + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'AccountEventLog' }], + 'public', + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + const model = createMockModel('SELECT ael.'); + model.getValueInRange = vi.fn(() => 'SELECT ael.'); + + const result = await provider.provideCompletionItems(model, { lineNumber: 1, column: 12 }); + + expect(result.suggestions[0]?.insertText).toBe('"CreatedAt"'); + }); }); describe('suggestion limits', () => { From d04434d8fccbede167427bdf1c0c74bdc054fa13 Mon Sep 17 00:00:00 2001 From: Matheus Tonon Date: Thu, 28 May 2026 15:42:30 -0300 Subject: [PATCH 3/8] feat(autocomplete): add disposeSqlAutocomplete mock for testing --- tests/contexts/DatabaseProvider.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/contexts/DatabaseProvider.test.tsx b/tests/contexts/DatabaseProvider.test.tsx index adc92e4b..b3e6dc33 100644 --- a/tests/contexts/DatabaseProvider.test.tsx +++ b/tests/contexts/DatabaseProvider.test.tsx @@ -15,6 +15,7 @@ vi.mock('@tauri-apps/api/event', () => ({ vi.mock('../../src/utils/autocomplete', () => ({ clearAutocompleteCache: vi.fn(), + disposeSqlAutocomplete: vi.fn(), })); vi.mock('../../src/hooks/useSettings', () => ({ From cbb028c86826776bd18c56640e9c2c11e146315b Mon Sep 17 00:00:00 2001 From: Matheus Tonon Date: Sun, 14 Jun 2026 18:26:22 -0300 Subject: [PATCH 4/8] refactor(autocomplete): remove disposeSqlAutocomplete calls and update identifier formatting for PostgreSQL --- src/contexts/DatabaseProvider.tsx | 4 +- src/hooks/useSqlAutocompleteRegistration.ts | 15 +++--- src/utils/autocomplete.ts | 13 +---- src/utils/identifiers.ts | 17 +++++-- tests/contexts/DatabaseProvider.test.tsx | 1 - tests/utils/autocomplete.test.ts | 53 ++++++++++++++++++++- tests/utils/filterBar.test.ts | 22 ++++++--- tests/utils/identifiers.test.ts | 25 +++++++++- 8 files changed, 115 insertions(+), 35 deletions(-) diff --git a/src/contexts/DatabaseProvider.tsx b/src/contexts/DatabaseProvider.tsx index eb3fcf20..a6620839 100644 --- a/src/contexts/DatabaseProvider.tsx +++ b/src/contexts/DatabaseProvider.tsx @@ -14,7 +14,7 @@ import { } from './DatabaseContext'; import type { ReactNode } from 'react'; import type { PluginManifest } from '../types/plugins'; -import { clearAutocompleteCache, disposeSqlAutocomplete } from '../utils/autocomplete'; +import { clearAutocompleteCache } from '../utils/autocomplete'; import { toErrorMessage } from '../utils/errors'; import { useSettings } from '../hooks/useSettings'; import { findConnectionsForDrivers } from '../utils/connectionManager'; @@ -691,7 +691,6 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { if (!targetId) return; clearAutocompleteCache(targetId); - disposeSqlAutocomplete(); try { await invoke('disconnect_connection', { connectionId: targetId }); @@ -783,7 +782,6 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { console.warn(`[DatabaseProvider] Connection health check failed for ${connectionId}: ${event.payload.error}`); clearAutocompleteCache(connectionId); - disposeSqlAutocomplete(); setOpenConnectionIds(prev => prev.filter(id => id !== connectionId)); setConnectionDataMap(prev => { diff --git a/src/hooks/useSqlAutocompleteRegistration.ts b/src/hooks/useSqlAutocompleteRegistration.ts index 1f3e476f..6456a6ad 100644 --- a/src/hooks/useSqlAutocompleteRegistration.ts +++ b/src/hooks/useSqlAutocompleteRegistration.ts @@ -3,7 +3,7 @@ import type { Monaco } from "@monaco-editor/react"; import { loader } from "@monaco-editor/react"; import { useDatabase } from "./useDatabase"; import { isMultiDatabaseCapable } from "../utils/database"; -import { registerSqlAutocomplete } from "../utils/autocomplete"; +import { registerSqlAutocomplete, disposeSqlAutocomplete } from "../utils/autocomplete"; type Options = { monaco?: Monaco | null; @@ -62,17 +62,18 @@ export function useSqlAutocompleteRegistration( ); }; + const cleanup = () => { + cancelled = true; + disposeSqlAutocomplete(); + }; + if (options?.monaco) { register(options.monaco); - return () => { - cancelled = true; - }; + return cleanup; } loader.init().then((monaco) => register(monaco)); - return () => { - cancelled = true; - }; + return cleanup; }, [ connectionId, enabled, diff --git a/src/utils/autocomplete.ts b/src/utils/autocomplete.ts index 34c5f31b..8189a0d9 100644 --- a/src/utils/autocomplete.ts +++ b/src/utils/autocomplete.ts @@ -1,7 +1,7 @@ import type { Monaco } from "@monaco-editor/react"; import { invoke } from "@tauri-apps/api/core"; import type { TableInfo } from "../contexts/DatabaseContext"; -import { formatSqlIdentifier, quoteTableRef } from "./identifiers"; +import { formatSqlIdentifier } from "./identifiers"; import { getCurrentStatement, parseTablesFromQuery } from "./sqlAnalysis"; // Lightweight column cache with TTL and size limits @@ -103,15 +103,6 @@ export const clearAutocompleteCache = (connectionId?: string) => { const findTableByName = (name: string, tables: TableInfo[]) => tables.find((t) => t.name.toLowerCase() === name.toLowerCase())?.name; -const tableInsertText = ( - tableName: string, - driver?: string | null, - schema?: string | null, -) => - schema - ? quoteTableRef(tableName, driver, schema) - : formatSqlIdentifier(tableName, driver); - let sqlCompletionProvider: { dispose: () => void } | null = null; export const disposeSqlAutocomplete = (): void => { @@ -283,7 +274,7 @@ export const registerSqlAutocomplete = ( label: t.name, kind: monaco.languages.CompletionItemKind.Class, detail: "Table", - insertText: tableInsertText(t.name, driver, schema), + insertText: formatSqlIdentifier(t.name, driver), range, sortText: `1_${t.name}` })); diff --git a/src/utils/identifiers.ts b/src/utils/identifiers.ts index 466fc062..07a4a62a 100644 --- a/src/utils/identifiers.ts +++ b/src/utils/identifiers.ts @@ -36,6 +36,14 @@ export function shouldQuoteIdentifiers( return driver === "postgres"; } +// PostgreSQL folds unquoted identifiers to lowercase and only needs quotes for +// reserved words, mixed case, or special characters — mirroring quote_ident(). +const PG_SAFE_IDENTIFIER = /^[a-z_][a-z0-9_$]*$/; +const PG_RESERVED = new Set([ + "select", "from", "where", "table", "user", "order", "group", "join", "and", "or", + "as", "in", "on", "by", "null", "true", "false", "default", "check", "column", "limit", "offset", +]); + /** * Formats a SQL identifier for WHERE / ORDER BY fragments. * Quotes only when required (PostgreSQL); otherwise returns the name unchanged. @@ -44,9 +52,11 @@ export function formatSqlIdentifier( identifier: string, driver: string | null | undefined, ): string { - return shouldQuoteIdentifiers(driver) - ? quoteIdentifier(identifier, driver) - : identifier; + if (!shouldQuoteIdentifiers(driver)) return identifier; + if (PG_SAFE_IDENTIFIER.test(identifier) && !PG_RESERVED.has(identifier)) { + return identifier; + } + return quoteIdentifier(identifier, driver); } export function quoteIdentifier( @@ -76,3 +86,4 @@ export function quoteTableRef( } return quoteIdentifier(table, driver); } + diff --git a/tests/contexts/DatabaseProvider.test.tsx b/tests/contexts/DatabaseProvider.test.tsx index b3e6dc33..adc92e4b 100644 --- a/tests/contexts/DatabaseProvider.test.tsx +++ b/tests/contexts/DatabaseProvider.test.tsx @@ -15,7 +15,6 @@ vi.mock('@tauri-apps/api/event', () => ({ vi.mock('../../src/utils/autocomplete', () => ({ clearAutocompleteCache: vi.fn(), - disposeSqlAutocomplete: vi.fn(), })); vi.mock('../../src/hooks/useSettings', () => ({ diff --git a/tests/utils/autocomplete.test.ts b/tests/utils/autocomplete.test.ts index 647f917c..2db56cf1 100644 --- a/tests/utils/autocomplete.test.ts +++ b/tests/utils/autocomplete.test.ts @@ -173,7 +173,7 @@ describe('autocomplete', () => { expect(tableSuggestions[0]?.insertText).toBe('"AccountEventLog"'); }); - it('inserts schema-qualified table names for postgres when schema is set', async () => { + it('does not prefix schema and quotes table name only if needed for postgres', async () => { const monaco = createMockMonaco(); registerSqlAutocomplete( monaco as unknown as Parameters[0], @@ -192,7 +192,29 @@ describe('autocomplete', () => { const tableSuggestions = result.suggestions.filter((s: { sortText?: string }) => s.sortText?.startsWith('1_'), ); - expect(tableSuggestions[0]?.insertText).toBe('"public"."AccountEventLog"'); + expect(tableSuggestions[0]?.insertText).toBe('"AccountEventLog"'); + }); + + it('does not quote plain lowercase table names for postgres', async () => { + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'users' }], + null, + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + const result = await provider.provideCompletionItems( + createMockModel('SELECT * FROM '), + { lineNumber: 1, column: 15 }, + ); + + const tableSuggestions = result.suggestions.filter((s: { sortText?: string }) => + s.sortText?.startsWith('1_'), + ); + expect(tableSuggestions[0]?.insertText).toBe('users'); }); it('should include all table suggestions regardless of count', async () => { @@ -392,6 +414,33 @@ describe('autocomplete', () => { expect(result.suggestions[0]?.insertText).toBe('"CreatedAt"'); }); + + it('does not quote plain lowercase column names for postgres', async () => { + const mockInvoke = invoke as unknown as ReturnType; + mockInvoke.mockResolvedValue([{ name: 'email', data_type: 'varchar' }]); + + const { parseTablesFromQuery } = await import('../../src/utils/sqlAnalysis'); + (parseTablesFromQuery as ReturnType).mockReturnValue( + new Map([['u', 'users']]), + ); + + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'users' }], + 'public', + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + const model = createMockModel('SELECT u.'); + model.getValueInRange = vi.fn(() => 'SELECT u.'); + + const result = await provider.provideCompletionItems(model, { lineNumber: 1, column: 10 }); + + expect(result.suggestions[0]?.insertText).toBe('email'); + }); }); describe('suggestion limits', () => { diff --git a/tests/utils/filterBar.test.ts b/tests/utils/filterBar.test.ts index 6bfe2f44..082c3ecb 100644 --- a/tests/utils/filterBar.test.ts +++ b/tests/utils/filterBar.test.ts @@ -297,14 +297,24 @@ describe("filterBar utils", () => { expect(buildSingleFilterClause(filter)).toBe("price >= 9.99"); }); - it("should quote the column name with double quotes for postgres driver", () => { + it("should quote mixed-case column names for postgres driver", () => { + const filter: StructuredFilter = { + id: "1", + column: "UserStatus", + operator: "=", + value: "active", + }; + expect(buildSingleFilterClause(filter, "postgres")).toBe('"UserStatus" = \'active\''); + }); + + it("should not quote plain lowercase column names for postgres driver", () => { const filter: StructuredFilter = { id: "1", column: "user_status", operator: "=", value: "active", }; - expect(buildSingleFilterClause(filter, "postgres")).toBe('"user_status" = \'active\''); + expect(buildSingleFilterClause(filter, "postgres")).toBe("user_status = 'active'"); }); it("should not quote the column name for mysql driver", () => { @@ -340,13 +350,13 @@ describe("filterBar utils", () => { ); }); - it("should quote columns for postgres in structured filters", () => { + it("should quote columns that require quoting for postgres in structured filters", () => { const filters: StructuredFilter[] = [ - { id: "1", column: "status", operator: "=", value: "active" }, - { id: "2", column: "age", operator: ">", value: "18" }, + { id: "1", column: "UserStatus", operator: "=", value: "active" }, + { id: "2", column: "order", operator: ">", value: "18" }, ]; expect(buildStructuredFilterClause(filters, "postgres")).toBe( - '"status" = \'active\' AND "age" > 18' + '"UserStatus" = \'active\' AND "order" > 18' ); }); diff --git a/tests/utils/identifiers.test.ts b/tests/utils/identifiers.test.ts index 7bc83164..3348005b 100644 --- a/tests/utils/identifiers.test.ts +++ b/tests/utils/identifiers.test.ts @@ -103,18 +103,39 @@ describe('quoteTableRef', () => { }); describe('formatSqlIdentifier', () => { - it('should quote identifiers for postgres', () => { + it('should not quote plain lowercase identifiers for postgres', () => { + expect(formatSqlIdentifier('users', 'postgres')).toBe('users'); + expect(formatSqlIdentifier('user_status', 'postgres')).toBe('user_status'); + }); + + it('should quote mixed case identifiers for postgres', () => { expect(formatSqlIdentifier('Status', 'postgres')).toBe('"Status"'); - expect(formatSqlIdentifier('user_status', 'postgres')).toBe('"user_status"'); + expect(formatSqlIdentifier('AccountEventLog', 'postgres')).toBe('"AccountEventLog"'); + expect(formatSqlIdentifier('AccountId', 'postgres')).toBe('"AccountId"'); + }); + + it('should quote reserved words for postgres', () => { + expect(formatSqlIdentifier('select', 'postgres')).toBe('"select"'); + expect(formatSqlIdentifier('user', 'postgres')).toBe('"user"'); + expect(formatSqlIdentifier('table', 'postgres')).toBe('"table"'); + }); + + it('should quote identifiers with special characters or spaces for postgres', () => { + expect(formatSqlIdentifier('my table', 'postgres')).toBe('"my table"'); + expect(formatSqlIdentifier('table-name', 'postgres')).toBe('"table-name"'); }); it('should leave identifiers unchanged for mysql', () => { expect(formatSqlIdentifier('Status', 'mysql')).toBe('Status'); expect(formatSqlIdentifier('user_status', 'mariadb')).toBe('user_status'); + expect(formatSqlIdentifier('AccountEventLog', 'mysql')).toBe('AccountEventLog'); + expect(formatSqlIdentifier('select', 'mysql')).toBe('select'); }); it('should leave identifiers unchanged for sqlite and unknown drivers', () => { expect(formatSqlIdentifier('Status', 'sqlite')).toBe('Status'); expect(formatSqlIdentifier('Status', null)).toBe('Status'); + expect(formatSqlIdentifier('users', 'sqlite')).toBe('users'); + expect(formatSqlIdentifier('AccountEventLog', 'sqlite')).toBe('AccountEventLog'); }); }); \ No newline at end of file From 76d49f25893d8b759bb82098b07ac38f71212d3b Mon Sep 17 00:00:00 2001 From: Matheus Tonon Date: Wed, 17 Jun 2026 06:17:32 -0300 Subject: [PATCH 5/8] fix(editor): remove duplicate SQL autocomplete registration --- src/pages/Editor.tsx | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/pages/Editor.tsx b/src/pages/Editor.tsx index 99e364e3..e71fa29c 100644 --- a/src/pages/Editor.tsx +++ b/src/pages/Editor.tsx @@ -83,7 +83,6 @@ import { SqlEditorWrapper } from "../components/ui/SqlEditorWrapper"; import { NotebookView } from "../components/notebook/NotebookView"; import { useSqlAutocompleteRegistration } from "../hooks/useSqlAutocompleteRegistration"; import { createNotebook, renameNotebook } from "../utils/notebookStore"; -import { registerSqlAutocomplete } from "../utils/autocomplete"; import { type OnMount, type Monaco } from "@monaco-editor/react"; import { save } from "@tauri-apps/plugin-dialog"; import { useAlert } from "../hooks/useAlert"; @@ -139,15 +138,12 @@ export const Editor = () => { const { activeConnectionId, views, - tables, activeDriver, activeSchema, activeCapabilities, selectedDatabases, activeConnectionName, activeDatabaseName, - schemaDataMap, - databaseDataMap, } = useDatabase(); const { explorerConnectionId } = useConnectionLayoutContext(); const { settings } = useSettings(); @@ -2193,26 +2189,6 @@ export const Editor = () => { enabled: !isNotebookTab, }); - useEffect(() => { - if (monacoInstance && activeConnectionId) { - let effectiveTables = tables; - if (activeCapabilities?.schemas && activeSchema) { - effectiveTables = schemaDataMap[activeSchema]?.tables ?? tables; - } else if (isMultiDb) { - effectiveTables = selectedDatabases.flatMap(db => - (databaseDataMap[db]?.tables ?? []).map(t => ({ ...t, schema: db })) - ); - } - const disposable = registerSqlAutocomplete( - monacoInstance, - activeConnectionId, - effectiveTables, - activeSchema, - ); - return () => disposable.dispose(); - } - }, [monacoInstance, activeConnectionId, tables, activeSchema, activeCapabilities, schemaDataMap, databaseDataMap, isMultiDb, selectedDatabases]); - useEffect(() => { const state = location.state as EditorState; if (activeConnectionId) { From 2daa3ec32d9a074f09efe4f11ad2501699fa0a41 Mon Sep 17 00:00:00 2001 From: NewtTheWolf Date: Thu, 18 Jun 2026 17:37:32 +0200 Subject: [PATCH 6/8] fix(autocomplete): avoid doubling quotes for quoted identifier completion When a completion's insertText became a fully quoted identifier, typing an opening quote first (which Monaco auto-closes) produced ""Name" because the replacement range only covered the bare word. Expanding the range to swallow surrounding quotes broke filtering (Monaco matched items against the leading quote and hid them all). Now, when an opening quote precedes the range, swallow the surrounding quote(s), emit a fully quoted identifier, and set filterText to the same quoted form so suggestions still match. Canonical result for 0, 1 or 2 surrounding quotes. --- src/utils/autocomplete.ts | 57 +++++++++++++++++++++++++---- tests/utils/autocomplete.test.ts | 63 ++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 7 deletions(-) diff --git a/src/utils/autocomplete.ts b/src/utils/autocomplete.ts index 441ee5ab..6d0304e2 100644 --- a/src/utils/autocomplete.ts +++ b/src/utils/autocomplete.ts @@ -1,7 +1,7 @@ import type { Monaco } from "@monaco-editor/react"; import { invoke } from "@tauri-apps/api/core"; import type { TableInfo } from "../contexts/DatabaseContext"; -import { formatSqlIdentifier } from "./identifiers"; +import { formatSqlIdentifier, getQuoteChar, quoteIdentifier } from "./identifiers"; import { getCurrentStatement, parseTablesFromQuery, type ParsedTableRef } from "./sqlAnalysis"; // Lightweight column cache with TTL and size limits @@ -132,6 +132,51 @@ export const registerSqlAutocomplete = ( endColumn: wordUntil.endColumn, }; + // When the user has already typed an opening quote, Monaco auto-closes it + // so the cursor sits inside a pair (`"|"`); the user may also have deleted + // the closing one (`"|`). Inserting a freshly quoted identifier into the + // word range would then double the opening quote (`""AccountEventLog"`) or + // leave it dangling. So when an opening quote precedes the replacement + // range, expand the range to swallow it (and the closing quote if present) + // and emit a fully-quoted identifier — yielding a canonical result for 0, 1 + // or 2 surrounding quotes alike. + const quoteChar = getQuoteChar(driver); + const charAt = (column: number): string => + column < 1 + ? "" + : model.getValueInRange({ + startLineNumber: position.lineNumber, + startColumn: column, + endLineNumber: position.lineNumber, + endColumn: column + 1, + }); + const buildIdentifierInsert = ( + name: string, + baseRange: { startLineNumber: number; endLineNumber: number; startColumn: number; endColumn: number }, + ): { + insertText: string; + range: { startLineNumber: number; endLineNumber: number; startColumn: number; endColumn: number }; + filterText?: string; + } => { + if (baseRange.startColumn <= 1 || charAt(baseRange.startColumn - 1) !== quoteChar) { + return { insertText: formatSqlIdentifier(name, driver), range: baseRange }; + } + const swallowsClosing = charAt(baseRange.endColumn) === quoteChar; + const insertText = quoteIdentifier(name, driver); + return { + insertText, + // The range now starts at the opening quote, so Monaco filters items + // against the leading quote; match it by giving filterText the same + // quoted form, otherwise every suggestion gets filtered out. + filterText: insertText, + range: { + ...baseRange, + startColumn: baseRange.startColumn - 1, + endColumn: swallowsClosing ? baseRange.endColumn + 1 : baseRange.endColumn, + }, + }; + }; + // Get text until cursor position const textUntilPosition = model.getValueInRange({ startLineNumber: position.lineNumber, @@ -198,8 +243,7 @@ export const registerSqlAutocomplete = ( label: c.label, kind: monaco.languages.CompletionItemKind.Field, detail: c.detail, - insertText: formatSqlIdentifier(c.label, driver), - range: columnRange, + ...buildIdentifierInsert(c.label, columnRange), sortText: `0_${c.label}`, })), }; @@ -218,6 +262,7 @@ export const registerSqlAutocomplete = ( insertText: string; range: { startLineNumber: number; endLineNumber: number; startColumn: number; endColumn: number }; sortText: string; + filterText?: string; }> = []; if (tableAliases && tableAliases.size > 0) { @@ -273,8 +318,7 @@ export const registerSqlAutocomplete = ( label: col.label, kind: monaco.languages.CompletionItemKind.Field, detail: `${col.detail} — ${table.name}${aliasHint}`, - insertText: formatSqlIdentifier(col.label, driver), - range, + ...buildIdentifierInsert(col.label, range), sortText: `0_${col.label}`, }); } @@ -303,8 +347,7 @@ export const registerSqlAutocomplete = ( label: t.name, kind: monaco.languages.CompletionItemKind.Class, detail: "Table", - insertText: formatSqlIdentifier(t.name, driver), - range, + ...buildIdentifierInsert(t.name, range), sortText: `1_${t.name}` })); diff --git a/tests/utils/autocomplete.test.ts b/tests/utils/autocomplete.test.ts index af597167..c1b32928 100644 --- a/tests/utils/autocomplete.test.ts +++ b/tests/utils/autocomplete.test.ts @@ -195,6 +195,69 @@ describe('autocomplete', () => { expect(tableSuggestions[0]?.insertText).toBe('"AccountEventLog"'); }); + it('swallows the auto-closed quote pair when an opening quote was typed (postgres)', async () => { + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'AccountEventLog' }], + null, + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + // `SELECT * FROM ""` — Monaco auto-closed the quote, cursor between the pair. + const model = createMockModel('SELECT * FROM ""'); + model.getWordUntilPosition = vi.fn(() => ({ startColumn: 16, endColumn: 16 })); + + const result = await provider.provideCompletionItems(model, { + lineNumber: 1, + column: 16, + }); + + const table = result.suggestions.find((s: { sortText?: string }) => + s.sortText?.startsWith('1_'), + ); + // Canonical quoted identifier, range swallows BOTH surrounding quotes so + // the result is exactly "AccountEventLog" (not ""AccountEventLog""). + expect(table?.insertText).toBe('"AccountEventLog"'); + expect(table?.range.startColumn).toBe(15); + expect(table?.range.endColumn).toBe(17); + // Range starts at the opening quote, so filterText must also be quoted or + // Monaco filters every suggestion out. + expect(table?.filterText).toBe('"AccountEventLog"'); + }); + + it('still closes the identifier when the auto-closed quote was deleted (postgres)', async () => { + const monaco = createMockMonaco(); + registerSqlAutocomplete( + monaco as unknown as Parameters[0], + 'conn1', + [{ name: 'AccountEventLog' }], + null, + 'postgres', + ); + + const provider = monaco.languages.registerCompletionItemProvider.mock.calls[0][1]; + // `SELECT * FROM "` — user deleted the auto-closed quote, only the opening one remains. + const model = createMockModel('SELECT * FROM "'); + model.getWordUntilPosition = vi.fn(() => ({ startColumn: 16, endColumn: 16 })); + + const result = await provider.provideCompletionItems(model, { + lineNumber: 1, + column: 16, + }); + + const table = result.suggestions.find((s: { sortText?: string }) => + s.sortText?.startsWith('1_'), + ); + // Full quoted identifier replaces the lone opening quote → "AccountEventLog". + expect(table?.insertText).toBe('"AccountEventLog"'); + expect(table?.range.startColumn).toBe(15); + expect(table?.range.endColumn).toBe(16); + expect(table?.filterText).toBe('"AccountEventLog"'); + }); + it('does not quote plain lowercase table names for postgres', async () => { const monaco = createMockMonaco(); registerSqlAutocomplete( From 089a33573058e13951f7c6c604eab205241575bb Mon Sep 17 00:00:00 2001 From: Dominik Spitzli Date: Fri, 19 Jun 2026 10:52:30 +0200 Subject: [PATCH 7/8] Update src/utils/sqlAnalysis.ts Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com> --- src/utils/sqlAnalysis.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/sqlAnalysis.ts b/src/utils/sqlAnalysis.ts index 278620c0..84068887 100644 --- a/src/utils/sqlAnalysis.ts +++ b/src/utils/sqlAnalysis.ts @@ -9,7 +9,7 @@ export interface ParsedTableRef { // Unquoted identifiers are normalized to lowercase. function stripIdentifierQuotes(token: string): string { const q = token[0]; - if (q === '"' || q === '`') return token.slice(1, -1); + if (q === '"' || q === '`') return token.slice(1, -1).replaceAll(q + q, q); return token.toLowerCase(); } From 4021e6703dbf1522aef85bcdcf63f1b245d11586 Mon Sep 17 00:00:00 2001 From: Dominik Spitzli Date: Fri, 19 Jun 2026 19:15:46 +0200 Subject: [PATCH 8/8] Update src/utils/sqlAnalysis.ts Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com> --- src/utils/sqlAnalysis.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/sqlAnalysis.ts b/src/utils/sqlAnalysis.ts index 84068887..2f05ddec 100644 --- a/src/utils/sqlAnalysis.ts +++ b/src/utils/sqlAnalysis.ts @@ -55,7 +55,7 @@ export const parseTablesFromQuery = (sql: string): Map | const schema = schemaToken ? stripIdentifierQuotes(schemaToken) : undefined; const aliasToken = match[3]; const alias = aliasToken ? stripIdentifierQuotes(aliasToken) : tableName; - tableMap.set(alias.toLowerCase(), { name: tableName, schema }); + tableMap.set(alias, { name: tableName, schema }); } return tableMap.size > 0 ? tableMap : null;