Skip to content

Commit 3f7a5ee

Browse files
Merge pull request #1245 from QueryaHub/fix/1171-schema-cache-per-table
perf(sql-workspace): table schema lookups are cached per table, not per query text (#1171)
2 parents de9aff8 + 8bbc373 commit 3f7a5ee

2 files changed

Lines changed: 38 additions & 5 deletions

File tree

‎lib/features/workspace/generic_sql_workspace.dart‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -506,8 +506,15 @@ class GenericSqlWorkspaceState extends material.State<GenericSqlWorkspace> {
506506

507507
final Map<String, SqlResultGridSchema> _gridSchemaCache = {};
508508

509-
static String _schemaCacheKey(String sql, List<String> cols) =>
510-
'${cols.join('\u0001')}\u0000$sql';
509+
/// Table schemas are cached per (database, schema, table), so any query on a
510+
/// table reuses the lookup, not only the same text. A query with no single
511+
/// target table is not cached.
512+
String? _schemaCacheKey(String sql) {
513+
final target = SqlTableTargetExtractor.extract(sql);
514+
if (target == null) return null;
515+
return '$effectiveDatabase\u0001${target.schema ?? ''}\u0001'
516+
'${target.tableName}';
517+
}
511518

512519
static final _readQueryRegex = RegExp(
513520
r'^\s*(select|with|values|show|explain|pragma|describe)\b',
@@ -725,8 +732,8 @@ class GenericSqlWorkspaceState extends material.State<GenericSqlWorkspace> {
725732

726733
// Rows show now. The table schema (keys, types, edit hint) follows: from
727734
// the cache, or from the delegate in the background.
728-
final cacheKey = _schemaCacheKey(shownSql, cols);
729-
final cached = _gridSchemaCache[cacheKey];
735+
final cacheKey = _schemaCacheKey(shownSql);
736+
final cached = cacheKey == null ? null : _gridSchemaCache[cacheKey];
730737
invalidatePane(session);
731738
setState(() {
732739
session.columns = cols;
@@ -753,7 +760,11 @@ class GenericSqlWorkspaceState extends material.State<GenericSqlWorkspace> {
753760
widget.delegate.resolveTableSchema(shownSql, cols).then(
754761
(gridSchema) {
755762
if (!mounted) return;
756-
_gridSchemaCache[cacheKey] = gridSchema;
763+
// A result with no columns gets no schema from the delegate, so
764+
// it is not worth keeping for the table.
765+
if (cacheKey != null && cols.isNotEmpty) {
766+
_gridSchemaCache[cacheKey] = gridSchema;
767+
}
757768
_applyGridSchema(session, shownSql, cols, outRows, gridSchema,
758769
result, scriptStatus);
759770
},

‎test/features/workspace/generic_sql_workspace_schema_test.dart‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,26 @@ void main() {
167167
await runAndWait(tester, state);
168168
expect(delegate.probes, 1);
169169
});
170+
171+
testWidgets('another query on the same table reuses the lookup (#1171)',
172+
(tester) async {
173+
final delegate = _SchemaDelegate();
174+
final state = await pumpWorkspace(tester, delegate, 'select * from users');
175+
await runAndWait(tester, state);
176+
state.activeSession.controller.text = 'select name from users';
177+
await runAndWait(tester, state);
178+
expect(delegate.lookups.length, 1);
179+
});
180+
181+
testWidgets('a DDL run makes the next query on the table look it up again',
182+
(tester) async {
183+
final delegate = _SchemaDelegate();
184+
final state = await pumpWorkspace(tester, delegate, 'select * from users');
185+
await runAndWait(tester, state);
186+
state.activeSession.controller.text = 'alter table users add column age int';
187+
await runAndWait(tester, state);
188+
state.activeSession.controller.text = 'select * from users';
189+
await runAndWait(tester, state);
190+
expect(delegate.lookups.length, 3);
191+
});
170192
}

0 commit comments

Comments
 (0)