Skip to content

Commit a3a28dc

Browse files
committed
Fix MySQL autocomplete in multi-database setups
Two bugs surfaced by driving the TUI's completion engine against a real MySQL server with multiple user databases: 1. Qualified table/view names rendered as `db`.``.`table`. MySQL has no schema-within-database, so the schema slot comes back as ''; the inline name-builder inserted it unconditionally, producing a three-part identifier with an empty middle. Promote the composition rule to a new `Dialect.qualified_name(database, schema, name)` method. Base default skips empty segments, which yields the correct result for every current adapter: MySQL/MariaDB: `db`.`table` PostgreSQL: "schema"."table" SQL Server: [db].[schema].[table] SQLite: "table" Autocomplete's four inline builders (tables/views × sync/async loader) now call `dialect.qualified_name(...)` directly. 2. Completions stuck on "Loading..." for cursor positions like `SELECT * FROM shop.cu`. The completion engine classifies this as an ALIAS_COLUMN with scope 'shop', which isn't a real table. The loader skips unknown keys silently; the caller kept returning Loading... forever because the guard assumed "not yet loaded" meant "in flight". Guard the sentinel on the table key actually existing in `_table_metadata`. Covered by 8 new unit tests pinning `qualified_name` on MySQL, MariaDB, PostgreSQL, SQL Server, empty-segment behavior, quote-char escaping, and the stuck-Loading regression. May fix #151.
1 parent a132802 commit a3a28dc

File tree

5 files changed

+203
-32
lines changed

5 files changed

+203
-32
lines changed

sqlit/domains/connections/providers/adapters/base.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,22 @@ def quote_identifier(self, name: str) -> str:
417417
"""Quote an identifier (table name, column name, etc.)."""
418418
pass
419419

420+
def qualified_name(self, database: str | None, schema: str | None, name: str) -> str:
421+
"""Build a quoted qualified identifier, skipping empty segments.
422+
423+
Default handles SQL Server-style `[db].[schema].[name]`, PostgreSQL-
424+
style `"schema"."name"`, and single-part `"name"` by omitting any
425+
empty/None component. Dialects that want different composition
426+
(e.g. MySQL, which has no schemas within databases) can override.
427+
"""
428+
parts: list[str] = []
429+
if database:
430+
parts.append(self.quote_identifier(database))
431+
if schema:
432+
parts.append(self.quote_identifier(schema))
433+
parts.append(self.quote_identifier(name))
434+
return ".".join(parts)
435+
420436
@abstractmethod
421437
def build_select_query(self, table: str, limit: int, database: str | None = None, schema: str | None = None) -> str:
422438
"""Build a SELECT query with limit.

sqlit/domains/connections/providers/model.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ def build_select_query(self, table: str, limit: int, database: str | None = None
6262

6363
def format_table_name(self, schema: str | None, table: str) -> str: ...
6464

65+
def qualified_name(self, database: str | None, schema: str | None, name: str) -> str:
66+
"""Build a quoted qualified identifier for use in SQL.
67+
68+
Called when completing tables/views across multiple databases. Each
69+
dialect decides how many segments to emit:
70+
- SQL Server / generic: `[db].[schema].[name]`
71+
- PostgreSQL: `"schema"."name"` (databases are isolated)
72+
- MySQL/MariaDB: `` `db`.`name` `` (no schemas within databases)
73+
- SQLite: `"name"`
74+
"""
75+
...
76+
6577

6678
@runtime_checkable
6779
class SchemaInspector(Protocol):

sqlit/domains/query/ui/mixins/autocomplete_schema.py

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -406,13 +406,7 @@ def work() -> None:
406406
if single_db:
407407
full_name = table_name
408408
else:
409-
quoted_db = dialect.quote_identifier(database) if database else ""
410-
quoted_schema = dialect.quote_identifier(schema_name)
411-
quoted_table = dialect.quote_identifier(table_name)
412-
if database:
413-
full_name = f"{quoted_db}.{quoted_schema}.{quoted_table}"
414-
else:
415-
full_name = f"{quoted_schema}.{quoted_table}"
409+
full_name = dialect.qualified_name(database, schema_name, table_name)
416410

417411
display_name = dialect.format_table_name(schema_name, table_name)
418412
metadata = [
@@ -495,13 +489,7 @@ def work() -> None:
495489
if single_db:
496490
full_name = view_name
497491
else:
498-
quoted_db = dialect.quote_identifier(database) if database else ""
499-
quoted_schema = dialect.quote_identifier(schema_name)
500-
quoted_view = dialect.quote_identifier(view_name)
501-
if database:
502-
full_name = f"{quoted_db}.{quoted_schema}.{quoted_view}"
503-
else:
504-
full_name = f"{quoted_schema}.{quoted_view}"
492+
full_name = dialect.qualified_name(database, schema_name, view_name)
505493

506494
display_name = dialect.format_table_name(schema_name, view_name)
507495
metadata = [
@@ -816,14 +804,8 @@ async def run_db_call(fn: Any, *args: Any, **kwargs: Any) -> Any:
816804
# Single database - use simple table name
817805
schema_cache["tables"].append(table_name)
818806
else:
819-
# Multiple databases - use full qualifier [db].[schema].[table]
820-
quoted_db = dialect.quote_identifier(database) if database else ""
821-
quoted_schema = dialect.quote_identifier(schema_name)
822-
quoted_table = dialect.quote_identifier(table_name)
823-
if database:
824-
full_name = f"{quoted_db}.{quoted_schema}.{quoted_table}"
825-
else:
826-
full_name = f"{quoted_schema}.{quoted_table}"
807+
# Multiple databases - use qualified identifier
808+
full_name = dialect.qualified_name(database, schema_name, table_name)
827809
schema_cache["tables"].append(full_name)
828810
# Keep metadata for column loading (multiple keys for flexible lookup)
829811
display_name = dialect.format_table_name(schema_name, table_name)
@@ -843,14 +825,8 @@ async def run_db_call(fn: Any, *args: Any, **kwargs: Any) -> Any:
843825
# Single database - use simple view name
844826
schema_cache["views"].append(view_name)
845827
else:
846-
# Multiple databases - use full qualifier [db].[schema].[view]
847-
quoted_db = dialect.quote_identifier(database) if database else ""
848-
quoted_schema = dialect.quote_identifier(schema_name)
849-
quoted_view = dialect.quote_identifier(view_name)
850-
if database:
851-
full_name = f"{quoted_db}.{quoted_schema}.{quoted_view}"
852-
else:
853-
full_name = f"{quoted_schema}.{quoted_view}"
828+
# Multiple databases - use qualified identifier
829+
full_name = dialect.qualified_name(database, schema_name, view_name)
854830
schema_cache["views"].append(full_name)
855831
# Keep metadata for column loading (multiple keys for flexible lookup)
856832
display_name = dialect.format_table_name(schema_name, view_name)

sqlit/domains/query/ui/mixins/autocomplete_suggestions.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,25 @@ def _get_autocomplete_suggestions(self: AutocompleteMixinHost, text: str, cursor
7474
alias_map = self._build_alias_map(text)
7575
table_refs = extract_table_refs(text)
7676
loading: set[str] = getattr(self, "_columns_loading", set())
77+
table_metadata = getattr(self, "_table_metadata", {}) or {}
78+
79+
def needs_column_load(key: str) -> bool:
80+
"""Return True only if the key names a known table that hasn't
81+
been loaded yet. Returning Loading... for an unknown key would
82+
wedge forever because the loader skips unknown tables silently.
83+
"""
84+
if key in columns:
85+
return False
86+
if key not in table_metadata:
87+
return False
88+
return True
7789

7890
for suggestion in suggestions:
7991
if suggestion.type == SuggestionType.COLUMN:
8092
# Check if any tables need column loading
8193
for ref in table_refs:
8294
table_key = ref.name.lower()
83-
if table_key not in columns and table_key not in loading:
95+
if needs_column_load(table_key) and table_key not in loading:
8496
self._load_columns_for_table(table_key)
8597
return ["Loading..."]
8698
elif table_key in loading:
@@ -92,7 +104,7 @@ def _get_autocomplete_suggestions(self: AutocompleteMixinHost, text: str, cursor
92104
scope_lower = scope.lower()
93105
table_key = alias_map.get(scope_lower, scope_lower)
94106

95-
if table_key not in columns and table_key not in loading:
107+
if needs_column_load(table_key) and table_key not in loading:
96108
self._load_columns_for_table(table_key)
97109
return ["Loading..."]
98110
elif table_key in loading:
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
"""Regression tests for MySQL autocomplete in multi-database scenarios (#151).
2+
3+
Two narrow bugs are covered:
4+
5+
1. Qualified identifiers for databases without schemas (MySQL/MariaDB) used
6+
to render as `db`.``.`table` — an empty-backticked middle segment. The
7+
qualifying logic is now a Dialect method so each adapter owns its own
8+
composition rule.
9+
10+
2. Autocomplete returned a permanent "Loading..." sentinel whenever the
11+
table reference in the query didn't resolve to anything in the schema
12+
cache (e.g. the user typed `SELECT * FROM shop.cu`: `shop` was treated
13+
as an alias and the loader spun forever for an unknown key).
14+
"""
15+
16+
from __future__ import annotations
17+
18+
import pytest
19+
20+
21+
# --------------------------------------------------------------------------
22+
# Bug 1: Dialect.qualified_name
23+
# --------------------------------------------------------------------------
24+
25+
26+
def _get_dialect(db_type: str):
27+
from sqlit.domains.connections.providers.catalog import get_provider
28+
29+
return get_provider(db_type).dialect
30+
31+
32+
def test_mysql_qualified_name_is_two_part() -> None:
33+
"""MySQL has no schema-within-database; the qualified form is
34+
`db`.`table`, NOT `db`.``.`table`."""
35+
dialect = _get_dialect("mysql")
36+
assert dialect.qualified_name("shop", "", "customers") == "`shop`.`customers`"
37+
38+
39+
def test_mariadb_qualified_name_is_two_part() -> None:
40+
dialect = _get_dialect("mariadb")
41+
assert dialect.qualified_name("shop", "", "customers") == "`shop`.`customers`"
42+
43+
44+
def test_postgresql_qualified_name_uses_schema_only() -> None:
45+
"""PostgreSQL databases are isolated; only schema.table makes sense
46+
for cross-reference within the connected database."""
47+
dialect = _get_dialect("postgresql")
48+
# No db segment expected when schema is present.
49+
assert dialect.qualified_name(None, "public", "users") == '"public"."users"'
50+
51+
52+
def test_sqlserver_qualified_name_is_three_part() -> None:
53+
"""SQL Server explicitly uses [db].[schema].[table]."""
54+
dialect = _get_dialect("mssql")
55+
assert dialect.qualified_name("app", "dbo", "Users") == "[app].[dbo].[Users]"
56+
57+
58+
def test_qualified_name_skips_empty_segments_everywhere() -> None:
59+
"""Regardless of dialect, empty db/schema segments must be omitted,
60+
never rendered as empty-quoted placeholders."""
61+
for db_type in ("mysql", "postgresql", "mssql", "sqlite"):
62+
dialect = _get_dialect(db_type)
63+
# bare table name: single quoted segment, no dot joins.
64+
bare = dialect.qualified_name(None, None, "t")
65+
assert "t" in bare, f"{db_type}: {bare}"
66+
assert "." not in bare, f"{db_type} unexpected joins: {bare}"
67+
# empty schema segment must not produce empty quote pair like `` or "" or [].
68+
out = dialect.qualified_name(None, "", "t")
69+
for marker in ("``", '""', "[]"):
70+
assert marker not in out, f"{db_type} emitted empty-quoted segment: {out}"
71+
72+
73+
def test_qualified_name_escapes_embedded_quote_chars() -> None:
74+
"""Each dialect must escape its own quote char, not leak raw input."""
75+
for db_type, payload, expected_substr in [
76+
("mysql", "app`evil", "app``evil"),
77+
("postgresql", 'app"evil', 'app""evil'),
78+
("mssql", "app]evil", "app]]evil"),
79+
]:
80+
dialect = _get_dialect(db_type)
81+
out = dialect.qualified_name(None, None, payload)
82+
assert expected_substr in out, f"{db_type}: {out}"
83+
84+
85+
# --------------------------------------------------------------------------
86+
# Bug 2: stuck Loading... for unknown table references
87+
# --------------------------------------------------------------------------
88+
89+
90+
class _SchemaHost:
91+
"""Minimal stand-in for AutocompleteMixinHost — just enough to drive
92+
`_get_autocomplete_suggestions`."""
93+
94+
def __init__(self, tables, metadata, columns=None, loading=None):
95+
self._schema_cache = {
96+
"tables": tables,
97+
"views": [],
98+
"columns": columns or {},
99+
"procedures": [],
100+
}
101+
self._table_metadata = metadata
102+
self._columns_loading = loading or set()
103+
self.load_calls: list[str] = []
104+
105+
def _load_columns_for_table(self, table_name: str) -> None:
106+
self.load_calls.append(table_name)
107+
108+
def _build_alias_map(self, text: str) -> dict:
109+
return {}
110+
111+
112+
def test_unknown_table_ref_does_not_stick_on_loading() -> None:
113+
"""`SELECT * FROM shop.cu` parses `shop` as an ALIAS_COLUMN scope. It
114+
isn't a real table. Before the fix the completion engine called
115+
_load_columns_for_table('shop') and returned `Loading...`; the loader
116+
skipped unknown keys silently, so the sentinel never cleared."""
117+
from sqlit.domains.query.ui.mixins.autocomplete_suggestions import AutocompleteSuggestionsMixin
118+
119+
host = _SchemaHost(
120+
tables=["customers", "orders", "products"],
121+
metadata={
122+
"customers": ("", "customers", "shop"),
123+
"shop.customers": ("", "customers", "shop"),
124+
"orders": ("", "orders", "shop"),
125+
"products": ("", "products", "shop"),
126+
},
127+
)
128+
129+
get_suggestions = AutocompleteSuggestionsMixin._get_autocomplete_suggestions.__get__(host)
130+
131+
text = "SELECT * FROM shop.cu"
132+
result = get_suggestions(text, len(text))
133+
134+
assert result != ["Loading..."], (
135+
"unknown table key must not pin Loading... — loader never clears it"
136+
)
137+
assert "shop" not in host.load_calls
138+
139+
140+
def test_known_table_ref_still_triggers_loading_on_first_call() -> None:
141+
"""Sanity check: the fix must not regress legit lazy loading."""
142+
from sqlit.domains.query.ui.mixins.autocomplete_suggestions import AutocompleteSuggestionsMixin
143+
144+
host = _SchemaHost(
145+
tables=["customers"],
146+
metadata={"customers": ("", "customers", None)},
147+
columns={},
148+
)
149+
get_suggestions = AutocompleteSuggestionsMixin._get_autocomplete_suggestions.__get__(host)
150+
151+
text = "SELECT * FROM customers WHERE em"
152+
result = get_suggestions(text, len(text))
153+
154+
assert result == ["Loading..."]
155+
assert "customers" in host.load_calls

0 commit comments

Comments
 (0)