From 8a4122c16d5e43a8b555a520ca7d3805cb9bc38e Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 17 Oct 2026 06:44:00 +0300 Subject: [PATCH 1/3] perf(catalog): ERD neighbourhood and MCP lookups read only the tables they need (#1572) loadNeighbourhood ran the column catalog query for every column of each schema in the neighbourhood and filtered in Dart; describe_table and sample_rows loaded the columns, keys and view columns of the whole schema to find one table. The cost followed the schema, not the request, and a schema over the 200,000-row limit was cut before filtering, so a table could come back without its columns. - columnsSql / viewColumnsSql take tables: an IN filter on the table name (relname, table_name, m.name), literal-escaped. - loadNeighbourhood walks the keys first, then asks each schema only for its tables in the neighbourhood. - MCP resolves the name with tableNamesSql (names only, exact match else the single case-insensitive one, as before), then describe_table loads that table's columns and view columns; sample_rows needs only the name. list_tables and the schema:// resource still read the whole catalog. SQLite, 3,000 tables x 17 columns: the neighbourhood of one table read 51,000 column rows in 225 ms; filtered, 51 rows in 0.7 ms. --- lib/core/erd/erd_catalog.dart | 72 +++++++++++++++--- lib/core/mcp/mcp_query_service.dart | 61 +++++++++++---- test/core/mcp/mcp_query_service_test.dart | 31 +++++++- test/features/erd/erd_test.dart | 90 +++++++++++++++++++---- 4 files changed, 213 insertions(+), 41 deletions(-) diff --git a/lib/core/erd/erd_catalog.dart b/lib/core/erd/erd_catalog.dart index 6cdd320b..d3ccb815 100644 --- a/lib/core/erd/erd_catalog.dart +++ b/lib/core/erd/erd_catalog.dart @@ -26,7 +26,17 @@ class ErdCatalog { /// Columns of [schema] (PostgreSQL) or of the connected database. [schema] /// `null` means the current schema. - static String columnsSql(SqlDialect d, {String? schema}) => switch (d) { + /// + /// [tables] limits it to those bare table names (of [schema]): a diagram + /// around one table or an MCP lookup of one table then reads those tables + /// and not the whole schema, which also kept the row limit from cutting a + /// large schema before the tables that were wanted (#1572). + static String columnsSql( + SqlDialect d, { + String? schema, + Iterable? tables, + }) => + switch (d) { // pg_catalog is readable by every role, unlike information_schema, // which hides keys of tables the current role has only SELECT on. SqlDialect.postgres => ''' @@ -61,7 +71,7 @@ LEFT JOIN pg_catalog.pg_constraint pk LEFT JOIN pg_catalog.pg_attrdef df ON df.adrelid = c.oid AND df.adnum = a.attnum WHERE ns.nspname = ${schema == null ? 'current_schema()' : "'${_lit(schema)}'"} - AND c.relkind IN ('r', 'p') + AND c.relkind IN ('r', 'p')${_nameFilter('c.relname', tables)} AND a.attnum > 0 AND NOT a.attisdropped ORDER BY table_name, a.attnum''', SqlDialect.mysql => ''' @@ -77,7 +87,7 @@ FROM information_schema.columns c JOIN information_schema.tables t ON t.table_schema = c.table_schema AND t.table_name = c.table_name AND t.table_type = 'BASE TABLE' -WHERE c.table_schema = DATABASE() +WHERE c.table_schema = DATABASE()${_nameFilter('c.table_name', tables)} ORDER BY c.table_name, c.ordinal_position''', SqlDialect.sqlite => ''' SELECT m.name AS table_name, p.name AS column_name, p.type AS data_type, @@ -96,14 +106,16 @@ SELECT m.name AS table_name, p.name AS column_name, p.type AS data_type, NULL AS column_comment, NULL AS table_comment FROM sqlite_master m JOIN pragma_table_info(m.name) p -WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%' +WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%'${_nameFilter('m.name', tables)} ORDER BY m.name, p.cid''', }; /// Columns of the views (and PostgreSQL materialized views) of the current /// schema or database, in the shape of [columnsSql]: no primary key, every - /// column nullable. MCP lists them next to the tables. - static String viewColumnsSql(SqlDialect d) => switch (d) { + /// column nullable. MCP lists them next to the tables. [tables] limits it + /// to those names, as in [columnsSql]. + static String viewColumnsSql(SqlDialect d, {Iterable? tables}) => + switch (d) { SqlDialect.postgres => ''' SELECT ${_pgQualified('c', 'ns')} AS table_name, a.attname AS column_name, format_type(a.atttypid, a.atttypmod) AS data_type, @@ -112,7 +124,7 @@ FROM pg_catalog.pg_attribute a JOIN pg_catalog.pg_class c ON c.oid = a.attrelid JOIN pg_catalog.pg_namespace ns ON ns.oid = c.relnamespace WHERE ns.nspname = current_schema() - AND c.relkind IN ('v', 'm') + AND c.relkind IN ('v', 'm')${_nameFilter('c.relname', tables)} AND a.attnum > 0 AND NOT a.attisdropped ORDER BY table_name, a.attnum''', SqlDialect.mysql => ''' @@ -121,13 +133,13 @@ FROM information_schema.columns c JOIN information_schema.tables t ON t.table_schema = c.table_schema AND t.table_name = c.table_name AND t.table_type = 'VIEW' -WHERE c.table_schema = DATABASE() +WHERE c.table_schema = DATABASE()${_nameFilter('c.table_name', tables)} ORDER BY c.table_name, c.ordinal_position''', SqlDialect.sqlite => ''' SELECT m.name AS table_name, p.name AS column_name, p.type AS data_type, 0 AS is_pk, 1 AS is_nullable FROM sqlite_master m JOIN pragma_table_info(m.name) p -WHERE m.type = 'view' AND m.name NOT LIKE 'sqlite_%' +WHERE m.type = 'view' AND m.name NOT LIKE 'sqlite_%'${_nameFilter('m.name', tables)} ORDER BY m.name, p.cid''', }; @@ -165,8 +177,37 @@ WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%' ''', }; + /// Names of the tables and views of the current schema or database, and + /// nothing else: what a lookup by name needs, without reading columns. + static String tableNamesSql(SqlDialect d) => switch (d) { + SqlDialect.postgres => ''' +SELECT c.relname AS object_name +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace ns ON ns.oid = c.relnamespace +WHERE ns.nspname = current_schema() AND c.relkind IN ('r', 'p', 'v', 'm') +ORDER BY 1''', + SqlDialect.mysql => ''' +SELECT table_name AS object_name +FROM information_schema.tables +WHERE table_schema = DATABASE() AND table_type IN ('BASE TABLE', 'VIEW') +ORDER BY 1''', + SqlDialect.sqlite => ''' +SELECT name AS object_name +FROM sqlite_master +WHERE type IN ('table', 'view') AND name NOT LIKE 'sqlite_%' +ORDER BY 1''', + }; + static String _lit(String s) => s.replaceAll("'", "''"); + /// `AND [column] IN ('a', 'b')` for [names], or nothing when [names] is + /// null; an empty list matches no row. + static String _nameFilter(String column, Iterable? names) { + if (names == null) return ''; + if (names.isEmpty) return '\n AND 1 = 0'; + return "\n AND $column IN (${names.map((n) => "'${_lit(n)}'").join(', ')})"; + } + /// Schema part of a diagram name: `sales` for `sales.orders`, `null` for a /// table of the current schema. static String? schemaOf(String name) { @@ -217,11 +258,18 @@ WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%' } var names = neighbourhood(fks, focus, depth); + // Only the columns of these tables, grouped by PostgreSQL schema (#1572). + // `schema.table` asks its schema for `table`, which also brings the bare + // name the fallback below looks for when that schema is the current one. + final wanted = >{}; + for (final n in names) { + final s = dialect == SqlDialect.postgres ? schemaOf(n) : null; + (wanted[s] ??= {}).add(s == null ? n : n.substring(s.length + 1)); + } final fetched = >[]; - final schemas = {for (final n in names) schemaOf(n)}; - for (final s in schemas) { + for (final MapEntry(key: s, value: tables) in wanted.entries) { final rows = await delegate.executeQuery( - columnsSql(dialect, schema: dialect == SqlDialect.postgres ? s : null), + columnsSql(dialect, schema: s, tables: tables), limit: catalogRowLimit, ); truncated = truncated || rows.isTruncated; diff --git a/lib/core/mcp/mcp_query_service.dart b/lib/core/mcp/mcp_query_service.dart index a4d964fd..fdf46817 100644 --- a/lib/core/mcp/mcp_query_service.dart +++ b/lib/core/mcp/mcp_query_service.dart @@ -107,9 +107,10 @@ class McpQueryService { String table, ) { return _withDelegate(connectionId, (delegate, dialect) async { - final cat = await _loadCatalog(delegate, dialect); + final name = await _resolveTable(delegate, dialect, table); + final cat = await _loadCatalog(delegate, dialect, tables: [name]); final schema = cat.schema; - final t = _findTable(schema, table); + final t = _findTable(schema, name); if (cat.views.contains(t.name)) return _describe(schema, t, const []); final indexes = await _run( () => delegate.executeQuery( @@ -162,11 +163,10 @@ class McpQueryService { }) { final n = rows.clamp(1, maxSampleRows); return _withDelegate(connectionId, (delegate, dialect) async { - final t = - _findTable((await _loadCatalog(delegate, dialect)).schema, table); + final name = await _resolveTable(delegate, dialect, table); final result = await _run( () => delegate.executeQuery( - 'SELECT * FROM ${quoteIdentifier(t.name, dialect)}', + 'SELECT * FROM ${quoteIdentifier(name, dialect)}', limit: n, timeout: timeout, ), @@ -242,14 +242,19 @@ class McpQueryService { /// Tables and keys, then the views: a name that is already a table stays a /// table. Views have no keys and no indexes. + /// The catalog of the current schema or database; with [tables], of those + /// tables only (#1572). Future<({ErdSchema schema, Set views})> _loadCatalog( SqlQueryRunner delegate, - SqlDialect dialect, - ) async { - final base = await _loadSchema(delegate, dialect); + SqlDialect dialect, { + Iterable? tables, + }) async { + final base = await _loadSchema(delegate, dialect, tables: tables); final viewRows = await _run( - () => delegate.executeQuery(ErdCatalog.viewColumnsSql(dialect), - limit: _catalogRows, timeout: timeout), + () => delegate.executeQuery( + ErdCatalog.viewColumnsSql(dialect, tables: tables), + limit: _catalogRows, + timeout: timeout), dialect, ); final known = {for (final t in base.tables) t.name}; @@ -271,11 +276,14 @@ class McpQueryService { Future _loadSchema( SqlQueryRunner delegate, - SqlDialect dialect, - ) async { + SqlDialect dialect, { + Iterable? tables, + }) async { final cols = await _run( - () => delegate.executeQuery(ErdCatalog.columnsSql(dialect), - limit: _catalogRows, timeout: timeout), + () => delegate.executeQuery( + ErdCatalog.columnsSql(dialect, tables: tables), + limit: _catalogRows, + timeout: timeout), dialect, ); final fks = await _run( @@ -288,6 +296,31 @@ class McpQueryService { /// Exact name first, then a unique case-insensitive match. The table must /// exist in the catalog, so model input never reaches SQL unchecked. + /// The catalog's name for [name]: the exact one, else the only one that + /// matches ignoring case. Reads the names only, not the whole catalog: one + /// table's description or sample no longer loads every column (#1572). + Future _resolveTable( + SqlQueryRunner delegate, + SqlDialect dialect, + String name, + ) async { + final result = await _run( + () => delegate.executeQuery(ErdCatalog.tableNamesSql(dialect), + limit: _catalogRows, timeout: timeout), + dialect, + ); + final names = [ + for (final r in result.rows) + if (r.isNotEmpty) r.first, + ]; + if (names.contains(name)) return name; + final lower = name.toLowerCase(); + final loose = names.where((n) => n.toLowerCase() == lower).toList(); + if (loose.length == 1) return loose.single; + throw McpToolException( + 'Table "$name" not found. Call list_tables to see the available tables.'); + } + ErdTable _findTable(ErdSchema schema, String name) { final exact = schema.tables.where((t) => t.name == name); if (exact.isNotEmpty) return exact.first; diff --git a/test/core/mcp/mcp_query_service_test.dart b/test/core/mcp/mcp_query_service_test.dart index d51023e4..81fb9a91 100644 --- a/test/core/mcp/mcp_query_service_test.dart +++ b/test/core/mcp/mcp_query_service_test.dart @@ -55,6 +55,12 @@ ConnectionRow _row(int id, String type, {String? env}) { } SqlExecutionResult _catalog(String sql) { + if (sql.contains('object_name')) { + return const SqlExecutionResult(columns: ['n'], rows: [ + ['orders'], + ['users'], + ]); + } if (sql.contains('pg_indexes')) { return const SqlExecutionResult(columns: ['n', 'd'], rows: [ ['users_pkey', 'CREATE UNIQUE INDEX users_pkey ON users (id)'], @@ -226,7 +232,14 @@ void main() { group('views', () { setUp(() { db.onExecute = (sql) { - if (sql == ErdCatalog.viewColumnsSql(SqlDialect.postgres)) { + if (sql.contains('object_name')) { + return const SqlExecutionResult(columns: ['n'], rows: [ + ['active_users'], + ['orders'], + ['users'], + ]); + } + if (sql.contains("relkind IN ('v', 'm')")) { return const SqlExecutionResult(columns: ['t', 'c', 'ty', 'pk'], rows: [ ['active_users', 'id', 'integer', '0'], ['active_users', 'name', 'text', '0'], @@ -272,6 +285,22 @@ void main() { expect(orders.columns.last.references, 'users.id'); }); + test('describeTable and sampleRows read only the table asked for (#1572)', + () async { + await service().describeTable(1, 'users'); + final columnQueries = + db.executed.where((s) => s.contains('pg_attribute')).toList(); + expect(columnQueries, isNotEmpty); + for (final q in columnQueries) { + expect(q, contains("AND c.relname IN ('users')")); + } + + db.executed.clear(); + await service().sampleRows(1, 'USERS', rows: 3); + expect(db.executed.where((s) => s.contains('pg_attribute')), isEmpty); + expect(db.executed.last, 'SELECT * FROM "users"'); + }); + test('an unknown table is an error and runs no table query', () async { await expectLater(service().describeTable(1, 'users; DROP TABLE x'), throwsA(isA())); diff --git a/test/features/erd/erd_test.dart b/test/features/erd/erd_test.dart index b5b140a7..8963656d 100644 --- a/test/features/erd/erd_test.dart +++ b/test/features/erd/erd_test.dart @@ -33,6 +33,11 @@ ErdSchema sample() => ErdSchema.fromCatalog( ], ); +/// [sql] without the table-name filter `columnsSql(tables: ...)` adds, so a +/// fake can recognise the catalog query whatever tables it asks for. +String _unfiltered(String sql) => sql.replaceAll( + RegExp(r"\n AND (c\.relname|c\.table_name|m\.name) IN \([^)]*\)"), ''); + void main() { group('ErdSchema.fromCatalog', () { test('marks PK/FK and drops relations to unknown tables', () { @@ -148,7 +153,7 @@ void main() { test('loadNeighbourhood loads only the tables around the one asked for', () async { final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['a', 'id', 'INTEGER', '1'], ['b', 'id', 'INTEGER', '1'], @@ -181,7 +186,7 @@ void main() { ['users', 'org_id', 'orgs', 'id'], ]); } - if (sql == ErdCatalog.columnsSql(SqlDialect.postgres)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.postgres)) { return const SqlExecutionResult(rows: [ ['users', 'id', 'integer', '1'], ['users', 'org_id', 'integer', '0'], @@ -205,7 +210,7 @@ void main() { test('a PostgreSQL table with no keys is found by its columns', () async { final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.postgres, schema: 'public')) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.postgres, schema: 'public')) { return const SqlExecutionResult(rows: [ ['lonely', 'id', 'integer', '1'], ['other', 'id', 'integer', '1'], @@ -230,13 +235,13 @@ void main() { ['sales.orders', 'customer_id', 'customers', 'id'], ]); } - if (sql == ErdCatalog.columnsSql(SqlDialect.postgres, schema: 'sales')) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.postgres, schema: 'sales')) { return const SqlExecutionResult(rows: [ ['sales.orders', 'id', 'integer', '1'], ['sales.orders', 'customer_id', 'integer', '0'], ]); } - if (sql == ErdCatalog.columnsSql(SqlDialect.postgres)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.postgres)) { return const SqlExecutionResult(rows: [ ['customers', 'id', 'integer', '1'], ['users', 'id', 'integer', '1'], @@ -253,9 +258,66 @@ void main() { expect(s.relations, hasLength(1)); }); - test('load runs both queries through the delegate', () async { + test('the neighbourhood asks only for its tables (#1572)', () async { + final asked = []; + final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { + if (sql == ErdCatalog.foreignKeysSql(SqlDialect.sqlite)) { + return const SqlExecutionResult(rows: [ + ['b', 'a_id', 'a', 'id'], + ]); + } + asked.add(sql); + return const SqlExecutionResult(); + }); + await ErdCatalog.loadNeighbourhood(delegate, SqlDialect.sqlite, table: 'a'); + expect(asked.single, contains("AND m.name IN ('a', 'b')")); + }); + + test('a schema larger than the row limit does not lose the tables asked for', + () async { + // The whole-schema query is cut at the row limit before `target`; the + // filtered one returns it. final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { + if (sql.contains("m.name IN ('target')")) { + return const SqlExecutionResult(rows: [ + ['target', 'id', 'INTEGER', '1'], + ['target', 'name', 'TEXT', '0'], + ]); + } if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + return SqlExecutionResult( + isTruncated: true, + rows: [for (var i = 0; i < 100; i++) ['t$i', 'id', 'INTEGER', '1']], + ); + } + return const SqlExecutionResult(); + }); + final s = await ErdCatalog.loadNeighbourhood( + delegate, + SqlDialect.sqlite, + table: 'target', + ); + expect(s.tables.single.columns.map((c) => c.name), ['id', 'name']); + expect(s.truncated, isFalse); + }); + + test('columnsSql filters by table name in every dialect', () { + expect( + ErdCatalog.columnsSql(SqlDialect.postgres, schema: 'sales', tables: ["o'rders"]), + allOf(contains("nspname = 'sales'"), contains("AND c.relname IN ('o''rders')")), + ); + expect(ErdCatalog.columnsSql(SqlDialect.mysql, tables: ['a', 'b']), + contains("AND c.table_name IN ('a', 'b')")); + expect(ErdCatalog.viewColumnsSql(SqlDialect.sqlite, tables: ['v']), + contains("AND m.name IN ('v')")); + expect(ErdCatalog.columnsSql(SqlDialect.sqlite, tables: const []), + contains('AND 1 = 0')); + expect(ErdCatalog.columnsSql(SqlDialect.sqlite), isNot(contains(' IN ('))); + }); + + test('load runs both queries through the delegate', () async { + final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['a', 'id', 'INTEGER', '1'], ]); @@ -655,7 +717,7 @@ void main() { group('ErdView', () { FakeSqlExecutionDelegate delegate() => FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['users', 'id', 'INTEGER', '1'], ['orders', 'user_id', 'INTEGER', '0'], @@ -902,7 +964,7 @@ void main() { (t) async { await t.binding.setSurfaceSize(const material.Size(1200, 800)); final wide = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return SqlExecutionResult(rows: [ for (var i = 0; i < 20; i++) ['t$i', 'id', 'INTEGER', '1'], ['child', 'id', 'INTEGER', '1'], @@ -1051,7 +1113,7 @@ void main() { (t) async { await t.binding.setSurfaceSize(const material.Size(1200, 800)); final junction = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['orders', 'id', 'INTEGER', '1'], ['order_items', 'order_id', 'INTEGER', '1'], @@ -1087,7 +1149,7 @@ void main() { /// users <- orders; lonely has no relation. FakeSqlExecutionDelegate threeTables() => FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['users', 'id', 'INTEGER', '1'], ['orders', 'id', 'INTEGER', '1'], @@ -1180,7 +1242,7 @@ void main() { testWidgets('a large diagram fits the viewport on first load', (t) async { await t.binding.setSurfaceSize(const material.Size(1200, 800)); final tall = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return SqlExecutionResult(rows: [ for (final table in ['a', 'b', 'c']) for (var i = 0; i < 400; i++) [table, 'col$i', 'INTEGER', '0'], @@ -1244,7 +1306,7 @@ void main() { await t.binding.setSurfaceSize(const material.Size(1200, 800)); String? opened; final sales = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['sales.orders', 'id', 'INTEGER', '1'], ]); @@ -1335,7 +1397,7 @@ void main() { (t) async { await t.binding.setSurfaceSize(const material.Size(1200, 800)); final wide = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult(rows: [ ['users', 'id', 'INTEGER', '1'], ['orders', 'id', 'INTEGER', '1'], @@ -1414,7 +1476,7 @@ void main() { test('a cut catalog is reported as truncated', () async { final delegate = FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return const SqlExecutionResult( rows: [ ['users', 'id', 'INTEGER', '1'], From 44193c7cd96ae2bcdbb185c58e99f62dfafb9499 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 17 Oct 2026 07:00:23 +0300 Subject: [PATCH 2/3] fix(mcp): a described table keeps its references; tests follow the filtered catalog (#1572) describe_table loaded the columns of the asked table only, and ErdSchema drops a relation to a table it has no columns for, so orders.user_id lost its users.id reference. The keys are read first and the tables they point to are loaded with it. Tests: the MCP check now picks column queries by format_type (the key query also joins pg_attribute), the ERD layout test's fake recognises the filtered column query, and an unused import is gone. --- lib/core/mcp/mcp_query_service.dart | 26 +++++++++++++------ test/core/mcp/mcp_query_service_test.dart | 6 ++--- .../erd/erd_layout_persistence_test.dart | 6 ++++- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/lib/core/mcp/mcp_query_service.dart b/lib/core/mcp/mcp_query_service.dart index fdf46817..c78d83a5 100644 --- a/lib/core/mcp/mcp_query_service.dart +++ b/lib/core/mcp/mcp_query_service.dart @@ -279,23 +279,31 @@ class McpQueryService { SqlDialect dialect, { Iterable? tables, }) async { + final fks = await _run( + () => delegate.executeQuery(ErdCatalog.foreignKeysSql(dialect), + limit: _catalogRows, timeout: timeout), + dialect, + ); + // With [tables], also the tables their keys point to: ErdSchema drops a + // relation to a table it has no columns for, and a description lists + // each column's reference. + final wanted = tables == null + ? null + : { + ...tables, + for (final r in fks.rows) + if (r.length >= 4 && tables.contains(r[0])) r[2], + }; final cols = await _run( () => delegate.executeQuery( - ErdCatalog.columnsSql(dialect, tables: tables), + ErdCatalog.columnsSql(dialect, tables: wanted), limit: _catalogRows, timeout: timeout), dialect, ); - final fks = await _run( - () => delegate.executeQuery(ErdCatalog.foreignKeysSql(dialect), - limit: _catalogRows, timeout: timeout), - dialect, - ); return ErdSchema.fromCatalog(columnRows: cols.rows, fkRows: fks.rows); } - /// Exact name first, then a unique case-insensitive match. The table must - /// exist in the catalog, so model input never reaches SQL unchecked. /// The catalog's name for [name]: the exact one, else the only one that /// matches ignoring case. Reads the names only, not the whole catalog: one /// table's description or sample no longer loads every column (#1572). @@ -321,6 +329,8 @@ class McpQueryService { 'Table "$name" not found. Call list_tables to see the available tables.'); } + /// Exact name first, then a unique case-insensitive match. The table must + /// exist in the catalog, so model input never reaches SQL unchecked. ErdTable _findTable(ErdSchema schema, String name) { final exact = schema.tables.where((t) => t.name == name); if (exact.isNotEmpty) return exact.first; diff --git a/test/core/mcp/mcp_query_service_test.dart b/test/core/mcp/mcp_query_service_test.dart index 81fb9a91..3449c010 100644 --- a/test/core/mcp/mcp_query_service_test.dart +++ b/test/core/mcp/mcp_query_service_test.dart @@ -3,7 +3,6 @@ import 'dart:convert'; import 'package:flutter_test/flutter_test.dart'; import 'package:querya_desktop/core/database/table_mutation_engine.dart'; -import 'package:querya_desktop/core/erd/erd_catalog.dart'; import 'package:querya_desktop/core/mcp/mcp_access_store.dart'; import 'package:querya_desktop/core/mcp/mcp_query_service.dart'; import 'package:querya_desktop/core/security/connection_environment.dart'; @@ -288,8 +287,9 @@ void main() { test('describeTable and sampleRows read only the table asked for (#1572)', () async { await service().describeTable(1, 'users'); + // Column and view-column queries (the key query has no format_type). final columnQueries = - db.executed.where((s) => s.contains('pg_attribute')).toList(); + db.executed.where((s) => s.contains('format_type')).toList(); expect(columnQueries, isNotEmpty); for (final q in columnQueries) { expect(q, contains("AND c.relname IN ('users')")); @@ -297,7 +297,7 @@ void main() { db.executed.clear(); await service().sampleRows(1, 'USERS', rows: 3); - expect(db.executed.where((s) => s.contains('pg_attribute')), isEmpty); + expect(db.executed.where((s) => s.contains('format_type')), isEmpty); expect(db.executed.last, 'SELECT * FROM "users"'); }); diff --git a/test/features/erd/erd_layout_persistence_test.dart b/test/features/erd/erd_layout_persistence_test.dart index d9bf63a8..41f19f78 100644 --- a/test/features/erd/erd_layout_persistence_test.dart +++ b/test/features/erd/erd_layout_persistence_test.dart @@ -30,7 +30,7 @@ const _key = ErdLayoutKey(connectionId: 1, scope: 'shop'); FakeSqlExecutionDelegate _catalog({bool withPayments = false}) => FakeSqlExecutionDelegate(onExecute: (sql) { - if (sql == ErdCatalog.columnsSql(SqlDialect.sqlite)) { + if (_unfiltered(sql) == ErdCatalog.columnsSql(SqlDialect.sqlite)) { return SqlExecutionResult(rows: [ const ['users', 'id', 'INTEGER', '1'], const ['orders', 'user_id', 'INTEGER', '0'], @@ -45,6 +45,10 @@ FakeSqlExecutionDelegate _catalog({bool withPayments = false}) => Finder _card(String table) => find.byKey(material.ValueKey('erd_table_$table')); +/// [sql] without the table-name filter `columnsSql(tables: ...)` adds (#1572). +String _unfiltered(String sql) => sql.replaceAll( + RegExp(r"\n AND (c\.relname|c\.table_name|m\.name) IN \([^)]*\)"), ''); + void main() { Future pumpDiagram( WidgetTester t, From 0e4f7de66128b7bd5180527bb015f7d481ed6532 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 17 Oct 2026 07:06:05 +0300 Subject: [PATCH 3/3] test(workspace): the relations view fake recognises the filtered column query (#1572) --- .../workspace/generic_table_view_relations_test.dart | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/features/workspace/generic_table_view_relations_test.dart b/test/features/workspace/generic_table_view_relations_test.dart index 2e935946..ae5d654c 100644 --- a/test/features/workspace/generic_table_view_relations_test.dart +++ b/test/features/workspace/generic_table_view_relations_test.dart @@ -23,8 +23,9 @@ TableDataPage _catalog(String sql) { ['orgs', 'region_id', 'regions', 'id'], ]); } - if (sql == ErdCatalog.columnsSql(d) || - sql == ErdCatalog.columnsSql(d, schema: 'public')) { + final columns = _unfiltered(sql); + if (columns == ErdCatalog.columnsSql(d) || + columns == ErdCatalog.columnsSql(d, schema: 'public')) { return const TableDataPage(columns: ['t', 'c', 'ty', 'pk'], rows: [ ['users', 'id', 'integer', '1'], ['users', 'org_id', 'integer', '0'], @@ -55,6 +56,10 @@ Iterable _opacityOf(WidgetTester t, String table) => matching: find.byType(material.Opacity), )).map((o) => o.opacity); +/// [sql] without the table-name filter `columnsSql(tables: ...)` adds (#1572). +String _unfiltered(String sql) => sql.replaceAll( + RegExp(r"\n AND (c\.relname|c\.table_name|m\.name) IN \([^)]*\)"), ''); + void main() { setUp(() => TableViewCommandBridge.instance.resetForTest());