Repository navigation
perf(catalog): ERD neighbourhood and MCP lookups read only the tables they need (#1572) - #1662
Merged
Merged
Conversation
… 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.
…ltered 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1572.
Before (measured)
ErdCatalog.loadNeighbourhood): for every schema in the neighbourhood it ran the column catalog query over all tables of that schema and kept the few it needed in Dart. On PostgreSQL that query has per-column correlated subqueries (unique index, enum labels, comments, defaults), so its cost follows the size of the schema. WithcatalogRowLimit = 200000, a large schema was cut before filtering, and the table the user opened could come back without columns.describe_table/sample_rows: to find one table, three catalog queries over the whole schema (columns, keys, view columns) on every call.sample_rowsused them only to check the name.SQLite (no PostgreSQL here; the app's own SQL run with Python's
sqlite3), tables with 17 columns:On PostgreSQL the gap should be wider (the subqueries run per column), and the rows also travel to the app and get decoded.
Change
columnsSql/viewColumnsSql(…, tables:)addAND <name> IN ('a', 'b')(c.relname/c.table_name/m.name), literal-escaped like the rest of the catalog SQL. An empty list matches nothing;nullis the old whole-schema query.loadNeighbourhood: keys first (unchanged; the walk needs them), then for each PostgreSQL schema only its tables of the neighbourhood.schema.tableasks that schema fortable, which also gives the fallback for a key-less table its bare name.tableNamesSql(names of tables and views only) resolves the name with the same rule as before (exact, else the single case-insensitive match).describe_tablethen loads that table's columns and view columns;sample_rowsneeds only the name.list_tablesand theschema://resource still read the whole catalog, as they should.Tests
erd_test.dart:INfilter;mcp_query_service_test.dart:describeTablesends only filtered column queries, andsampleRows('USERS')resolvesuserswithout any column query. The fakes now answer the names query.Not run with
flutter testlocally; CI decides.