From af8055688e0f728f8339576772ef99afef319c7e Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 14 Aug 2026 00:04:23 +1200 Subject: [PATCH 1/2] fix: stop silently dropping input that a dialect cannot honour An audit for the pattern behind the previous commits turned up its quieter half. Rather than gating with a throw, these paths accepted input and discarded it, which is harder to notice and in one case unsafe. The MongoDB builder dropped Hook\Filter. Hook\Filter::filter() returns a Condition -- a raw SQL expression plus bindings -- and an operation document has nowhere to put one, so addHook() stored the hook and nothing ever read it. A Hook\Filter\Tenant that correctly emits `WHERE tenant_id IN (?)` on MySQL produced an unscoped MongoDB query returning every tenant's documents. MongoDB::addHook() now rejects Hook\Filter and Hook\Join\Filter; Hook\Attribute and Hook\Write are dialect-neutral and still apply. No test covered this, which is why it survived; there are four now, in SecurityRegressionTest. Column::collation() was dead on all five dialects: the setter stored a value, no compiler ever read it, and the only test asserted the property round-tripped rather than that any DDL changed. The one COLLATE emission in the codebase is for index collations, a separate path. It is now emitted for MySQL, MariaDB, PostgreSQL and SQLite -- PostgreSQL quotes the name via a quoteCollation() hook -- and removed from ClickHouse (which collates in ORDER BY) and MongoDB (per collection). Three more modifiers moved to the dialects that emit them, verified by diffing DDL with and without each one rather than by reading the source: unique() MySQL, MariaDB, PostgreSQL, SQLite after() MySQL, MariaDB, SQLite (PostgreSQL cannot order columns) comment() all but PostgreSQL, which needs a separate COMMENT ON -- commentOnColumn() already emits it, and a test asserting the inline form produced nothing becomes a type-level assertion Schema::compileCreate() branched on `$this instanceof Feature\Partitioning` to decide whether to append a partitioning clause -- the same gate, one level up. It now calls a protected compileCreateSuffix() hook that returns '' by default and is overridden by Trait\Partitioning, so the base class no longer enumerates the features that might exist. MySQL and PostgreSQL emit identical partitioning DDL. Left alone and documented as known gaps, because the library's own column factories set them internally and unpicking that is a separate change: unsigned() renders nothing on PostgreSQL and SQLite, and srid(), autoIncrement() and vector()'s $dimensions are accepted on dialects that cannot express them. Co-Authored-By: Claude Opus 5 --- README.md | 26 +++++--- src/Query/Builder/MongoDB.php | 27 +++++++++ src/Query/Schema.php | 33 +++++++++-- src/Query/Schema/Column.php | 28 --------- src/Query/Schema/Column/ClickHouse.php | 1 + src/Query/Schema/Column/MongoDB.php | 1 + src/Query/Schema/Column/MySQL.php | 4 ++ src/Query/Schema/Column/PostgreSQL.php | 2 + src/Query/Schema/Column/SQLite.php | 4 ++ src/Query/Schema/Column/Trait/Collation.php | 18 ++++++ src/Query/Schema/Column/Trait/Comment.php | 18 ++++++ src/Query/Schema/Column/Trait/Positioning.php | 17 ++++++ src/Query/Schema/Column/Trait/Unique.php | 17 ++++++ src/Query/Schema/PostgreSQL.php | 10 ++++ src/Query/Schema/Trait/Partitioning.php | 6 ++ .../Regression/SecurityRegressionTest.php | 59 +++++++++++++++++++ tests/Query/Schema/ClickHouseTest.php | 9 +++ tests/Query/Schema/FluentBuilderTest.php | 23 +++++++- tests/Query/Schema/MySQLTest.php | 13 ++-- tests/Query/Schema/PostgreSQLTest.php | 23 ++++++-- tests/Query/Schema/SQLiteTest.php | 9 +++ 21 files changed, 294 insertions(+), 54 deletions(-) create mode 100644 src/Query/Schema/Column/Trait/Collation.php create mode 100644 src/Query/Schema/Column/Trait/Comment.php create mode 100644 src/Query/Schema/Column/Trait/Positioning.php create mode 100644 src/Query/Schema/Column/Trait/Unique.php diff --git a/README.md b/README.md index 188f091..7d2a550 100644 --- a/README.md +++ b/README.md @@ -1008,6 +1008,8 @@ $result = (new Builder()) // WHERE `status` IN (?) AND `tenant_id` IN (?) ``` +> **SQL builders only.** `Hook\Filter` and `Hook\Join\Filter` return a `Condition` — a raw SQL expression plus bindings — so there is nowhere to put one in a MongoDB operation document. `MongoDB::addHook()` rejects them with `UnsupportedException` rather than accepting and dropping them; a silently ignored `Hook\Filter\Tenant` would leave every query unscoped. `Hook\Attribute` and `Hook\Write` are dialect-neutral and work everywhere. On MongoDB, express the constraint as a `Query` passed to `filter()`. + **Custom filter hooks** implement `Hook\Filter`: ```php @@ -1983,20 +1985,26 @@ $result = $schema->table('users') Available column types: `id`, `uuid`, `string`, `text`, `mediumText`, `longText`, `tinyInteger`, `smallInteger`, `integer`, `bigInteger`, `serial`, `bigSerial`, `smallSerial`, `float`, `decimal`, `boolean`, `datetime`, `timestamp`, `json`, `binary`, `enum`, `point`, `linestring`, `polygon`, `vector` (PostgreSQL, ClickHouse, MongoDB), `timestamps`. -Column modifiers available on every dialect: `nullable()`, `default($value)`, `defaultRaw($expression)`, `unsigned()`, `unique()`, `primary()`, `autoIncrement()`, `after($column)`, `comment($text)`, `collation($collation)`, `srid($srid)` (spatial columns), `dimensions($dimensions)` (vector columns). +Column modifiers available on every dialect: `nullable()`, `default($value)`, `defaultRaw($expression)`, `primary()`, `unsigned()`, `autoIncrement()`, `srid($srid)` (spatial columns), `dimensions($dimensions)` (vector columns). -The rest are only on the dialects that honour them, so an unsupported combination is a type error rather than a runtime one: +The rest are only on the dialects that emit them, so an unsupported combination is a type error rather than something silently dropped: -| Modifier | Dialects | -|---|---| -| `check($expression)` | MySQL, MariaDB, PostgreSQL, SQLite | -| `generatedAs($expression)`, `stored()` | MySQL, MariaDB, PostgreSQL, SQLite | -| `virtual()` | MySQL, MariaDB, SQLite (PostgreSQL supports `STORED` only) | -| `ttl($expression)` | ClickHouse | -| `userType($name)` | PostgreSQL | +| Modifier | Dialects | Why not the others | +|---|---|---| +| `check($expression)` | MySQL, MariaDB, PostgreSQL, SQLite | ClickHouse and MongoDB have no CHECK | +| `generatedAs($expression)`, `stored()` | MySQL, MariaDB, PostgreSQL, SQLite | — | +| `virtual()` | MySQL, MariaDB, SQLite | PostgreSQL supports `STORED` only | +| `collation($collation)` | MySQL, MariaDB, PostgreSQL, SQLite | ClickHouse collates in `ORDER BY`; MongoDB per collection | +| `unique()` | MySQL, MariaDB, PostgreSQL, SQLite | ClickHouse enforces no uniqueness; MongoDB uses a unique index | +| `after($column)` | MySQL, MariaDB, SQLite | PostgreSQL cannot order columns; MongoDB documents have none | +| `comment($text)` | MySQL, MariaDB, SQLite, ClickHouse, MongoDB | PostgreSQL needs a separate statement — use `commentOnColumn()` | +| `ttl($expression)` | ClickHouse | — | +| `userType($name)` | PostgreSQL | — | Likewise `serial()` / `bigSerial()` / `smallSerial()` are absent from the ClickHouse table, and `dropColumn()` / `renameColumn()` are absent from the MongoDB table. +> **Known gaps.** `unsigned()` is accepted everywhere but renders nothing on PostgreSQL and SQLite, which have no unsigned integer types — you get a signed column. `srid()`, `autoIncrement()` and the `$dimensions` argument to `vector()` are likewise accepted on dialects that cannot express them, because the library's own column factories set them internally. Treat those four as advisory rather than guaranteed. + **Raw default expressions** — use `defaultRaw($expression)` for dialect-specific server-generated defaults that `default()` would otherwise quote as a string literal (`now()`, `CURRENT_TIMESTAMP`, `gen_random_uuid()`, `generateUUIDv4()`, `UUID()`, …). The expression is emitted verbatim and must come from a trusted source; it must not be empty or contain a semicolon. Takes precedence over `default()` when both are set. **SERIAL types** — auto-incrementing integers. PostgreSQL emits native `SERIAL` / `BIGSERIAL` / `SMALLSERIAL`; MySQL/MariaDB compile to `INT AUTO_INCREMENT` / `BIGINT AUTO_INCREMENT` / `SMALLINT AUTO_INCREMENT`; SQLite maps to `INTEGER`; MongoDB maps them to the BSON `int` type. ClickHouse has no sequence type, so `Table\ClickHouse` does not expose these factories at all: diff --git a/src/Query/Builder/MongoDB.php b/src/Query/Builder/MongoDB.php index 27845d5..5a49f81 100644 --- a/src/Query/Builder/MongoDB.php +++ b/src/Query/Builder/MongoDB.php @@ -18,6 +18,9 @@ use Utopia\Query\Builder\MongoDB\UpdateOperator; use Utopia\Query\Exception\UnsupportedException; use Utopia\Query\Exception\ValidationException; +use Utopia\Query\Hook; +use Utopia\Query\Hook\Filter; +use Utopia\Query\Hook\Join\Filter as JoinFilter; use Utopia\Query\Method; use Utopia\Query\Query; @@ -221,6 +224,30 @@ public function reset(): static return $this; } + /** + * {@inheritDoc} + * + * Hook\Filter and Hook\Join\Filter return a {@see Condition}: a raw SQL + * expression plus bindings. A MongoDB operation document has nowhere to put + * one, so they are rejected rather than accepted and dropped -- silently + * ignoring a Hook\Filter\Tenant would leave every query unscoped. + * + * Hook\Attribute and Hook\Write are dialect-neutral and still apply. + */ + #[\Override] + public function addHook(Hook $hook): static + { + if ($hook instanceof Filter || $hook instanceof JoinFilter) { + throw new UnsupportedException( + 'Filter hooks are not supported on the MongoDB builder: ' + . Filter::class . ' returns a SQL expression, which has no place in an ' + . 'operation document. Express the constraint as a Query passed to filter() instead.' + ); + } + + return parent::addHook($hook); + } + #[\Override] public function build(): Statement { diff --git a/src/Query/Schema.php b/src/Query/Schema.php index bf590f9..6a347b3 100644 --- a/src/Query/Schema.php +++ b/src/Query/Schema.php @@ -122,11 +122,9 @@ public function compileCreate(Table $table, bool $ifNotExists = false): Statemen $sql = 'CREATE TABLE ' . ($ifNotExists ? 'IF NOT EXISTS ' : '') . $this->quote($table->name) . ' (' . \implode(', ', $columnDefs) . ')'; - if ($this instanceof Schema\Feature\Partitioning) { - $partitioning = $this->compileCreatePartitioning($table); - if ($partitioning !== '') { - $sql .= ' ' . $partitioning; - } + $suffix = $this->compileCreateSuffix($table); + if ($suffix !== '') { + $sql .= ' ' . $suffix; } return new Statement($sql, [], executor: $this->executor); @@ -273,6 +271,10 @@ protected function compileColumnDefinition(Column $column): string $this->compileColumnType($column), ]; + if ($column->collation !== null) { + $parts[] = 'COLLATE ' . $this->quoteCollation($column->collation); + } + if ($column->isUnsigned) { $unsigned = $this->compileUnsigned(); if ($unsigned !== '') { @@ -356,6 +358,27 @@ protected function compileDefaultValue(mixed $value): string return "'" . \str_replace(['\\', "'"], ['\\\\', "''"], (string) $value) . "'"; } + /** + * Dialect-specific clauses appended after the closing paren of CREATE TABLE. + * + * Overridden by capability traits (e.g. Trait\Partitioning) rather than + * gated on the schema's own type, so the base class does not need to know + * which features exist. + */ + protected function compileCreateSuffix(Table $table): string + { + return ''; + } + + /** + * Render a column COLLATE name. MySQL and SQLite take a bare identifier; + * PostgreSQL requires a quoted one. + */ + protected function quoteCollation(string $collation): string + { + return $collation; + } + protected function compileUnsigned(): string { return 'UNSIGNED'; diff --git a/src/Query/Schema/Column.php b/src/Query/Schema/Column.php index b7d144a..20c356f 100644 --- a/src/Query/Schema/Column.php +++ b/src/Query/Schema/Column.php @@ -123,13 +123,6 @@ public function unsigned(): static return $this; } - public function unique(): static - { - $this->isUnique = true; - - return $this; - } - /** * Mark this column as a primary key. Dialect Column subclasses that * support composite primary keys also accept a list of column names to @@ -142,13 +135,6 @@ public function primary(): static|Table return $this; } - public function after(string $column): static - { - $this->after = $column; - - return $this; - } - public function autoIncrement(): static { $this->isAutoIncrement = true; @@ -156,20 +142,6 @@ public function autoIncrement(): static return $this; } - public function comment(string $comment): static - { - $this->comment = $comment; - - return $this; - } - - public function collation(string $collation): static - { - $this->collation = $collation; - - return $this; - } - /** * Set the allowed values on this enum column (when called with one array * argument), or add a new enum column to the parent table (when called diff --git a/src/Query/Schema/Column/ClickHouse.php b/src/Query/Schema/Column/ClickHouse.php index 78c7c48..c940974 100644 --- a/src/Query/Schema/Column/ClickHouse.php +++ b/src/Query/Schema/Column/ClickHouse.php @@ -13,6 +13,7 @@ */ class ClickHouse extends Column { + use Trait\Comment; use Forwarder\ClickHouse; public protected(set) bool $isLowCardinality = false; diff --git a/src/Query/Schema/Column/MongoDB.php b/src/Query/Schema/Column/MongoDB.php index 7dc230c..e279ace 100644 --- a/src/Query/Schema/Column/MongoDB.php +++ b/src/Query/Schema/Column/MongoDB.php @@ -11,5 +11,6 @@ */ class MongoDB extends Column { + use Trait\Comment; use Forwarder\MongoDB; } diff --git a/src/Query/Schema/Column/MySQL.php b/src/Query/Schema/Column/MySQL.php index dc57dbc..986c862 100644 --- a/src/Query/Schema/Column/MySQL.php +++ b/src/Query/Schema/Column/MySQL.php @@ -11,6 +11,10 @@ */ class MySQL extends Column { + use Trait\Collation; + use Trait\Positioning; + use Trait\Comment; + use Trait\Unique; use Trait\Generated; use Trait\VirtualGenerated; use Forwarder\MySQL; diff --git a/src/Query/Schema/Column/PostgreSQL.php b/src/Query/Schema/Column/PostgreSQL.php index f3ae8b7..d93c9aa 100644 --- a/src/Query/Schema/Column/PostgreSQL.php +++ b/src/Query/Schema/Column/PostgreSQL.php @@ -12,6 +12,8 @@ */ class PostgreSQL extends Column { + use Trait\Collation; + use Trait\Unique; use Trait\Generated; use Forwarder\PostgreSQL; diff --git a/src/Query/Schema/Column/SQLite.php b/src/Query/Schema/Column/SQLite.php index 2045a10..58981bd 100644 --- a/src/Query/Schema/Column/SQLite.php +++ b/src/Query/Schema/Column/SQLite.php @@ -11,6 +11,10 @@ */ class SQLite extends Column { + use Trait\Collation; + use Trait\Positioning; + use Trait\Comment; + use Trait\Unique; use Trait\Generated; use Trait\VirtualGenerated; use Forwarder\SQLite; diff --git a/src/Query/Schema/Column/Trait/Collation.php b/src/Query/Schema/Column/Trait/Collation.php new file mode 100644 index 0000000..4016336 --- /dev/null +++ b/src/Query/Schema/Column/Trait/Collation.php @@ -0,0 +1,18 @@ +collation = $collation; + + return $this; + } +} diff --git a/src/Query/Schema/Column/Trait/Comment.php b/src/Query/Schema/Column/Trait/Comment.php new file mode 100644 index 0000000..be7bad5 --- /dev/null +++ b/src/Query/Schema/Column/Trait/Comment.php @@ -0,0 +1,18 @@ +comment = $comment; + + return $this; + } +} diff --git a/src/Query/Schema/Column/Trait/Positioning.php b/src/Query/Schema/Column/Trait/Positioning.php new file mode 100644 index 0000000..a32eb3f --- /dev/null +++ b/src/Query/Schema/Column/Trait/Positioning.php @@ -0,0 +1,17 @@ +after = $column; + + return $this; + } +} diff --git a/src/Query/Schema/Column/Trait/Unique.php b/src/Query/Schema/Column/Trait/Unique.php new file mode 100644 index 0000000..32b3acc --- /dev/null +++ b/src/Query/Schema/Column/Trait/Unique.php @@ -0,0 +1,17 @@ +isUnique = true; + + return $this; + } +} diff --git a/src/Query/Schema/PostgreSQL.php b/src/Query/Schema/PostgreSQL.php index 06831d2..88820e5 100644 --- a/src/Query/Schema/PostgreSQL.php +++ b/src/Query/Schema/PostgreSQL.php @@ -91,6 +91,12 @@ protected function compileAutoIncrement(): string return 'GENERATED BY DEFAULT AS IDENTITY'; } + #[\Override] + protected function quoteCollation(string $collation): string + { + return $this->quote($collation); + } + protected function compileUnsigned(): string { return ''; @@ -103,6 +109,10 @@ protected function compileColumnDefinition(Column $column): string $this->compileColumnType($column), ]; + if ($column->collation !== null) { + $parts[] = 'COLLATE ' . $this->quoteCollation($column->collation); + } + if ($column->isUnsigned) { $unsigned = $this->compileUnsigned(); if ($unsigned !== '') { diff --git a/src/Query/Schema/Trait/Partitioning.php b/src/Query/Schema/Trait/Partitioning.php index 9d17bcf..916de70 100644 --- a/src/Query/Schema/Trait/Partitioning.php +++ b/src/Query/Schema/Trait/Partitioning.php @@ -6,6 +6,12 @@ trait Partitioning { + #[\Override] + protected function compileCreateSuffix(Table $table): string + { + return $this->compileCreatePartitioning($table); + } + public function compileCreatePartitioning(Table $table): string { if ($table->partitionType === null) { diff --git a/tests/Query/Regression/SecurityRegressionTest.php b/tests/Query/Regression/SecurityRegressionTest.php index 0480bbb..f01c1b3 100644 --- a/tests/Query/Regression/SecurityRegressionTest.php +++ b/tests/Query/Regression/SecurityRegressionTest.php @@ -4,12 +4,16 @@ use PHPUnit\Framework\TestCase; use Utopia\Query\Builder\JoinBuilder; +use Utopia\Query\Builder\MongoDB as MongoDBBuilder; use Utopia\Query\Builder\MySQL as MySQLBuilder; use Utopia\Query\Builder\PostgreSQL as PostgreSQLBuilder; use Utopia\Query\Classifier\MongoDB as MongoDBClassifier; use Utopia\Query\Classifier\MySQL as MySQLClassifier; use Utopia\Query\Classifier\PostgreSQL as PostgreSQLClassifier; +use Utopia\Query\Exception\UnsupportedException; use Utopia\Query\Exception\ValidationException; +use Utopia\Query\Hook\Attribute\Map; +use Utopia\Query\Hook\Filter\Tenant; use Utopia\Query\Method; use Utopia\Query\Query; use Utopia\Query\Schema\Index; @@ -34,6 +38,11 @@ * - 5662d27 fix: cast/window selectors + mongo field names + parser depth + tokenizer bounds * testMongoBuilderRejectsDollarPrefixedFieldName * testMongoBuilderRejectsEmptyFieldName + * - (this branch) fix: MongoDB silently dropped Hook\Filter + * testMongoBuilderRejectsFilterHookInsteadOfDroppingIt + * testMongoBuilderRejectsJoinFilterHook + * testSqlBuildersStillScopeByTenantHook + * testMongoBuilderStillAppliesAttributeHooks * - c5a4ed3 fix: escape backslashes in DDL string literals * testMySqlCreateTypeEnumEscapesTrailingBackslash * testPostgreSqlCreateCollationRejectsInvalidOptionKey @@ -366,4 +375,54 @@ public function testJoinOnAcceptsValidIdentifiers(): void $this->assertCount(1, $join->ons); } + /** + * A Hook\Filter returns a Condition -- a raw SQL expression plus bindings -- + * which cannot be placed in a MongoDB operation document. The builder used + * to accept the hook and drop it, so a Hook\Filter\Tenant that correctly + * scoped MySQL left every MongoDB query unscoped and returned other + * tenants' documents. It must refuse the hook instead. + */ + public function testMongoBuilderRejectsFilterHookInsteadOfDroppingIt(): void + { + $this->expectException(UnsupportedException::class); + $this->expectExceptionMessage('Filter hooks are not supported on the MongoDB builder'); + + (new MongoDBBuilder())->addHook(new Tenant(['7'])); + } + + public function testMongoBuilderRejectsJoinFilterHook(): void + { + // Tenant implements both Hook\Filter and Hook\Join\Filter. + $this->expectException(UnsupportedException::class); + + (new MongoDBBuilder())->addHook(new Tenant(['7'], 'org_id')); + } + + /** The SQL builders must keep scoping, so the fix cannot regress them. */ + public function testSqlBuildersStillScopeByTenantHook(): void + { + foreach ([MySQLBuilder::class, PostgreSQLBuilder::class] as $builderClass) { + $result = (new $builderClass()) + ->addHook(new Tenant(['7'])) + ->from('docs') + ->select(['a']) + ->build(); + + $this->assertStringContainsString('tenant_id', $result->query); + $this->assertContains('7', $result->bindings); + } + } + + /** Attribute hooks are dialect-neutral and must still apply on MongoDB. */ + public function testMongoBuilderStillAppliesAttributeHooks(): void + { + $result = (new MongoDBBuilder()) + ->addHook(new Map(['a' => 'renamed'])) + ->from('docs') + ->filter([Query::equal('a', ['x'])]) + ->build(); + + $this->assertStringContainsString('renamed', $result->query); + $this->assertStringNotContainsString('"a"', $result->query); + } } diff --git a/tests/Query/Schema/ClickHouseTest.php b/tests/Query/Schema/ClickHouseTest.php index bc9de52..be6cb81 100644 --- a/tests/Query/Schema/ClickHouseTest.php +++ b/tests/Query/Schema/ClickHouseTest.php @@ -23,6 +23,15 @@ class ClickHouseTest extends TestCase { use AssertsBindingCount; + public function testColumnDoesNotExposeCollation(): void + { + $methods = \get_class_methods((new Schema())->table('t')->string('s')); + + $this->assertNotContains('collation', $methods); + $this->assertNotContains('unique', $methods); + $this->assertNotContains('after', $methods); + } + public function testTableDoesNotExposeSerialFactories(): void { $methods = \get_class_methods((new Schema())->table('t')); diff --git a/tests/Query/Schema/FluentBuilderTest.php b/tests/Query/Schema/FluentBuilderTest.php index f78b19d..d36e29a 100644 --- a/tests/Query/Schema/FluentBuilderTest.php +++ b/tests/Query/Schema/FluentBuilderTest.php @@ -17,6 +17,7 @@ use Utopia\Query\Schema\PostgreSQL; use Utopia\Query\Schema\SQLite; use Utopia\Query\Schema\Table as BaseTable; +use Utopia\Query\Schema\Table\MySQL as MySQLTable; use Utopia\Query\Schema\Table\PostgreSQL as Table; /** @@ -802,9 +803,29 @@ public function testColumnReturnsItselfForChainingFluentMethods(): void $this->assertSame($col, $col->unique()); $this->assertSame($col, $col->primary()); $this->assertSame($col, $col->autoIncrement()); + $this->assertSame($col, $col->collation('C')); + } + + public function testMySQLColumnReturnsItselfForDialectScopedFluentMethods(): void + { + $col = (new MySQLTable())->string('name'); + $this->assertSame($col, $col->comment('hi')); - $this->assertSame($col, $col->collation('utf8mb4_bin')); $this->assertSame($col, $col->after('id')); + $this->assertSame($col, $col->collation('utf8mb4_bin')); + } + + public function testColumnModifiersAreScopedToDialectsThatEmitThem(): void + { + $mysql = \get_class_methods((new MySQLTable())->string('s')); + $pg = \get_class_methods((new Table())->string('s')); + + // PostgreSQL cannot order columns, and cannot inline a comment in + // CREATE TABLE -- commentOnColumn() is the supported path there. + $this->assertContains('after', $mysql); + $this->assertContains('comment', $mysql); + $this->assertNotContains('after', $pg); + $this->assertNotContains('comment', $pg); } public function testForeignKeyReturnsItselfForChainingFluentMethods(): void diff --git a/tests/Query/Schema/MySQLTest.php b/tests/Query/Schema/MySQLTest.php index 0ea34e6..6faafba 100644 --- a/tests/Query/Schema/MySQLTest.php +++ b/tests/Query/Schema/MySQLTest.php @@ -1091,13 +1091,16 @@ public function testTableBinaryColumn(): void $this->assertSame('CREATE TABLE `t` (`data` BLOB NOT NULL)', $result->query); } - public function testColumnCollation(): void + public function testColumnCollationIsEmitted(): void { - $bp = (new Schema())->table('t'); - $col = new Column($bp, 'name', ColumnType::String, 255); - $col->collation('utf8mb4_unicode_ci'); + $result = (new Schema())->table('t') + ->string('name')->collation('utf8mb4_unicode_ci') + ->create(); - $this->assertSame('utf8mb4_unicode_ci', $col->collation); + $this->assertSame( + 'CREATE TABLE `t` (`name` VARCHAR(255) COLLATE utf8mb4_unicode_ci NOT NULL)', + $result->query + ); } public function testColumnPrecision(): void diff --git a/tests/Query/Schema/PostgreSQLTest.php b/tests/Query/Schema/PostgreSQLTest.php index 495af3b..314c3e6 100644 --- a/tests/Query/Schema/PostgreSQLTest.php +++ b/tests/Query/Schema/PostgreSQLTest.php @@ -116,15 +116,26 @@ public function testCreateTableUnsignedIgnored(): void $this->assertSame('CREATE TABLE "t" ("age" INTEGER NOT NULL)', $result->query); } - public function testCreateTableNoInlineComment(): void + public function testColumnCollationIsEmittedAndQuoted(): void { - $schema = new Schema(); - $result = $schema->table('t') - ->string('name')->comment('User display name') + $result = (new Schema())->table('t') + ->string('name')->collation('C') ->create(); - $this->assertBindingCount($result); - $this->assertStringNotContainsString('COMMENT', $result->query); + $this->assertSame('CREATE TABLE "t" ("name" VARCHAR(255) COLLATE "C" NOT NULL)', $result->query); + } + + public function testColumnDoesNotExposeInlineComment(): void + { + // PostgreSQL cannot inline a comment in CREATE TABLE; it needs a + // separate COMMENT ON statement, which commentOnColumn() emits. + $column = (new Schema())->table('t')->string('name'); + + $this->assertNotContains('comment', \get_class_methods($column)); + $this->assertStringContainsString( + 'COMMENT ON COLUMN', + (new Schema())->commentOnColumn('t', 'name', 'User display name')->query + ); } public function testAutoIncrementUsesIdentity(): void diff --git a/tests/Query/Schema/SQLiteTest.php b/tests/Query/Schema/SQLiteTest.php index 8c1b2f1..ddf4413 100644 --- a/tests/Query/Schema/SQLiteTest.php +++ b/tests/Query/Schema/SQLiteTest.php @@ -545,6 +545,15 @@ public function testSerialColumnMapsToInteger(): void $this->assertSame('CREATE TABLE `t` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL)', $result->query); } + public function testColumnCollationIsEmitted(): void + { + $result = (new Schema())->table('t') + ->string('name')->collation('NOCASE') + ->create(); + + $this->assertStringContainsString('COLLATE NOCASE', $result->query); + } + public function testColumnDoesNotExposeUserType(): void { $column = (new Schema())->table('t')->string('mood'); From 1c1e28e2bddcd9d8550611676119052098c6574d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 14 Aug 2026 00:12:09 +1200 Subject: [PATCH 2/2] fix(schema): drop after() from SQLite, whose ALTER has no AFTER clause Exposing after() on Column\SQLite carried forward a pre-existing bug: base Schema::compileAlter() emits `AFTER `, SQLite uses that base path, and SQLite's ALTER TABLE ADD COLUMN has no AFTER clause, so the statement was a syntax error at execution: ALTER TABLE `t` ADD COLUMN `b` INTEGER NOT NULL AFTER `a` -> SQLSTATE[HY000]: General error: 1 near "AFTER": syntax error after() is now on MySQL and MariaDB only, the two dialects that accept it. My audit missed this because the detector compared generated output with and without each modifier and treated "the string changed" as honoured, which says nothing about whether the result is valid DDL. Two tests close that gap by executing the emitted statement instead of matching it: one runs the SQLite ALTER against an in-memory database and checks the column list via pragma_table_info, the other runs the SQLite COLLATE CREATE. For the same reason the new collation() emission now has integration coverage on MySQL and PostgreSQL, asserting COLLATION_NAME/collation_name from information_schema rather than trusting the emitted string. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- src/Query/Schema/Column/SQLite.php | 1 - src/Query/Schema/Column/Trait/Positioning.php | 8 +++-- .../Schema/MySQLIntegrationTest.php | 15 ++++++++ .../Schema/PostgreSQLIntegrationTest.php | 15 ++++++++ tests/Query/Schema/SQLiteTest.php | 34 +++++++++++++++++++ 6 files changed, 71 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 7d2a550..2263ce1 100644 --- a/README.md +++ b/README.md @@ -1996,7 +1996,7 @@ The rest are only on the dialects that emit them, so an unsupported combination | `virtual()` | MySQL, MariaDB, SQLite | PostgreSQL supports `STORED` only | | `collation($collation)` | MySQL, MariaDB, PostgreSQL, SQLite | ClickHouse collates in `ORDER BY`; MongoDB per collection | | `unique()` | MySQL, MariaDB, PostgreSQL, SQLite | ClickHouse enforces no uniqueness; MongoDB uses a unique index | -| `after($column)` | MySQL, MariaDB, SQLite | PostgreSQL cannot order columns; MongoDB documents have none | +| `after($column)` | MySQL, MariaDB | PostgreSQL cannot order columns; SQLite's `ADD COLUMN` has no `AFTER` clause; MongoDB documents have no order | | `comment($text)` | MySQL, MariaDB, SQLite, ClickHouse, MongoDB | PostgreSQL needs a separate statement — use `commentOnColumn()` | | `ttl($expression)` | ClickHouse | — | | `userType($name)` | PostgreSQL | — | diff --git a/src/Query/Schema/Column/SQLite.php b/src/Query/Schema/Column/SQLite.php index 58981bd..12b7d70 100644 --- a/src/Query/Schema/Column/SQLite.php +++ b/src/Query/Schema/Column/SQLite.php @@ -12,7 +12,6 @@ class SQLite extends Column { use Trait\Collation; - use Trait\Positioning; use Trait\Comment; use Trait\Unique; use Trait\Generated; diff --git a/src/Query/Schema/Column/Trait/Positioning.php b/src/Query/Schema/Column/Trait/Positioning.php index a32eb3f..96a949b 100644 --- a/src/Query/Schema/Column/Trait/Positioning.php +++ b/src/Query/Schema/Column/Trait/Positioning.php @@ -3,8 +3,12 @@ namespace Utopia\Query\Schema\Column\Trait; /** - * Positioning a column relative to another via AFTER. PostgreSQL cannot - * control column order, and MongoDB documents have no column order. + * Positioning a column relative to another via AFTER, which only MySQL and + * MariaDB accept in ALTER TABLE. + * + * PostgreSQL cannot control column order and MongoDB documents have none. + * SQLite's ALTER TABLE ADD COLUMN has no AFTER clause -- emitting one is a + * syntax error -- and ClickHouse supports it but the compiler does not emit it. */ trait Positioning { diff --git a/tests/Integration/Schema/MySQLIntegrationTest.php b/tests/Integration/Schema/MySQLIntegrationTest.php index 3b73703..92d45b0 100644 --- a/tests/Integration/Schema/MySQLIntegrationTest.php +++ b/tests/Integration/Schema/MySQLIntegrationTest.php @@ -42,6 +42,21 @@ public function testCreateTableWithBasicColumns(): void $this->assertSame('100', (string) $nameCol['CHARACTER_MAXIMUM_LENGTH']); // @phpstan-ignore cast.string } + public function testColumnCollationIsAppliedByTheServer(): void + { + $table = 'test_collation_' . uniqid(); + $this->trackMysqlTable($table); + + $result = $this->schema->table($table) + ->string('name', 100)->collation('utf8mb4_bin') + ->create(); + + $this->mysqlStatement($result->query); + + $nameCol = $this->findColumn($this->fetchMysqlColumns($table), 'name'); + $this->assertSame('utf8mb4_bin', $nameCol['COLLATION_NAME']); + } + public function testCreateTableWithPrimaryKeyAndUnique(): void { $table = 'test_pk_uniq_' . uniqid(); diff --git a/tests/Integration/Schema/PostgreSQLIntegrationTest.php b/tests/Integration/Schema/PostgreSQLIntegrationTest.php index 7b23898..3a434bb 100644 --- a/tests/Integration/Schema/PostgreSQLIntegrationTest.php +++ b/tests/Integration/Schema/PostgreSQLIntegrationTest.php @@ -41,6 +41,21 @@ public function testCreateTableWithBasicColumns(): void $this->assertSame('100', (string) $nameCol['character_maximum_length']); // @phpstan-ignore cast.string } + public function testColumnCollationIsAppliedByTheServer(): void + { + $table = 'test_collation_' . uniqid(); + $this->trackPostgresTable($table); + + $result = $this->schema->table($table) + ->string('name', 100)->collation('C') + ->create(); + + $this->postgresStatement($result->query); + + $nameCol = $this->findColumn($this->fetchPostgresColumns($table), 'name'); + $this->assertSame('C', $nameCol['collation_name']); + } + public function testCreateTableWithIdentityColumn(): void { $table = 'test_identity_' . uniqid(); diff --git a/tests/Query/Schema/SQLiteTest.php b/tests/Query/Schema/SQLiteTest.php index ddf4413..2162c07 100644 --- a/tests/Query/Schema/SQLiteTest.php +++ b/tests/Query/Schema/SQLiteTest.php @@ -545,6 +545,40 @@ public function testSerialColumnMapsToInteger(): void $this->assertSame('CREATE TABLE `t` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL)', $result->query); } + /** + * SQLite's ALTER TABLE ADD COLUMN has no AFTER clause, so exposing after() + * here produced DDL that failed at execution. Asserted by running it. + */ + public function testColumnDoesNotExposeAfter(): void + { + $this->assertNotContains('after', \get_class_methods((new Schema())->table('t')->integer('b'))); + } + + public function testEmittedAlterExecutesAgainstRealSqlite(): void + { + $pdo = new \PDO('sqlite::memory:'); + $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + $pdo->exec('CREATE TABLE t (a INTEGER)'); + + $alter = (new Schema())->table('t')->integer('b')->alter()->query; + $pdo->exec($alter); + + $statement = $pdo->query('SELECT name FROM pragma_table_info(\'t\')'); + $this->assertNotFalse($statement); + $this->assertSame(['a', 'b'], $statement->fetchAll(\PDO::FETCH_COLUMN)); + } + + public function testEmittedCollationExecutesAgainstRealSqlite(): void + { + $pdo = new \PDO('sqlite::memory:'); + $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + + $create = (new Schema())->table('t')->string('name')->collation('NOCASE')->create()->query; + $pdo->exec($create); + + $this->assertStringContainsString('COLLATE NOCASE', $create); + } + public function testColumnCollationIsEmitted(): void { $result = (new Schema())->table('t')