perf(Schema): bulk table introspection (getSchemas) - #265
Open
roxblnfk wants to merge 7 commits into
Open
Conversation
Introduce BulkSchemaProviderInterface::getSchemas(array $tables, ?string $prefix) so a set of tables can be introspected together instead of one full introspection per table. The table list is mandatory by design — the queries must never read catalog rows of tables the caller did not ask for; the "whole database" case is the explicit Database::getSchemas(null) path that first resolves names via getTableNames(). The base Handler ships a per-table-loop implementation (correct for every driver) that batching drivers override; ReadonlyHandler delegates. AbstractTable gains an internal prefetch construction path: when raw rows are supplied it derives existence from their presence (no hasTable() query) and drops the payload after the initial load so any later re-introspection still hits the database. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Override getSchemas() to read the whole set of tables with a constant number of queries. Every query is the per-table one widened from `table_schema = ? AND table_name = ?` to a row-value `IN (...)` list, grouped back per table in PHP and injected into PostgresTable via the prefetch path. Because the SQL and the createInstance factories are unchanged, the bulk result is identical to getSchema() by construction. The pair list is chunked so the bound parameter count stays bounded on any set size. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Override getSchemas() to introspect the whole set with a constant number of queries: the per-table `table_name = ?` filters become `table_name IN (...)`, and DEFAULT/CHECK constraints are read once for all object ids (their keys are database-wide unique). Foreign keys — normally one sp_fkeys call per table, which cannot be batched — are read for the whole set from sys.foreign_keys in a shape that matches the sp_fkeys rows SQLServerForeignKey consumes, with the referential action mapped back to sp_fkeys semantics (0 = CASCADE, non-zero = NO ACTION). Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assert, per driver, that getSchemas() returns a schema identical to the per-table getSchema() (via assertSameAsInDB), that non-existent tables come back as empty STATUS_NEW schemas rather than being skipped, that the prefix is honored, and that Database::getSchemas() works both with an explicit list and for the whole database. Composite primary keys, foreign keys and indexes are covered specifically because that is where the batched queries rely on ORDER BY to reproduce the per-table column order. ReadonlyHandler delegation is covered too. Batching drivers (Postgres, SQLServer) additionally assert the query count does not grow with the number of tables; loop drivers (MySQL, SQLite) skip that check. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
type: feature
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 2.x #265 +/- ##
============================================
+ Coverage 95.48% 95.50% +0.02%
- Complexity 2119 2209 +90
============================================
Files 141 142 +1
Lines 5974 6312 +338
============================================
+ Hits 5704 6028 +324
- Misses 270 284 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Without an explicit ORDER BY, Postgres and SQL Server return catalog rows in planner-dependent order once several tables are fetched in one query, so bulk-introspected tables could get a column order different from the per-table path (varchar columns drifting to the end). Order columns by ordinal position and Postgres index rows by name. Assisted-By: Claude Fable 5 <noreply@anthropic.com>
assertSameAsInDB matches columns by name, so the ordering regression fixed in the previous commit was invisible to the existing identity tests. The new case introspects 30 tables with interleaved column types — enough rows for the Postgres planner to prefer a hash join, which is what scrambled the order — and compares the exact column order against the per-table path. Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Add DB-free unit tests for the two defensive fallbacks that the functional suite cannot reach, since every built-in handler implements BulkSchemaProviderInterface: ReadonlyHandler and Database::getSchemas() both loop over getSchema() when the wrapped handler does not support bulk introspection. Mocking a plain HandlerInterface (which is not a BulkSchemaProviderInterface) exercises exactly those branches. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a bulk schema introspection API to reduce round-trips by fetching schema metadata for multiple tables in one batched operation (where supported), while preserving parity with existing per-table introspection behavior.
Changes:
- Added
BulkSchemaProviderInterface::getSchemas(array $tables, ?string $prefix)and a default per-table fallback implementation in the baseHandler. - Implemented batched introspection for Postgres and SQL Server using
IN (...)catalog queries and aPrefetchedIntrospectioncarrier consumed byAbstractTableduring initial load. - Added
Database::getSchemas(?array $tables = null)plus functional/unit tests validating bulk↔per-table identity, missing-table semantics, prefix handling, and query-count behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Database/Unit/Driver/ReadonlyHandlerTest.php | Adds unit coverage for ReadonlyHandler::getSchemas() fallback behavior when parent lacks bulk support. |
| tests/Database/Unit/DatabaseTest.php | Adds unit coverage for Database::getSchemas() fallback behavior and prefix application. |
| tests/Database/Functional/Driver/Common/Schema/BulkIntrospectionTest.php | Introduces shared functional assertions for bulk introspection identity, ordering, missing tables, prefix, and query counts. |
| tests/Database/Functional/Driver/Postgres/Schema/BulkIntrospectionTest.php | Enables the shared bulk introspection suite for Postgres and asserts it is batched. |
| tests/Database/Functional/Driver/SQLServer/Schema/BulkIntrospectionTest.php | Enables the shared bulk introspection suite for SQL Server and asserts it is batched. |
| tests/Database/Functional/Driver/MySQL/Schema/BulkIntrospectionTest.php | Enables the shared bulk introspection suite for MySQL (non-batched path). |
| tests/Database/Functional/Driver/SQLite/Schema/BulkIntrospectionTest.php | Enables the shared bulk introspection suite for SQLite (non-batched path). |
| src/Schema/PrefetchedIntrospection.php | Adds an internal carrier for per-table raw introspection buckets produced by bulk queries. |
| src/Schema/AbstractTable.php | Adds a prefetch-based initialization path to build schemas without per-table queries and then discard prefetched rows. |
| src/Driver/BulkSchemaProviderInterface.php | Adds the new bulk-introspection interface contract and documentation. |
| src/Driver/Handler.php | Implements BulkSchemaProviderInterface with a default per-table loop. |
| src/Driver/HandlerInterface.php | Updates docblocks/return typing for table-name listing. |
| src/Driver/ReadonlyHandler.php | Implements bulk introspection delegation with a safe per-table fallback when the wrapped handler isn’t a bulk provider. |
| src/Driver/Postgres/PostgresHandler.php | Implements batched Postgres catalog reads and groups rows into per-table prefetch buckets. |
| src/Driver/Postgres/Schema/PostgresTable.php | Consumes prefetched buckets for columns, references, and index rows when present. |
| src/Driver/SQLServer/SQLServerHandler.php | Implements batched SQL Server catalog reads (including FKs via sys.foreign_keys) and prepares per-table prefetch buckets. |
| src/Driver/SQLServer/Schema/SQLServerTable.php | Consumes prefetched buckets for columns/indexes/FKs/PKs when present. |
| src/Database.php | Exposes Database::getSchemas(?array $tables = null) and the “whole database” resolution path when called with no list. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+22
to
+24
| * "all tables of the database" scenario is an explicit path in | ||
| * {@see \Cycle\Database\Database::getTables()} that first resolves the names via | ||
| * {@see HandlerInterface::getTableNames()} and then passes them here. |
3 tasks
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.
🔍 What was changed
Introduce bulk schema introspection so a set of tables can be read together instead of one full introspection per table.
BulkSchemaProviderInterface::getSchemas(array $tables, ?string $prefix), implemented by the baseHandlerand exposed through the newDatabase::getSchemas(?array $tables = null).WHERE table_name = ?filters becomeIN (...), grouped back per table in PHP. SQLServer foreign keys, normally onesp_fkeyscall per table (not batchable), are read for the whole set fromsys.foreign_keys.Handlerloop). SQLite is embedded so batching buys nothing; MySQL needs aSHOW→information_schemamigration first, tracked as a follow-up spec.Database::getSchemas(null), which resolves the names first and then feeds them in.How it works
AbstractTablegains an internal prefetch path: given the raw rows it builds the schema through the samecreateInstance()factories as the per-table path, so the two results are identical by construction.hasTable()query; a table that does not exist yet comes back as an emptySTATUS_NEWschema rather than being skipped.Why?
Continues the introspection-performance work tracked in cycle/orm#466 ("Cycle schema hydration is too slow"). Per-table query batching already removed the N+1 within a single table, but a schema of N tables still costs
N × ~5sequential round-trips, which dominates bootstrap on a remote database at ~100 ms latency. Bulk introspection collapses that to a constant number of queries for a given set of tables. Thecycle/schema-builderconsumer (lazy, batched loading viaDatabase::getSchemas()) lands in a separate PR there.Checklist
BulkIntrospectionTestrun locally against Postgres, MySQL, SQL Server and SQLite (Docker), green with no expectation changes.BulkIntrospectionTestacross all four drivers: per-table ↔ bulk identity (assertSameAsInDB), composite PK/FK/index ordering, missing tables →STATUS_NEW, prefix handling,ReadonlyHandlerdelegation, and (Postgres/SQLServer) query count independent of the number of tables.Review notes
sp_fkeyssemantics (0= CASCADE, non-zero = NO ACTION) soSQLServerForeignKey::createInstance()stays untouched —sys.foreign_keysuses the inverse encoding.getSchemas()is additive.