From 274a009ebebd5d4fb1a5beb1a20248c2d505fcdf Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 23 Sep 2026 14:45:34 +0700 Subject: [PATCH] fix(editor): stop completing another schema's tables without their schema --- CHANGELOG.md | 1 + .../Core/Autocomplete/SQLSchemaProvider.swift | 75 +++-- ...LSchemaProviderUnqualifiedScopeTests.swift | 291 ++++++++++++++++++ docs/features/autocomplete.mdx | 9 +- 4 files changed, 357 insertions(+), 19 deletions(-) create mode 100644 TableProTests/Core/Autocomplete/SQLSchemaProviderUnqualifiedScopeTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index e2e438bd59..f55a4bae65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Autocomplete offering another schema's tables without their schema once that schema was completed or expanded. - Unresponsive app and a dropped keystroke when typing in the row inspector's JSON field. (#3051) - Raw Oracle driver error in the schema switch failure dialog. (#3053) - Oracle health check closing a connection a statement was still running on. (#3053) diff --git a/TablePro/Core/Autocomplete/SQLSchemaProvider.swift b/TablePro/Core/Autocomplete/SQLSchemaProvider.swift index 0171d37863..ce6ccc04fc 100644 --- a/TablePro/Core/Autocomplete/SQLSchemaProvider.swift +++ b/TablePro/Core/Autocomplete/SQLSchemaProvider.swift @@ -66,6 +66,12 @@ actor SQLSchemaProvider { private var fieldPathCache: [String: [PluginFieldPath]] = [:] private var fieldPathTasks: [String: Task<[PluginFieldPath], Never>] = [:] + /// Another schema's tables, fetched when a statement names that schema and a dot. They are + /// held apart from `tables`, which the scope's owner writes and every unqualified reader + /// trusts, so completing `attendance.` cannot make `timesheet` an answer for a bare name. + private var onDemandSchemaTables: [String: [TableInfo]] = [:] + private var onDemandSchemaTableTasks: [String: Task<[TableInfo]?, Never>] = [:] + private var knownSchemas: [String] = [] private var knownDatabases: [String] = [] @@ -207,6 +213,8 @@ actor SQLSchemaProvider { self.columnAccessOrder.removeAll() self.fieldPathCache.removeAll() self.fieldPathTasks.removeAll() + self.onDemandSchemaTables.removeAll() + self.onDemandSchemaTableTasks.removeAll() self.cachedDriver = driver self.eagerLoadSchema = (driver as? SchemaSwitchable)?.currentSchema if let connection { self.connectionInfo = connection } @@ -329,7 +337,7 @@ actor SQLSchemaProvider { } } - for table in tables { + for table in tablesResolvableUnqualified { if table.name.lowercased() == lowerName { return table.name } @@ -338,6 +346,17 @@ actor SQLSchemaProvider { return nil } + /// `tables` is every table the scope's owner loaded, which for the browse scope includes each + /// schema the sidebar has expanded. A bare name only reaches the schema unqualified names + /// resolve in, so only its tables may be offered or matched without their schema. + private var tablesResolvableUnqualified: [TableInfo] { + guard let defaultSchema = getDefaultSchema(), !defaultSchema.isEmpty else { return tables } + return tables.filter { table in + guard let tableSchema = table.schema, !tableSchema.isEmpty else { return true } + return tableSchema.caseInsensitiveCompare(defaultSchema) == .orderedSame + } + } + // MARK: - AI Schema Context func buildSchemaContextForAI(settings: AISettings) async -> String? { @@ -381,9 +400,9 @@ actor SQLSchemaProvider { // MARK: - Completion Items - /// Get completion items for tables + /// Tables a statement can name without their schema. func tableCompletionItems() async -> [SQLCompletionItem] { - let tableData = tables.map { (name: $0.name, isView: $0.type == .view) } + let tableData = tablesResolvableUnqualified.map { (name: $0.name, isView: $0.type == .view) } return await MainActor.run { tableData.map { SQLCompletionItem.table($0.name, isView: $0.isView) } } @@ -430,29 +449,49 @@ actor SQLSchemaProvider { /// Tables of one schema — suggested after a schema-qualified dot (e.g. "DBT_MARTS."). /// Falls back to fetching from the database when that schema's tables aren't loaded yet. func tableCompletionItems(inSchema schema: String) async -> [SQLCompletionItem] { - var matching = tables.filter { $0.schema?.caseInsensitiveCompare(schema) == .orderedSame } - if matching.isEmpty, let fetchSchemaTables = metadataSource?.fetchSchemaTables { - if let fetched = try? await fetchSchemaTables(schema), !fetched.isEmpty { - matching = fetched.filter { belongsToSchema($0, schema) } - mergeTables(fetched) - } - } - let tableData = matching.map { (name: $0.name, isView: $0.type == .view) } + let listed = await knownTables(inSchema: schema) + let tableData = listed.map { (name: $0.name, isView: $0.type == .view) } return await MainActor.run { tableData.map { SQLCompletionItem.table($0.name, isView: $0.isView) } } } - private func belongsToSchema(_ table: TableInfo, _ schema: String) -> Bool { - guard let tableSchema = table.schema, !tableSchema.isEmpty else { return true } - return tableSchema.caseInsensitiveCompare(schema) == .orderedSame + private func knownTables(inSchema schema: String) async -> [TableInfo] { + let loaded = tables.filter { $0.schema?.caseInsensitiveCompare(schema) == .orderedSame } + guard loaded.isEmpty else { return loaded } + return await onDemandTables(inSchema: schema) } - private func mergeTables(_ newTables: [TableInfo]) { - var seen = Set(tables.map(\.id)) - for table in newTables where seen.insert(table.id).inserted { - tables.append(table) + /// Concurrent callers await the fetch already in flight, and a fetch that a reset overtook + /// answers its own caller without writing into the scope that replaced it. An empty schema is + /// an answer and is kept; a failed fetch is not, so the next keystroke asks again. + private func onDemandTables(inSchema schema: String) async -> [TableInfo] { + let key = schema.lowercased() + if let cached = onDemandSchemaTables[key] { return cached } + if let inFlight = onDemandSchemaTableTasks[key] { return await inFlight.value ?? [] } + guard let fetchSchemaTables = metadataSource?.fetchSchemaTables else { return [] } + + let task = Task<[TableInfo]?, Never> { + do { + return try await fetchSchemaTables(schema).filter { Self.belongsToSchema($0, schema) } + } catch { + Self.logger.debug( + "[schema] on-demand schema tables failed: \(error.publicLogShape, privacy: .public)" + ) + return nil + } } + onDemandSchemaTableTasks[key] = task + let fetched = await task.value + guard onDemandSchemaTableTasks[key] == task else { return fetched ?? [] } + onDemandSchemaTableTasks[key] = nil + if let fetched { onDemandSchemaTables[key] = fetched } + return fetched ?? [] + } + + private static func belongsToSchema(_ table: TableInfo, _ schema: String) -> Bool { + guard let tableSchema = table.schema, !tableSchema.isEmpty else { return true } + return tableSchema.caseInsensitiveCompare(schema) == .orderedSame } /// Get completion items for columns of a specific table diff --git a/TableProTests/Core/Autocomplete/SQLSchemaProviderUnqualifiedScopeTests.swift b/TableProTests/Core/Autocomplete/SQLSchemaProviderUnqualifiedScopeTests.swift new file mode 100644 index 0000000000..5079feaeef --- /dev/null +++ b/TableProTests/Core/Autocomplete/SQLSchemaProviderUnqualifiedScopeTests.swift @@ -0,0 +1,291 @@ +// +// SQLSchemaProviderUnqualifiedScopeTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("SQLSchemaProvider unqualified scope") +struct SQLSchemaProviderUnqualifiedScopeTests { + private static func postgresDriver() -> MockDatabaseDriver { + let driver = MockDatabaseDriver(connection: TestFixtures.makeConnection(type: .postgresql)) + driver.currentSchema = "public" + return driver + } + + private static func source(_ script: ScriptedSchemaTablesFetch) -> SQLSchemaProvider.ColumnMetadataSource { + SQLSchemaProvider.ColumnMetadataSource( + fetchColumns: { _, _ in [] }, + fetchAllColumns: { [:] }, + fetchSchemaTables: { _ in try await script.fetch() } + ) + } + + private static let users = TestFixtures.makeTableInfo(name: "users", schema: "public") + private static let timesheet = TestFixtures.makeTableInfo(name: "timesheet", schema: "attendance") + + @Test("A table in a schema the tab does not resolve names in is not offered without its schema") + func expandedSchemaTableIsNotOfferedBare() async { + let driver = Self.postgresDriver() + let provider = SQLSchemaProvider() + await provider.resetForDatabase( + "db", tables: [Self.users, Self.timesheet], driver: driver, connection: driver.connection + ) + + let labels = await provider.tableCompletionItems().map(\.label) + + #expect(labels == ["users"]) + } + + @Test("Completing another schema's tables leaves bare completion and the scope's tables alone") + func qualifiedCompletionDoesNotWidenTheScope() async { + let script = ScriptedSchemaTablesFetch([.success([Self.timesheet])]) + let driver = Self.postgresDriver() + let provider = SQLSchemaProvider(metadataSource: Self.source(script)) + await provider.resetForDatabase("db", tables: [Self.users], driver: driver, connection: driver.connection) + + let qualified = await provider.tableCompletionItems(inSchema: "attendance").map(\.label) + let bare = await provider.tableCompletionItems().map(\.label) + let scopeTables = await provider.getTables().map(\.name) + + #expect(qualified == ["timesheet"]) + #expect(bare == ["users"]) + #expect(scopeTables == ["users"]) + } + + @Test("A bare FROM prefix never offers a table only another schema holds") + func bareFromPrefixAfterQualifiedCompletion() async { + let script = ScriptedSchemaTablesFetch([.success([Self.timesheet])]) + let driver = Self.postgresDriver() + let schemaProvider = SQLSchemaProvider(metadataSource: Self.source(script)) + await schemaProvider.resetForDatabase( + "db", tables: [Self.users], driver: driver, connection: driver.connection + ) + await schemaProvider.setNamespaces(schemas: ["public", "attendance"], databases: ["db"]) + let completion = SQLCompletionProvider(schemaProvider: schemaProvider, databaseType: .postgresql) + + let qualifiedText = "SELECT * FROM attendance." + let qualified = await completion.getCompletions( + text: qualifiedText, cursorPosition: (qualifiedText as NSString).length + ).items + let bareText = "SELECT * FROM times" + let bare = await completion.getCompletions( + text: bareText, cursorPosition: (bareText as NSString).length + ).items + + #expect(qualified.map(\.label).contains("timesheet")) + #expect(!bare.contains { $0.insertText.localizedCaseInsensitiveContains("timesheet") }) + } + + @Test("A bare name before a dot resolves only to a table the tab can name without its schema") + func bareNameResolvesWithinTheUnqualifiedScope() async { + let driver = Self.postgresDriver() + let provider = SQLSchemaProvider() + await provider.resetForDatabase( + "db", tables: [Self.users, Self.timesheet], driver: driver, connection: driver.connection + ) + let reference = TableReference(tableName: "timesheet", alias: "t", schema: "attendance") + + #expect(await provider.resolveAlias("timesheet", in: []) == nil) + #expect(await provider.resolveAlias("users", in: []) == "users") + #expect(await provider.resolveAlias("t", in: [reference]) == "timesheet") + #expect(await provider.resolveAlias("timesheet", in: [reference]) == "timesheet") + } + + @Test("The engine's implicit schema, not the browsed one, is what a bare name reaches") + func implicitSchemaDecidesTheUnqualifiedScope() async throws { + let connection = TestFixtures.makeConnection(type: .spanner) + let implicitSchema = try #require(connection.type.implicitSchemaName) + let driver = MockDatabaseDriver(connection: connection) + driver.currentSchema = "sales" + let provider = SQLSchemaProvider() + await provider.resetForDatabase( + "db", + tables: [ + TestFixtures.makeTableInfo(name: "singers", schema: implicitSchema), + TestFixtures.makeTableInfo(name: "orders", schema: "sales") + ], + driver: driver, + connection: connection + ) + + let labels = await provider.tableCompletionItems().map(\.label) + + #expect(labels == ["singers"]) + } + + @Test("An engine with no current schema offers every table without its schema") + func engineWithoutCurrentSchemaOffersEveryTable() async { + let provider = SQLSchemaProvider() + await provider.resetForDatabase( + "db", + tables: [ + TestFixtures.makeTableInfo(name: "orders"), + TestFixtures.makeTableInfo(name: "events", schema: "audit") + ], + driver: MockDatabaseDriver() + ) + + let labels = await provider.tableCompletionItems().map(\.label) + + #expect(Set(labels) == ["orders", "events"]) + } + + @Test("A schema with no tables is fetched once, not on every keystroke") + func emptySchemaIsFetchedOnce() async { + let script = ScriptedSchemaTablesFetch([.success([])]) + let provider = SQLSchemaProvider(metadataSource: Self.source(script)) + + let first = await provider.tableCompletionItems(inSchema: "archive") + let second = await provider.tableCompletionItems(inSchema: "archive") + + #expect(first.isEmpty) + #expect(second.isEmpty) + #expect(script.calls == 1) + } + + @Test("A failed fetch of another schema's tables is asked again") + func failedSchemaFetchIsRetried() async { + let script = ScriptedSchemaTablesFetch([ + .failure(DatabaseError.queryFailed("timeout")), + .success([Self.timesheet]) + ]) + let provider = SQLSchemaProvider(metadataSource: Self.source(script)) + + let first = await provider.tableCompletionItems(inSchema: "attendance").map(\.label) + let second = await provider.tableCompletionItems(inSchema: "attendance").map(\.label) + + #expect(first.isEmpty) + #expect(second == ["timesheet"]) + #expect(script.calls == 2) + } + + @Test("Concurrent completions of one schema share a single fetch") + func concurrentSchemaCompletionsShareOneFetch() async { + let script = ScriptedSchemaTablesFetch([.success([Self.timesheet])], holdsFirstCall: true) + let provider = SQLSchemaProvider(metadataSource: Self.source(script)) + + let first = Task { await provider.tableCompletionItems(inSchema: "attendance").map(\.label) } + await script.waitForCalls(1) + let second = Task { await provider.tableCompletionItems(inSchema: "attendance").map(\.label) } + for _ in 0..<50 where script.calls < 2 { + try? await Task.sleep(nanoseconds: 2_000_000) + } + script.releaseHeldCall() + + #expect(await first.value == ["timesheet"]) + #expect(await second.value == ["timesheet"]) + #expect(script.calls == 1) + } + + @Test("A fetch a database switch overtook answers its caller and nothing after the switch") + func overtakenFetchDoesNotAnswerForTheNewScope() async { + let roster = TestFixtures.makeTableInfo(name: "roster", schema: "attendance") + let script = ScriptedSchemaTablesFetch([.success([Self.timesheet]), .success([roster])], holdsFirstCall: true) + let driver = Self.postgresDriver() + let provider = SQLSchemaProvider(metadataSource: Self.source(script)) + await provider.resetForDatabase("first", tables: [Self.users], driver: driver, connection: driver.connection) + + let overtaken = Task { await provider.tableCompletionItems(inSchema: "attendance").map(\.label) } + await script.waitForCalls(1) + await provider.resetForDatabase("second", tables: [Self.users], driver: driver, connection: driver.connection) + script.releaseHeldCall() + let overtakenLabels = await overtaken.value + + let fresh = await provider.tableCompletionItems(inSchema: "attendance").map(\.label) + let bare = await provider.tableCompletionItems().map(\.label) + + #expect(overtakenLabels == ["timesheet"]) + #expect(fresh == ["roster"]) + #expect(bare == ["users"]) + #expect(script.calls == 2) + } +} + +/// Answers `fetchSchemaTables` from a list, one answer per call with the last repeated, and can +/// hold the first call until the test releases it. +private final class ScriptedSchemaTablesFetch: @unchecked Sendable { + private let lock = NSLock() + private let answers: [Result<[TableInfo], any Error>] + private let holdsFirstCall: Bool + private var startedCalls = 0 + private var arrivedCalls = 0 + private var heldCall: CheckedContinuation? + private var callWaiters: [(count: Int, continuation: CheckedContinuation)] = [] + + init(_ answers: [Result<[TableInfo], any Error>], holdsFirstCall: Bool = false) { + self.answers = answers + self.holdsFirstCall = holdsFirstCall + } + + var calls: Int { + lock.lock() + defer { lock.unlock() } + return arrivedCalls + } + + func fetch() async throws -> [TableInfo] { + let index = reserveCall() + let answer = answers[min(index, answers.count - 1)] + if index == 0 && holdsFirstCall { + await withCheckedContinuation { continuation in + hold(continuation) + noteArrival() + } + } else { + noteArrival() + } + return try answer.get() + } + + func waitForCalls(_ count: Int) async { + await withCheckedContinuation { continuation in + enqueueWaiter(count: count, continuation) + } + } + + func releaseHeldCall() { + lock.lock() + let held = heldCall + heldCall = nil + lock.unlock() + held?.resume() + } + + private func reserveCall() -> Int { + lock.lock() + defer { lock.unlock() } + let index = startedCalls + startedCalls += 1 + return index + } + + private func hold(_ continuation: CheckedContinuation) { + lock.lock() + heldCall = continuation + lock.unlock() + } + + private func noteArrival() { + lock.lock() + arrivedCalls += 1 + let arrived = arrivedCalls + let ready = callWaiters.filter { $0.count <= arrived } + callWaiters.removeAll { $0.count <= arrived } + lock.unlock() + ready.forEach { $0.continuation.resume() } + } + + private func enqueueWaiter(count: Int, _ continuation: CheckedContinuation) { + lock.lock() + guard arrivedCalls < count else { + lock.unlock() + continuation.resume() + return + } + callWaiters.append((count, continuation)) + lock.unlock() + } +} diff --git a/docs/features/autocomplete.mdx b/docs/features/autocomplete.mdx index 377ccfe62e..ef53e8d8d3 100644 --- a/docs/features/autocomplete.mdx +++ b/docs/features/autocomplete.mdx @@ -137,7 +137,14 @@ WHERE mood <> 'ha| -- 'happy' On multi-schema engines, schemas complete in FROM and `public.users` resolves in FROM, JOIN, UPDATE, INSERT INTO and CREATE INDEX. -Where the hierarchy is database, schema, table (Snowflake, BigQuery), every segment completes and a schema you have not opened in the sidebar is fetched on demand: +A bare table name completes with the tables the server finds without a schema: those in the tab's own schema, or on Spanner those in the default schema. For a table anywhere else, type its schema and a dot. That schema's tables follow, including a schema the sidebar has never opened: + +```sql +SELECT * FROM times| -- tables in the tab's own schema +SELECT * FROM attendance.| -- tables in attendance +``` + +Where the hierarchy is database, schema, table (Snowflake, BigQuery), every segment completes: ```sql SELECT * FROM ANALYTICS_| -- databases