refactor(schema/builder): no methods that throw — gate dialect features per type#9
Open
refactor(schema/builder): no methods that throw — gate dialect features per type#9
Conversation
…-dialect Tables Moves dialect-specific Schema methods off the abstract base into feature interfaces and traits, then splits the fluent Table/Column/ForeignKey builder per-dialect so methods that fail on a given dialect are gone from its type, not throwing at runtime. New feature interfaces under Schema/Feature: Views, ReplaceView, Databases, RenameIndex, AnalyzeTable, Partitioning. Default implementations live in matching Schema/Trait/ traits and dialects opt-in via implements + use. SQLite no longer exposes Procedures/Triggers/Databases/RenameIndex/ReplaceView; MongoDB only exposes Views/Databases/AnalyzeTable. The base Schema::compileCreate now gates partition emission via instanceof Partitioning and the column-level/table-level TTL guards are gone (only ClickHouse constructs that state). Schema::table() becomes abstract and each dialect returns its own Table\X via covariant return. The shared Table\Trait\* traits supply optional surface (ForeignKeys, Checks, CompositePrimary, StandardPartitioning, FulltextSpatialIndex). Per-dialect Column\X / ForeignKey\X classes compose the Schema\Forwarder\X traits so chained column-to-table forwarders only exist for methods the dialect supports. ClickHouse-specific knobs (engine, orderBy, ttl, settings, partitionBy(expr)) live directly on Table\ClickHouse. Tests: drop now-impossible UnsupportedException assertions, add ForwarderTest covering each dialect-specific forwarder for success/failure/edge cases.
Pulls Spatial, FullTextSearch, and Upsert off the SQL abstract base so SQLite stops inheriting filterDistance/filterSearch/etc. that only threw at compile time. Each SQL dialect now opts in via implements + use of the matching trait. The Upsert interface is split into three single-method interfaces: Upsert (upsert), InsertOrIgnore (insertOrIgnore), and UpsertSelect (upsertSelect). MongoDB implements Upsert + InsertOrIgnore (it has real impls) and no longer exposes the upsertSelect() that was a runtime UnsupportedException. The default-throw upsert() on the base Builder class is removed; builders opt in. PostgreSQL aliases the trait methods (baseUpsert/baseUpsertSelect) so its overrides can wrap with appendReturning while still sharing the trait body. Tests: drop now-impossible UnsupportedException assertions for SQLite spatial/search and MongoDB upsertSelect.
📊 Coverage
Full per-file breakdown in the job summary. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Schema and Builder dialect APIs to remove runtime UnsupportedException throws from the public surface by gating features at the type level (methods that aren’t supported by a dialect should not exist on that dialect’s types).
Changes:
- Introduces per-feature Schema interfaces + shared trait implementations, and updates dialect Schema classes to opt in explicitly.
- Introduces per-dialect
Table/*,Column/*,ForeignKey/*, andForwarder/*types to gate fluent-chain methods by dialect. - Updates unit/integration tests to match the new typed surfaces and to cover forwarder method exposure.
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Query/Schema/TableTest.php | Switches test to a dialect-specific Table type (PostgreSQL). |
| tests/Query/Schema/SQLiteTest.php | Removes tests that asserted runtime-throws for unsupported features; aligns with feature gating. |
| tests/Query/Schema/MySQLTest.php | Removes vector “throws” test (vector should be absent/unsupported by type). |
| tests/Query/Schema/MongoDBTest.php | Removes composite primary runtime-throw test; aligns with type gating. |
| tests/Query/Schema/ForwarderTest.php | Adds coverage to ensure forwarder traits expose only dialect-supported methods. |
| tests/Query/Schema/FluentBuilderTest.php | Adjusts fluent tests to use dialect-specific tables and removes TTL runtime-throw tests. |
| tests/Query/Schema/ClickHouseTest.php | Removes FK runtime-throw tests and updates partition API call. |
| tests/Query/Regression/CorrectnessRegressionTest.php | Removes base upsert runtime-throw regression test (method removed/gated). |
| tests/Query/Builder/SQLiteTest.php | Removes runtime-throw tests for unsupported spatial predicates (method removed/gated). |
| tests/Query/Builder/MongoDBTest.php | Removes runtime-throw test for upsertSelect() (method removed/gated). |
| tests/Integration/Schema/ClickHouseIntegrationTest.php | Updates ClickHouse partition call to new API (partitionBy). |
| src/Query/Schema/Trait/Views.php | Adds shared SQL view DDL helpers (create/drop). |
| src/Query/Schema/Trait/Triggers.php | Adds shared SQL trigger DDL helpers. |
| src/Query/Schema/Trait/ReplaceView.php | Adds shared SQL “create or replace view” helper. |
| src/Query/Schema/Trait/RenameIndex.php | Adds shared SQL rename-index helper (dialects can override). |
| src/Query/Schema/Trait/Procedures.php | Adds shared SQL stored-procedure helpers with param validation. |
| src/Query/Schema/Trait/Partitioning.php | Adds shared compilation of partitioning fragments for supporting dialects. |
| src/Query/Schema/Trait/ForeignKeys.php | Adds shared SQL FK DDL helpers (dialects can override specifics). |
| src/Query/Schema/Trait/Databases.php | Adds shared create/drop database helpers. |
| src/Query/Schema/Trait/AnalyzeTable.php | Adds shared analyze-table helper (dialects can override specifics). |
| src/Query/Schema/Table/Trait/StandardPartitioning.php | Adds table-side partitioning builder methods (range/list/hash). |
| src/Query/Schema/Table/Trait/FulltextSpatialIndex.php | Adds table-side fulltext/spatial index helpers. |
| src/Query/Schema/Table/Trait/ForeignKeys.php | Adds table-side FK registration helpers (create/alter). |
| src/Query/Schema/Table/Trait/CompositePrimary.php | Adds table-side composite primary helper with validation. |
| src/Query/Schema/Table/Trait/Checks.php | Adds table-side named check constraint registration. |
| src/Query/Schema/Table/SQLite.php | Introduces SQLite-specific Table returning SQLite-specific Column/FK types. |
| src/Query/Schema/Table/PostgreSQL.php | Introduces PostgreSQL-specific Table with partitioning, fulltext/spatial, vector, FK support. |
| src/Query/Schema/Table/MySQL.php | Introduces MySQL-specific Table with partitioning, fulltext/spatial, FK support. |
| src/Query/Schema/Table/MongoDB.php | Introduces MongoDB-specific Table (no FK), with vector column support. |
| src/Query/Schema/Table/ClickHouse.php | Introduces ClickHouse-specific Table (engine/orderBy/settings/ttl/partitionBy/vector). |
| src/Query/Schema/Table.php | Refactors base Table to support dialect-specific subclasses + trait composition. |
| src/Query/Schema/SQLite.php | Updates SQLite schema to implement only supported feature interfaces and return Table\\SQLite. |
| src/Query/Schema/SQL.php | Simplifies SQL schema base to avoid exposing features by default. |
| src/Query/Schema/PostgreSQL.php | Updates PostgreSQL schema to opt into explicit feature interfaces/traits and return Table\\PostgreSQL. |
| src/Query/Schema/MySQL.php | Updates MySQL schema to opt into explicit feature interfaces/traits and return Table\\MySQL. |
| src/Query/Schema/MongoDB.php | Updates MongoDB schema to implement Views/Databases/AnalyzeTable and return Table\\MongoDB. |
| src/Query/Schema/Forwarder/SQLite.php | Adds SQLite forwarder trait to expose only SQLite column/FK forwarders. |
| src/Query/Schema/Forwarder/PostgreSQL.php | Adds PostgreSQL forwarder trait including partitioning/fulltext/spatial/vector/FK. |
| src/Query/Schema/Forwarder/MySQL.php | Adds MySQL forwarder trait including partitioning/fulltext/spatial/FK. |
| src/Query/Schema/Forwarder/MongoDB.php | Adds MongoDB forwarder trait (vector only). |
| src/Query/Schema/Forwarder/ClickHouse.php | Adds ClickHouse forwarder trait (engine/orderBy/settings/partitionBy/vector). |
| src/Query/Schema/ForeignKey/SQLite.php | Adds SQLite-specific FK type with only table-level operations needed. |
| src/Query/Schema/ForeignKey/PostgreSQL.php | Adds PostgreSQL-specific FK type with table-level operations needed. |
| src/Query/Schema/ForeignKey/MySQL.php | Adds MySQL-specific FK type with table-level operations needed. |
| src/Query/Schema/ForeignKey.php | Removes broad FK forwarders from base type; narrows surface area. |
| src/Query/Schema/Feature/Views.php | Adds Views feature interface. |
| src/Query/Schema/Feature/ReplaceView.php | Adds ReplaceView feature interface. |
| src/Query/Schema/Feature/RenameIndex.php | Adds RenameIndex feature interface. |
| src/Query/Schema/Feature/Partitioning.php | Adds Partitioning feature interface. |
| src/Query/Schema/Feature/Databases.php | Adds Databases feature interface. |
| src/Query/Schema/Feature/AnalyzeTable.php | Adds AnalyzeTable feature interface. |
| src/Query/Schema/Column/SQLite.php | Adds SQLite-specific Column with dual-purpose primary/check + SQLite forwarders. |
| src/Query/Schema/Column/PostgreSQL.php | Adds PostgreSQL-specific Column with dual-purpose primary/check + PostgreSQL forwarders. |
| src/Query/Schema/Column/MySQL.php | Adds MySQL-specific Column with dual-purpose primary/check + MySQL forwarders. |
| src/Query/Schema/Column/MongoDB.php | Adds MongoDB-specific Column with vector forwarder. |
| src/Query/Schema/Column/ClickHouse.php | Adds ClickHouse-specific Column with dual-purpose primary + ClickHouse forwarders. |
| src/Query/Schema/Column.php | Refactors base Column API and forwarders for dialect-specific chaining. |
| src/Query/Schema/ClickHouse.php | Updates ClickHouse schema to opt into Views/Databases and return Table\\ClickHouse. |
| src/Query/Schema.php | Makes Schema::table() abstract; moves partition compilation behind feature interface. |
| src/Query/Builder/Trait/UpsertSelect.php | Adds shared SQL upsertSelect() implementation as a trait. |
| src/Query/Builder/Trait/Upsert.php | Removes upsertSelect() from Upsert trait (now split). |
| src/Query/Builder/SQLite.php | Opts SQLite builder into InsertOrIgnore/Upsert/UpsertSelect features and traits. |
| src/Query/Builder/SQL.php | Removes default FullTextSearch/Spatial/Upsert from SQL base; narrows default surface. |
| src/Query/Builder/PostgreSQL.php | Opts PostgreSQL into feature interfaces and wraps Upsert/UpsertSelect to append RETURNING. |
| src/Query/Builder/MySQL.php | Opts MySQL into feature interfaces and adds Upsert/UpsertSelect/Spatial/FullTextSearch traits. |
| src/Query/Builder/MongoDB.php | Removes upsertSelect() runtime-throw method and opts into InsertOrIgnore feature. |
| src/Query/Builder/Feature/UpsertSelect.php | Adds UpsertSelect feature interface. |
| src/Query/Builder/Feature/Upsert.php | Narrows Upsert feature interface to upsert() only. |
| src/Query/Builder/Feature/InsertOrIgnore.php | Adds InsertOrIgnore feature interface. |
| src/Query/Builder.php | Removes default-throw Builder::upsert() from base builder. |
Comments suppressed due to low confidence (1)
src/Query/Schema.php:273
Schema::compileColumnDefinition()no longer guards against$column->ttl, but it also never emits TTL for non-ClickHouse dialects. SinceColumn::ttl()still exists on the base Column, callers can now set TTL on any dialect and it will be silently ignored in the generated DDL. If the intent is compile-time feature gating, consider movingttl()onto a ClickHouse-specific Column subclass/forwarder (or otherwise ensuring unsupported dialects can’t set it).
protected function compileColumnDefinition(Column $column): string
{
$parts = [
$this->quote($column->name),
$this->compileColumnType($column),
];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Greptile SummaryThis PR refactors Schema and Builder dialects so that unsupported operations are compile-time type errors (missing interface) rather than runtime
Confidence Score: 5/5The refactoring is thorough and internally consistent — dialect-specific DDL is correctly isolated, and all previously flagged correctness issues from the prior review round are resolved. Every dialect-specific override is in place: PostgreSQL's dropTrigger adds ON table, dropForeignKey uses DROP CONSTRAINT, compileCreatePartitioning omits the MySQL-only PARTITIONS N, and dropProcedure appends the required empty parentheses. Table\SQLite correctly uses only InlineForeignKey (no ALTER TABLE FK mutations exposed). MongoDB's createView parses the builder result as JSON rather than generating SQL. The Upsert alias pattern in Builder\PostgreSQL correctly routes trait body through appendReturning without double-wrapping. No new incorrect DDL paths were introduced. No files require special attention. Important Files Changed
Reviews (9): Last reviewed commit: "fix(schema): explicit () on PostgreSQL D..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
ClickHouse only uses partitionExpression — the PartitionType enum is meaningless for it. Gate the partition emission on the expression itself instead of flipping a Range sentinel just to satisfy a non-null check.
The shared Schema\Trait\Partitioning emits `PARTITIONS N` after the PARTITION BY clause when a count is set — but that's MySQL-only syntax. PostgreSQL rejects it: hash partitions are declared with separate `CREATE TABLE … PARTITION OF parent FOR VALUES WITH (modulus N, remainder R)` statements. Override compileCreatePartitioning in Schema\PostgreSQL to omit the count clause so $table->partitionByHash($expr, 4) silently drops the 4 instead of generating invalid DDL. Includes a regression test asserting the count is not emitted.
The .claude/ directory holds Claude Code's runtime state (worktrees, scheduled task lockfiles) — none of it belongs in the repo. Broaden the existing .claude/worktrees/ ignore to .claude/ and untrack the scheduled_tasks.lock that slipped in.
The shared Schema\Trait\ForeignKeys emits ALTER TABLE ADD CONSTRAINT and ALTER TABLE DROP FOREIGN KEY — neither is valid SQLite. Schema\SQLite should not implement Feature\ForeignKeys; users that need foreign keys on SQLite declare them inline at CREATE TABLE time via the Table-level \$table->foreignKey() helper, which Table\SQLite still supports. Removes the schema-level addForeignKey/dropForeignKey from SQLite and the corresponding tests that asserted invalid SQLite DDL.
…override boilerplate Make Table generic over <TColumn, TForeignKey>, Column over <TTable>, and ForeignKey over <TColumn, TTable> with call-site covariance on the Table bound. Each dialect binds the templates via @extends, so PHPStan now narrows the return of $table->id() / $fk->id() / etc. to the dialect's Column subclass without per-dialect parent::id() pass-through overrides. Net 688 lines of pure-narrowing boilerplate deleted across MySQL, PostgreSQL, SQLite, MongoDB, ClickHouse Table/Column/ForeignKey classes. The @Property \$table hacks on Column\X and ForeignKey\X are also gone. Trait\ForeignKeys is now templated over TForeignKey so foreignKey()/addForeignKey() return the dialect FK type when used via @use Trait\ForeignKeys<ForeignKey\X>.
…e DDL The shared Trait\Triggers emitted MySQL-style DROP TRIGGER \"name\", which PostgreSQL rejects (it requires DROP TRIGGER name ON table_name). Schema\PostgreSQL inherited the trait without overriding, so the produced SQL was invalid against any real PostgreSQL server. Widen the Feature\Triggers / Trait\Triggers signature to accept an optional \$table; MySQL and SQLite ignore it, PostgreSQL overrides to require it and emit the correct DDL plus drop the backing PL/pgSQL function in the same statement. Updated the existing PG dropTrigger test to assert the corrected DDL and added a regression test for the throw when \$table is omitted.
…w payload Table\PostgreSQL/MongoDB/ClickHouse vector() declared base Column return; narrow to Column\X to match the docblock and keep dialect-specific chaining type-safe without a /** @var */ cast. MongoDB dropView() emitted {command: 'drop', view: <name>}; the executor uses 'collection' as the target key for all drop commands (compileDrop already does this). Switch to 'collection' for consistency and add a regression test.
…ey()
SQLite's ALTER TABLE doesn't support FK add/drop. Table\SQLite previously
mixed in Table\Trait\ForeignKeys (which bundled foreignKey + addForeignKey +
dropForeignKey) and Forwarder\SQLite re-exposed all three on Column\SQLite /
ForeignKey\SQLite, so chains like \$table->dropForeignKey('fk')->alter()
produced ALTER TABLE … DROP FOREIGN KEY DDL that SQLite rejects at runtime.
Extract foreignKey() into Table\Trait\InlineForeignKey (CREATE-time inline FK,
valid on every dialect including SQLite). Table\Trait\ForeignKeys now composes
InlineForeignKey and adds the ALTER-only methods, used only by MySQL and
PostgreSQL. Table\SQLite uses InlineForeignKey alone; Forwarder\SQLite drops
the ALTER forwarders. Added testSQLiteHasNoAlterForeignKeyMethods regression
asserting addForeignKey/dropForeignKey are absent across Table/Column/FK\SQLite.
…tibility dropFunction() and the trigger-cleanup chain in dropTrigger() emitted DROP FUNCTION "name" without the trailing argument list. PostgreSQL 12+ accepts the bare name when the function is unique, but PostgreSQL <12 requires DROP FUNCTION name() with the explicit empty argument list. Add () in both places and update the assertions.
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.
Summary
Removes runtime `UnsupportedException` throws from the public surface
of Schema and Builder dialects. Methods that don't apply to a given
dialect no longer exist on its type — calling them is a compile-time
error, not a deferred runtime failure.
Schema (commit 1):
Builder (commit 2):
Test plan
Coverage notes
Local coverage from `composer test:coverage`:
New code (Schema/Trait/, Schema/Table/, Schema/Column/, Schema/ForeignKey/, Schema/Forwarder/, Schema/Feature/, Builder/Feature/{InsertOrIgnore,UpsertSelect}, Builder/Trait/UpsertSelect) is mostly at 100% line coverage; gaps are in trivial one-line forwarders that paratest's coverage attributes to the using class rather than the trait.
🤖 Generated with Claude Code