Skip to content

Commit b70acf4

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 name-builder inserted it unconditionally between backticks, producing a three-part identifier with an empty middle. Add a small `_qualify_table_name` helper that skips empty db/schema segments, and route the four inline builders (tables/views × sync/async loader) through it. 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 7 new unit tests pinning the name-builder across several dialects and the stuck-Loading regression. Closes #151.
1 parent a132802 commit b70acf4

3 files changed

Lines changed: 166 additions & 32 deletions

File tree

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

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@
1212
SCHEMA_PROCESS_BATCH_SIZE = 200
1313

1414

15+
def _qualify_table_name(dialect: Any, database: str | None, schema: str, name: str) -> str:
16+
"""Build a quoted qualified identifier, skipping empty db/schema segments.
17+
18+
MySQL/MariaDB have no schema-within-database, so `schema` comes back as
19+
an empty string. Joining `db`.`` empty ``.`table` unconditionally produces
20+
garbage like `db`.``.`table`; skip empties instead.
21+
"""
22+
parts: list[str] = []
23+
if database:
24+
parts.append(dialect.quote_identifier(database))
25+
if schema:
26+
parts.append(dialect.quote_identifier(schema))
27+
parts.append(dialect.quote_identifier(name))
28+
return ".".join(parts)
29+
30+
1531
class AutocompleteSchemaMixin:
1632
"""Mixin providing schema loading and caching for autocomplete."""
1733

@@ -406,13 +422,7 @@ def work() -> None:
406422
if single_db:
407423
full_name = table_name
408424
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}"
425+
full_name = _qualify_table_name(dialect, database, schema_name, table_name)
416426

417427
display_name = dialect.format_table_name(schema_name, table_name)
418428
metadata = [
@@ -495,13 +505,7 @@ def work() -> None:
495505
if single_db:
496506
full_name = view_name
497507
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}"
508+
full_name = _qualify_table_name(dialect, database, schema_name, view_name)
505509

506510
display_name = dialect.format_table_name(schema_name, view_name)
507511
metadata = [
@@ -816,14 +820,8 @@ async def run_db_call(fn: Any, *args: Any, **kwargs: Any) -> Any:
816820
# Single database - use simple table name
817821
schema_cache["tables"].append(table_name)
818822
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}"
823+
# Multiple databases - use qualified identifier
824+
full_name = _qualify_table_name(dialect, database, schema_name, table_name)
827825
schema_cache["tables"].append(full_name)
828826
# Keep metadata for column loading (multiple keys for flexible lookup)
829827
display_name = dialect.format_table_name(schema_name, table_name)
@@ -843,14 +841,8 @@ async def run_db_call(fn: Any, *args: Any, **kwargs: Any) -> Any:
843841
# Single database - use simple view name
844842
schema_cache["views"].append(view_name)
845843
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}"
844+
# Multiple databases - use qualified identifier
845+
full_name = _qualify_table_name(dialect, database, schema_name, view_name)
854846
schema_cache["views"].append(full_name)
855847
# Keep metadata for column loading (multiple keys for flexible lookup)
856848
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: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
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.
7+
2. Autocomplete returned a permanent "Loading..." sentinel whenever the
8+
table reference in the query didn't resolve to anything in the schema
9+
cache (e.g. the user typed `SELECT * FROM shop.cu`: `shop` was treated
10+
as an alias and the loader spun forever for an unknown key).
11+
"""
12+
13+
from __future__ import annotations
14+
15+
from unittest.mock import MagicMock
16+
17+
from sqlit.domains.query.ui.mixins.autocomplete_schema import _qualify_table_name
18+
19+
20+
class _FakeDialect:
21+
def quote_identifier(self, name: str) -> str:
22+
escaped = name.replace("`", "``")
23+
return f"`{escaped}`"
24+
25+
26+
def test_qualify_skips_empty_schema_segment() -> None:
27+
"""MySQL/MariaDB have no schemas within databases; the schema slot
28+
arrives as ''. The qualified name must be `db`.`table`, not the
29+
pre-fix `db`.``.`table`."""
30+
result = _qualify_table_name(_FakeDialect(), "shop", "", "customers")
31+
assert result == "`shop`.`customers`"
32+
33+
34+
def test_qualify_skips_empty_database_segment() -> None:
35+
result = _qualify_table_name(_FakeDialect(), None, "public", "users")
36+
assert result == "`public`.`users`"
37+
38+
39+
def test_qualify_full_three_part() -> None:
40+
"""Normal case: both db and schema present (e.g. SQL Server)."""
41+
result = _qualify_table_name(_FakeDialect(), "app", "dbo", "Users")
42+
assert result == "`app`.`dbo`.`Users`"
43+
44+
45+
def test_qualify_only_table_when_both_empty() -> None:
46+
result = _qualify_table_name(_FakeDialect(), None, "", "customers")
47+
assert result == "`customers`"
48+
49+
50+
def test_qualify_escapes_embedded_backticks() -> None:
51+
result = _qualify_table_name(_FakeDialect(), "app`evil", "", "tbl")
52+
# Embedded backticks are doubled by the dialect quoter.
53+
assert result == "`app``evil`.`tbl`"
54+
55+
56+
class _SchemaHost:
57+
"""Minimal stand-in for the AutocompleteMixin protocol — just enough to
58+
drive `_get_autocomplete_suggestions`."""
59+
60+
def __init__(self, tables, metadata, columns=None, loading=None):
61+
self._schema_cache = {
62+
"tables": tables,
63+
"views": [],
64+
"columns": columns or {},
65+
"procedures": [],
66+
}
67+
self._table_metadata = metadata
68+
self._columns_loading = loading or set()
69+
self.load_calls: list[str] = []
70+
71+
def _load_columns_for_table(self, table_name: str) -> None:
72+
self.load_calls.append(table_name)
73+
74+
# _get_autocomplete_suggestions depends on these helpers from the real
75+
# mixin; stub them with minimal plausible behavior for these tests.
76+
def _build_alias_map(self, text: str) -> dict:
77+
return {}
78+
79+
80+
def test_unknown_table_ref_does_not_stick_on_loading() -> None:
81+
"""Issue #151: `SELECT * FROM shop.cu` parses `shop` as an ALIAS_COLUMN
82+
scope, which isn't in table_metadata. Before the fix the completion
83+
engine called _load_columns_for_table('shop') and returned Loading...;
84+
the loader skipped unknown keys silently, so the sentinel never cleared.
85+
"""
86+
from sqlit.domains.query.ui.mixins.autocomplete_suggestions import AutocompleteSuggestionsMixin
87+
88+
host = _SchemaHost(
89+
tables=["customers", "orders", "products"],
90+
metadata={
91+
"customers": ("", "customers", "shop"),
92+
"shop.customers": ("", "customers", "shop"),
93+
"orders": ("", "orders", "shop"),
94+
"shop.orders": ("", "orders", "shop"),
95+
"products": ("", "products", "shop"),
96+
"shop.products": ("", "products", "shop"),
97+
},
98+
)
99+
100+
# Bind the mixin method to the host so `self` resolves correctly.
101+
get_suggestions = AutocompleteSuggestionsMixin._get_autocomplete_suggestions.__get__(host)
102+
103+
text = "SELECT * FROM shop.cu"
104+
result = get_suggestions(text, len(text))
105+
106+
assert result != ["Loading..."], (
107+
"unknown table key must not pin Loading... — loader never clears it"
108+
)
109+
# Should not have triggered a load for 'shop' (not a real table).
110+
assert "shop" not in host.load_calls
111+
112+
113+
def test_known_table_ref_still_triggers_loading_on_first_call() -> None:
114+
"""Sanity check: the fix must not regress legit lazy loading.
115+
When the user references a real table whose columns aren't cached yet,
116+
we still return Loading... and schedule the loader."""
117+
from sqlit.domains.query.ui.mixins.autocomplete_suggestions import AutocompleteSuggestionsMixin
118+
119+
host = _SchemaHost(
120+
tables=["customers"],
121+
metadata={"customers": ("", "customers", None)},
122+
columns={}, # not loaded yet
123+
)
124+
get_suggestions = AutocompleteSuggestionsMixin._get_autocomplete_suggestions.__get__(host)
125+
126+
text = "SELECT * FROM customers WHERE em"
127+
result = get_suggestions(text, len(text))
128+
129+
assert result == ["Loading..."]
130+
assert "customers" in host.load_calls

0 commit comments

Comments
 (0)