Skip to content

refactor: express dialect capability in types instead of UnsupportedException - #16

Open
abnegate wants to merge 7 commits into
mainfrom
refactor/drop-unsupported-exception
Open

refactor: express dialect capability in types instead of UnsupportedException#16
abnegate wants to merge 7 commits into
mainfrom
refactor/drop-unsupported-exception

Conversation

@abnegate

Copy link
Copy Markdown
Member

Stacked on #15 — review that first, or read only the commits after c856577.

The problem

A method that exists only to throw is a broken promise. It satisfies the interface, passes instanceof, autocompletes, and then fails at runtime. MySQL::withTotals() was reachable, typed, documented by the GroupByModifiers interface — and unconditionally threw.

Worse, the loud cases were the minority. Auditing all 61 UnsupportedException sites turned up a larger set of silent failures with the same root cause: methods a dialect could not honour, which accepted input and discarded it.

On the MongoDB builder, ten of the twelve raw-SQL methods silently dropped their arguments:

(new MongoDB())->from('t')->selectRaw('a + 1 AS b')->build()->query;
// {"collection":"t","operation":"aggregate","pipeline":[]}   <- expression gone

Same for selectCase(), conflictSetRaw(), orderByRaw(), groupByRaw(), havingRaw(), selectCast(), insertColumnExpression(). On the schema side, MongoDB accepted check(), generatedAs() and stored() and dropped them, and ttl() was accepted by all five dialects while only ClickHouse read it.

The fix

Capability moves into the type. If a dialect cannot do something, the method is not on its class.

Split Was Now
GroupByModifiers one interface, 3 unrelated capabilities Rollup (MySQL, MariaDB, PostgreSQL, ClickHouse), Cube (PostgreSQL, ClickHouse), Totals (ClickHouse)
FullTextSearch required filterNotSearch() negation split to NegatedFullTextSearch; MongoDB keeps search, drops the negation $text cannot express
raw SQL 12 methods on every builder RawSql, implemented by Builder\SQL + ClickHouse only
Joins required crossJoin()/naturalJoin() CrossJoins split out; $lookup always joins on a field pair
Column modifiers all on the base class check/generatedAs/stored on MySQL, MariaDB, PostgreSQL, SQLite; virtual excludes PostgreSQL (STORED only); ttl ClickHouse; userType PostgreSQL
Table serial(), dropColumn(), renameColumn() everywhere serial factories off ClickHouse; column alterations off MongoDB

61 → 37 throw sites. 18 became unreachable and were deleted, 6 were the wrong exception type.

What UnsupportedException still means

It cannot be removed entirely, and shouldn't be. 37 sites remain, every one driven by an unsupported value arriving through a correctly typed API:

  • filter(array $queries) accepts any Query by contract — Query::regex(...) on SQLite has to be rejected somewhere.
  • addColumn(string $name, ColumnType $type) accepts any ColumnTypeColumnType::Serial on ClickHouse likewise.
  • Runtime combinations of individually valid calls: Nullable(Array(...)), lowCardinality() inside Array, sampleBy() on an engine without ORDER BY.
  • Caller-supplied strings: window function names, join operators.

No interface can exclude these, so the exception is now the answer to "you passed a value I can't compile" rather than "this method never worked". Where both paths exist, the message names the value path — e.g. ClickHouse's SERIAL throw now says the serial() factory is gone but addColumn() still reaches it.

I verified all 37 individually; none is reachable by calling a method that exists only to throw.

Notes for review

  • Breaking, but only for callers of methods whose sole behaviour was throwing. Pre-1.0 (0.3.3).
  • No output change. Every supported combination emits byte-identical SQL/DDL.
  • Builder::applyAstOrderBy() and applyAstJoins() went through orderByRaw()/crossJoin(); they now append the Query directly, so the base class no longer depends on a capability its subclasses may lack.
  • Absence is asserted with class_implements()/get_class_methods() rather than assertNotInstanceOf, because PHPStan at level max proves the latter statically true and fails the build. That it can prove it is the point.
  • Trait\Serial is generic over TColumn, declared at the use site as @use Trait\Serial<Column\MySQL>, matching the existing Trait\ForeignKeys convention.
  • The README commit also fixes a pre-existing wrong example: the withCube() snippet used the MySQL builder, which never supported it.

Test plan

  • composer test — 5308 tests, 12324 assertions, all pass (was 5299)
  • composer check — PHPStan level max, no errors
  • composer lint — pass
  • New tests assert capability absence per dialect, and that the surviving value-path throws still fire
  • All 18 capability claims in the README verified by reflecting over the built classes

Integration tests need Docker and were not run locally; CI covers them.

🤖 Generated with Claude Code

abnegate and others added 6 commits August 13, 2026 22:46
…hrows

A builder method that exists only to throw is a broken promise: it passes
`instanceof`, satisfies the interface, autocompletes, and then fails at
runtime. Replace the throwing stubs with capability interfaces narrow
enough that each dialect implements only what it can actually do.

Feature\GroupByModifiers bundled three unrelated capabilities, so every
dialect that supported any of them advertised all three. Split into
Rollup, Cube and Totals, matching what the dialects really support:

  withRollup  MySQL, MariaDB, PostgreSQL, ClickHouse
  withCube    PostgreSQL, ClickHouse
  withTotals  ClickHouse

That removes four footguns -- MySQL::withCube(), MySQL::withTotals(),
MariaDB::withCube(), MariaDB::withTotals() and PostgreSQL::withTotals()
no longer exist rather than throwing.

Feature\FullTextSearch required filterNotSearch(), which MongoDB cannot
implement: $text has no negated form. Negation moves to
Feature\NegatedFullTextSearch, so MongoDB keeps full-text search without
advertising a negation it has to reject.

The larger problem was raw SQL. MongoDB emits operation documents, so
there is nowhere in its output a SQL fragment could go, yet it inherited
twelve methods for splicing SQL in. Two threw; the other ten -- among
them selectRaw, orderByRaw, groupByRaw, havingRaw, selectCast,
selectCase and insertColumnExpression -- accepted the input and silently
discarded it, which is the worse failure. All twelve now live in
Feature\RawSql, implemented by Builder\SQL and ClickHouse only.

Builder::applyAstOrderBy() wrote through orderByRaw(); it now appends to
rawOrders directly, so the base class no longer depends on a capability
its subclasses may not have.

Absence is asserted against the runtime published surface
(class_implements/get_class_methods) rather than assertNotInstanceOf,
because PHPStan at level max proves the latter statically true and fails
the build -- the type system now knows these capabilities are gone,
which is the point.

Breaking change for callers of the removed methods, though the only
possible use was catching the exception. Pre-1.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… them

The Column class carried every modifier for every dialect, so the schema
builder let you call things the target could not do. Some throws were
loud, but the worse cases were silent: MongoDB accepted check(),
generatedAs() and stored() and dropped them, and ttl() was accepted by
all five dialects while only ClickHouse read it.

Modifiers now sit with the dialects that honour them:

  check         MySQL, PostgreSQL, SQLite  (already overridden there)
  generatedAs   MySQL, PostgreSQL, SQLite  -> Column\Trait\Generated
  stored        MySQL, PostgreSQL, SQLite  -> Column\Trait\Generated
  virtual       MySQL, SQLite              -> Column\Trait\VirtualGenerated
  ttl           ClickHouse
  userType      PostgreSQL

virtual() is a separate trait from generatedAs()/stored() because
PostgreSQL supports STORED only; that constraint is now in the type
rather than in a throw from compileGeneratedClause().

dropColumn()/renameColumn() move off Table into
Table\Trait\ColumnAlterations, used by the four SQL-family dialects.
MongoDB reshapes documents with $unset/$rename, so its Table no longer
offers a schema-level equivalent it would only reject. The matching
forwarders move from Column and ForeignKey into the per-dialect
Forwarder traits, following the existing pattern.

Six throws are deleted as unreachable: user-defined types on MySQL,
SQLite, ClickHouse and MongoDB; generated columns and CHECK on
ClickHouse; VIRTUAL generated columns on PostgreSQL; and MongoDB's
composite-primary-key guard, whose Table never used
Trait\CompositePrimary and so could never set the field.

No behaviour change for supported combinations -- every dialect emits
byte-identical DDL. The generics already in place (@template TColumn,
@extends Table<Column\MySQL, ...>) mean chained calls still resolve to
the dialect column, so callers see the narrowed surface statically.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lures

Six throws described neither a missing capability nor an unsupported
dialect feature, so UnsupportedException was the wrong type:

- Table::compile* when the Table has no Schema. Constructing a detached
  Table and compiling it is a usage error, not an unsupported operation.
- Five json_decode() === null guards in the MongoDB builder and schema
  (view creation, unions, WHERE IN and EXISTS subqueries, $facet). These
  fire when a nested builder yields something that is not a JSON
  operation document -- a malformed input, not a capability limit.

All six now raise ValidationException, matching how the rest of the
codebase reports bad input.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Feature\Joins required crossJoin() and naturalJoin(), which MongoDB
cannot express: $lookup always joins on a field pair. Both move to
Feature\CrossJoins, implemented by Builder\SQL and ClickHouse, so the
MongoDB builder no longer offers them.

The compileJoinStage() guard stays, because Query::crossJoin() can still
arrive through queries() at runtime -- that path is a value, not a
method, so the type system cannot close it. Its message now says so, and
a test covers it.

Three ClickHouse foreign-key guards and one CHECK guard were already
dead: Table\ClickHouse uses neither Trait\ForeignKeys nor
Trait\InlineForeignKey and its Forwarder has no foreignKey(), so
foreignKeys/dropForeignKeys could never be populated, and after the
previous commit neither Table\ClickHouse nor Column\ClickHouse exposes
check(). Deleted.

Builder::applyAstJoins() went through crossJoin()/naturalJoin(); like
applyAstOrderBy() before it, it now appends the Query directly so the
base class does not depend on a capability its subclasses may lack.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Table exposed serial(), bigSerial() and smallSerial() on every dialect,
but ClickHouse has no server-generated sequence type, so all three threw.
They move to Table\Trait\Serial, used by MySQL, PostgreSQL, SQLite and
MongoDB.

The throw in ClickHouse::compileColumnType() stays, because
addColumn('id', ColumnType::Serial) reaches it with a runtime enum value
that no interface can exclude. Its message now names that path so the
reader knows the factory is gone but the enum is still possible, and a
test covers both halves.

The matching Column and ForeignKey forwarders move into the per-dialect
Forwarder traits. Trait\Serial is generic over TColumn, declared at the
use site as @use Trait\Serial<Column\MySQL>, matching Trait\ForeignKeys.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the new shape and states what UnsupportedException now means: it
is reserved for an unsupported value arriving through a correctly typed
API -- Query::regex() passed to filter() on SQLite, ColumnType::Serial
passed to addColumn() on ClickHouse -- because filter(array $queries) and
addColumn(string, ColumnType) accept any query or column type by
contract. Everything a dialect cannot do is now absent from its class, so
the check is instanceof, not catch.

Adds Feature Matrix rows for Raw SQL, Cross/Natural Joins and Negated
Full-Text Search, and splits Group By Modifiers into Rollup, Cube and
Totals.

Fixes an example that was wrong before this branch: the withCube() snippet
used the MySQL builder, which never supported WITH CUBE and threw. It now
uses PostgreSQL, and the surrounding table gives the real per-modifier
support.

Column modifiers are split into those every dialect honours and those
scoped to particular dialects, since check(), generatedAs(), stored(),
virtual(), ttl() and userType() no longer exist everywhere.

Every capability claim in the new content was verified by reflecting over
the built classes rather than read off the source.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

📊 Coverage

Metric Covered Ratio
Lines 91.91% 7454 / 8110
Methods 84.25% 1107 / 1314
Classes 65.26% 139 / 213

Full per-file breakdown in the job summary.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces runtime-only unsupported dialect operations with capability-specific interfaces and traits while preserving valid dialect behavior.

  • Splits raw SQL, cross joins, grouping modifiers, and negated full-text search into independently typed capabilities.
  • Moves schema column and table operations onto only the dialect-specific classes that support them.
  • Updates AST handling, documentation, and capability-absence tests for the revised API.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/Query/Builder.php AST cross/natural joins and raw ordering now append their internal query representations directly without relying on optional public capabilities.
src/Query/Builder/Trait/RawSql.php Consolidates raw SQL methods into a capability trait used by SQL builders and ClickHouse.
src/Query/Builder/MongoDB.php Removes unsupported public methods and retains explicit rejection for unsupported values entering through typed query APIs.
src/Query/Schema/ForeignKey.php Removes generic forwarding methods while concrete supported dialects retain them through their dialect-specific forwarder traits.
src/Query/Schema/Table/Trait/Serial.php Encapsulates serial column factories for only the table dialects that support them.
README.md Documents the new capability interfaces and dialect-specific method availability.

Fix All in Greploop

Reviews (2): Last reviewed commit: "test(schema): pin the fluent chain throu..." | Re-trigger Greptile

Comment thread src/Query/Schema/ForeignKey.php
…tories

The serial and column-alteration forwarders live on the per-dialect
Forwarder traits, which Column\X and ForeignKey\X both use. Nothing
covered the ForeignKey half, so moving them off the base class looked
like it dropped them from foreign keys.

It did not -- the chain works on MySQL, PostgreSQL and SQLite -- but the
sharing is easy to miss, so pin both the chain and the forwarded method
list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant