diff --git a/src/Driver/Compiler.php b/src/Driver/Compiler.php index d346911e..f07e6d2a 100644 --- a/src/Driver/Compiler.php +++ b/src/Driver/Compiler.php @@ -205,11 +205,11 @@ protected function upsertQuery(QueryParameters $params, Quoter $q, array $tokens } $head = \sprintf( - 'INSERT INTO %s (%s) VALUES %s ON CONFLICT (%s)', + 'INSERT INTO %s (%s) VALUES %s ON CONFLICT %s', $this->name($params, $q, $tokens['table'], true), $this->columns($params, $q, $tokens['columns']), \implode(', ', $values), - $this->columns($params, $q, $target), + $this->conflictTarget($params, $q, $onConflict), ); if ($onConflict->getAction() === ConflictAction::Nothing) { @@ -228,6 +228,18 @@ protected function upsertQuery(QueryParameters $params, Quoter $q, array $tokens return $head . ' DO UPDATE SET ' . $updates; } + /** + * Render the conflict target after `ON CONFLICT`: `(col, ...)`, plus — on drivers + * that inherit the Postgres inference clause — an index predicate + * `(col, ...) WHERE `. Driver compilers override to extend it. + * + * @psalm-return non-empty-string + */ + protected function conflictTarget(QueryParameters $params, Quoter $q, OnConflict $onConflict): string + { + return '(' . $this->columns($params, $q, $onConflict->getTarget()) . ')'; + } + /** * @psalm-assert OnConflict $tokens['onConflict'] */ @@ -261,7 +273,10 @@ protected function upsertUpdateClause( string $sourceAlias, ?string $targetAlias = null, ): string { - $source = $this->quoteIdentifier($sourceAlias); + // EXCLUDED is a Postgres/SQLite keyword pseudo-table, not a real alias: it must + // stay unquoted (Postgres rejects the quoted "EXCLUDED" with "missing FROM-clause + // entry"). MySQL/SQLServer pass real aliases (new_row/source) that are quoted. + $source = $sourceAlias === 'EXCLUDED' ? 'EXCLUDED' : $this->quoteIdentifier($sourceAlias); $targetPrefix = $targetAlias !== null ? $this->quoteIdentifier($targetAlias) . '.' : ''; if ($update === null) { diff --git a/src/Driver/CompilerCache.php b/src/Driver/CompilerCache.php index 7a610179..03359b78 100644 --- a/src/Driver/CompilerCache.php +++ b/src/Driver/CompilerCache.php @@ -19,6 +19,7 @@ use Cycle\Database\Injection\ParameterInterface; use Cycle\Database\Injection\SubQuery; use Cycle\Database\Query\OnConflict; +use Cycle\Database\Query\OnConflictWithPredicate; use Cycle\Database\Query\QueryInterface; use Cycle\Database\Query\QueryParameters; use Cycle\Database\Query\SelectQuery; @@ -176,6 +177,13 @@ protected function hashUpsertQuery(QueryParameters $params, array $tokens): stri return $hash; } + // The index-inference predicate is rendered between the conflict target and + // DO UPDATE, so its parameters must be pushed before the update parameters. + // Reuse the regular where-hasher to keep that order identical to Compiler::where(). + if ($onConflict instanceof OnConflictWithPredicate && ($predicate = $onConflict->getIndexPredicate()) !== []) { + $hash .= '_w' . $this->hashWhere($params, $predicate); + } + // Driver-specific subclasses extend getCacheKey() to append their own fields // and push any embedded fragment parameters via $params. return $hash . '_oc' . $onConflict->getCacheKey($params); diff --git a/src/Driver/MySQL/MySQLCompiler.php b/src/Driver/MySQL/MySQLCompiler.php index 7659587f..2846a2e1 100644 --- a/src/Driver/MySQL/MySQLCompiler.php +++ b/src/Driver/MySQL/MySQLCompiler.php @@ -57,27 +57,28 @@ protected function upsertQuery(QueryParameters $params, Quoter $q, array $tokens $values[] = $this->value($params, $q, $value); } - $rowAlias = $onConflict->getRowAlias(); - - $head = \sprintf( - 'INSERT INTO %s (%s) VALUES %s AS %s', + $base = \sprintf( + 'INSERT INTO %s (%s) VALUES %s', $this->name($params, $q, $tokens['table'], true), $this->columns($params, $q, $tokens['columns']), \implode(', ', $values), - $this->quoteIdentifier($rowAlias), ); if ($onConflict->getAction() === ConflictAction::Nothing) { // MySQL has no DO NOTHING — emulate with a no-op self-assignment on the // conflict-target column (or the first inserted column as a fallback). - // Using the target column is the conventional idiom and is more predictable - // for schemas where the first inserted column is unrelated to the conflict. + // No row alias is emitted here: without `AS ` the bare `col = col` + // is unambiguous; with the alias in scope MySQL rejects it as ambiguous. $target = $onConflict->getTarget(); $noopColumn = $target[0] ?? $tokens['columns'][0]; $name = $this->name($params, $q, $noopColumn); - return $head . ' ON DUPLICATE KEY UPDATE ' . \sprintf('%s = %s', $name, $name); + return $base . ' ON DUPLICATE KEY UPDATE ' . \sprintf('%s = %s', $name, $name); } + // DO UPDATE references the inserted row via `col = .col`, so the alias is required. + $rowAlias = $onConflict->getRowAlias(); + $head = $base . ' AS ' . $this->quoteIdentifier($rowAlias); + $updates = $this->upsertUpdateClause( $params, $q, diff --git a/src/Driver/Postgres/PostgresCompiler.php b/src/Driver/Postgres/PostgresCompiler.php index f8f63052..451fc020 100644 --- a/src/Driver/Postgres/PostgresCompiler.php +++ b/src/Driver/Postgres/PostgresCompiler.php @@ -128,8 +128,14 @@ protected function compileJsonOrderBy(string $path): FragmentInterface */ private function postgresConflictTarget(QueryParameters $params, Quoter $q, PostgresOnConflict $onConflict): string { + $predicate = $onConflict->getIndexPredicate(); + $constraint = $onConflict->getConstraint(); if ($constraint !== null) { + $predicate === [] or throw new CompilerException( + 'ON CONFLICT ON CONSTRAINT cannot be combined with an index-inference predicate (targetWhere()).', + ); + return 'ON CONSTRAINT ' . $this->quoteIdentifier($constraint); } @@ -138,7 +144,15 @@ private function postgresConflictTarget(QueryParameters $params, Quoter $q, Post 'Upsert query must define a conflict target (columns or constraint).', ); - return \sprintf('(%s)', $this->columns($params, $q, $target)); + $result = \sprintf('(%s)', $this->columns($params, $q, $target)); + + if ($predicate === []) { + return $result; + } + + $where = \trim($this->where($params, $q, $predicate)); + + return $where === '' ? $result : $result . ' WHERE ' . $where; } /** diff --git a/src/Driver/Postgres/PostgresOnConflict.php b/src/Driver/Postgres/PostgresOnConflict.php index d8fd68a6..2e748d55 100644 --- a/src/Driver/Postgres/PostgresOnConflict.php +++ b/src/Driver/Postgres/PostgresOnConflict.php @@ -4,9 +4,11 @@ namespace Cycle\Database\Driver\Postgres; +use Cycle\Database\Driver\SQLite\SQLiteOnConflict; use Cycle\Database\Exception\BuilderException; use Cycle\Database\Query\ConflictAction; use Cycle\Database\Query\OnConflict; +use Cycle\Database\Query\OnConflictWithPredicate; use Cycle\Database\Query\QueryParameters; /** @@ -14,11 +16,13 @@ * * Adds: * - {@see self::onConstraint()} — `ON CONFLICT ON CONSTRAINT ` target. + * - {@see OnConflictWithPredicate::targetWhere()} — `ON CONFLICT (cols) WHERE ` + * index-inference for partial unique indexes (shared with {@see SQLiteOnConflict}). * * Use {@see self::from()} inside the Postgres compiler to narrow a base * {@see OnConflict} instance into this type. */ -final class PostgresOnConflict extends OnConflict +final class PostgresOnConflict extends OnConflictWithPredicate { /** * @param list $target @@ -30,8 +34,9 @@ protected function __construct( ConflictAction $action, null|array $update, protected ?string $constraint = null, + array $indexPredicate = [], ) { - parent::__construct($target, $action, $update); + parent::__construct($target, $action, $update, $indexPredicate); } /** @@ -57,12 +62,15 @@ public static function from(OnConflict $options): static return $options; } - if ($options::class !== OnConflict::class) { + // Base OnConflict and the feature-compatible SQLite sibling narrow cleanly; + // SQLite never carries a constraint, so nothing is lost. MySQL/SQLServer reject. + if (!$options instanceof OnConflictWithPredicate && $options::class !== OnConflict::class) { throw new BuilderException(\sprintf( - 'Cannot narrow %s to %s. Use the base OnConflict, or %s directly.', + 'Cannot narrow %s to %s. Use the base OnConflict, %s, or %s directly.', $options::class, self::class, self::class, + SQLiteOnConflict::class, )); } @@ -70,6 +78,7 @@ public static function from(OnConflict $options): static target: $options->getTarget(), action: $options->getAction(), update: $options->getUpdate(), + indexPredicate: $options instanceof OnConflictWithPredicate ? $options->getIndexPredicate() : [], ); } diff --git a/src/Driver/SQLite/SQLiteCompiler.php b/src/Driver/SQLite/SQLiteCompiler.php index 2ecd5ecb..d2e4e2d4 100644 --- a/src/Driver/SQLite/SQLiteCompiler.php +++ b/src/Driver/SQLite/SQLiteCompiler.php @@ -19,6 +19,7 @@ use Cycle\Database\Injection\FragmentInterface; use Cycle\Database\Injection\Parameter; use Cycle\Database\Injection\ParameterInterface; +use Cycle\Database\Query\OnConflict; use Cycle\Database\Query\QueryParameters; class SQLiteCompiler extends Compiler implements CachingCompilerInterface @@ -125,4 +126,26 @@ protected function compileJsonOrderBy(string $path): FragmentInterface { return new CompileJson($path); } + + /** + * SQLite inherits the Postgres `ON CONFLICT (cols) WHERE ` inference + * clause, so it supports an index predicate on the conflict target. + * + * @psalm-return non-empty-string + */ + protected function conflictTarget(QueryParameters $params, Quoter $q, OnConflict $onConflict): string + { + $onConflict = SQLiteOnConflict::from($onConflict); + + $target = '(' . $this->columns($params, $q, $onConflict->getTarget()) . ')'; + + $predicate = $onConflict->getIndexPredicate(); + if ($predicate === []) { + return $target; + } + + $where = \trim($this->where($params, $q, $predicate)); + + return $where === '' ? $target : $target . ' WHERE ' . $where; + } } diff --git a/src/Driver/SQLite/SQLiteOnConflict.php b/src/Driver/SQLite/SQLiteOnConflict.php new file mode 100644 index 00000000..8666984c --- /dev/null +++ b/src/Driver/SQLite/SQLiteOnConflict.php @@ -0,0 +1,52 @@ +` index-inference form via + * {@see OnConflictWithPredicate::targetWhere()}. It does NOT support + * `ON CONFLICT ON CONSTRAINT`, so narrowing a {@see PostgresOnConflict} that + * carries a constraint into this type is rejected. + */ +final class SQLiteOnConflict extends OnConflictWithPredicate +{ + public static function from(OnConflict $options): static + { + if ($options instanceof self) { + return $options; + } + + if ($options instanceof PostgresOnConflict && $options->getConstraint() !== null) { + throw new BuilderException( + 'SQLite does not support ON CONFLICT ON CONSTRAINT; use a column target instead.', + ); + } + + if (!$options instanceof OnConflictWithPredicate && $options::class !== OnConflict::class) { + throw new BuilderException(\sprintf( + 'Cannot narrow %s to %s. Use the base OnConflict, %s, or %s directly.', + $options::class, + self::class, + self::class, + PostgresOnConflict::class, + )); + } + + return new self( + target: $options->getTarget(), + action: $options->getAction(), + update: $options->getUpdate(), + indexPredicate: $options instanceof OnConflictWithPredicate ? $options->getIndexPredicate() : [], + ); + } +} diff --git a/src/Query/OnConflict.php b/src/Query/OnConflict.php index 47ba5112..46124470 100644 --- a/src/Query/OnConflict.php +++ b/src/Query/OnConflict.php @@ -16,7 +16,10 @@ * * Base class carries only cross-driver features (target columns, action, * column-level update spec). Driver-specific extensions live in subclasses: - * - {@see \Cycle\Database\Driver\Postgres\PostgresOnConflict} — onConstraint(), where(). + * - {@see OnConflictWithPredicate} — targetWhere() (index-inference predicate), + * shared base for the two drivers that inherit the Postgres inference clause: + * - {@see \Cycle\Database\Driver\Postgres\PostgresOnConflict} — also onConstraint(). + * - {@see \Cycle\Database\Driver\SQLite\SQLiteOnConflict}. * - {@see \Cycle\Database\Driver\MySQL\MySQLOnConflict} — withRowAlias(). * - {@see \Cycle\Database\Driver\SQLServer\SQLServerOnConflict} — where() (MERGE). */ @@ -60,8 +63,12 @@ public static function target(string|array ...$columns): static * fields are taken from the input if it is already of this type; otherwise * default values are used for those fields. * - * Subclasses MUST reject other driver-specific subclasses (e.g., passing - * PostgresOnConflict to MySQLOnConflict::from() must throw). + * Subclasses MUST reject driver-specific subclasses they cannot represent + * (e.g., passing PostgresOnConflict to MySQLOnConflict::from() must throw). + * Feature-compatible siblings, however, convert without error: PostgresOnConflict + * and SQLiteOnConflict both understand the index-inference predicate and narrow + * into each other (the one exception is a Postgres constraint target, which SQLite + * cannot express). */ public static function from(self $options): static { @@ -75,6 +82,22 @@ public static function from(self $options): static * null — overwrite every inserted column from the source row. * list of strings — overwrite only the listed columns from the source row. * column => value map — custom expressions/values per column. + * + * Referencing the inserted ("excluded") row inside a custom expression: use a raw + * {@see \Cycle\Database\Injection\Fragment}, NOT an {@see \Cycle\Database\Injection\Expression}. + * Expression quotes every identifier, and Postgres rejects the quoted "EXCLUDED" + * pseudo-table (`missing FROM-clause entry for table "EXCLUDED"`); a Fragment is + * emitted verbatim. The pseudo-table name is driver-specific — EXCLUDED on + * Postgres/SQLite, the row alias (default `new_row`) on MySQL — so such expressions + * are inherently non-portable: + * + * // Postgres / SQLite + * ->doUpdate(['hits' => new Fragment('counters.hits + EXCLUDED.hits')]) + * // MySQL + * ->doUpdate(['hits' => new Fragment('counters.hits + new_row.hits')]) + * + * Use {@see \Cycle\Database\Injection\Expression} only for expressions over real + * table columns (which should be quoted), not for the excluded-row reference. */ public function doUpdate(?array $columnsOrMap = null): static { diff --git a/src/Query/OnConflictWhere.php b/src/Query/OnConflictWhere.php new file mode 100644 index 00000000..bcc766b4 --- /dev/null +++ b/src/Query/OnConflictWhere.php @@ -0,0 +1,34 @@ +whereTokens; + } +} diff --git a/src/Query/OnConflictWithPredicate.php b/src/Query/OnConflictWithPredicate.php new file mode 100644 index 00000000..b55fa4b6 --- /dev/null +++ b/src/Query/OnConflictWithPredicate.php @@ -0,0 +1,98 @@ +` form used to match + * a partial unique index. + * + * Only drivers whose upsert syntax inherits the Postgres `ON CONFLICT` inference + * clause expose this: {@see \Cycle\Database\Driver\Postgres\PostgresOnConflict} and + * {@see \Cycle\Database\Driver\SQLite\SQLiteOnConflict}. MySQL (`ON DUPLICATE KEY + * UPDATE`) and SQL Server (`MERGE`) have no equivalent, which is why this feature + * lives below the base {@see OnConflict} instead of on it. + * + * Because the feature set matches, these two subclasses convert into each other + * through {@see self::from()} without error (see the subclasses for the one + * exception: a Postgres constraint target cannot be narrowed to SQLite). + * + * The predicate is stored as a where-token array (the same shape SelectQuery uses), + * so the compiler renders it through its regular where-renderer and the query cache + * hashes it through its regular where-hasher. + */ +abstract class OnConflictWithPredicate extends OnConflict +{ + /** + * @param list $target + * @param list|array|null $update + * @param array $indexPredicate Where tokens, see {@see OnConflictWhere}. + */ + protected function __construct( + array $target, + ConflictAction $action, + null|array $update, + protected array $indexPredicate = [], + ) { + parent::__construct($target, $action, $update); + } + + /** + * Index-inference predicate for the conflict target — the `WHERE` in + * `ON CONFLICT (cols) WHERE `. Use it to match a partial unique index. + * + * This is NOT the `DO UPDATE ... WHERE` condition on the update branch. + * + * Accepts the same argument shapes as {@see \Cycle\Database\Query\Traits\WhereTrait::where()}, + * plus a raw-SQL shorthand: + * - single raw string — emitted verbatim, identifiers NOT quoted (the one shape + * `where()` cannot take): `->targetWhere('resource_key IS NOT NULL')`; + * - simple condition — `->targetWhere('resource_key', '!=', null)` (→ `IS NOT NULL`), + * `->targetWhere('priority', '>', 5)`, `->targetWhere('col', $value)`; + * - array form — `->targetWhere(['priority' => ['>' => 5], 'resource_key' => ['!=' => null]])`; + * - {@see \Cycle\Database\Injection\FragmentInterface} — e.g. an {@see \Cycle\Database\Injection\Expression}; + * - {@see \Closure} — receives an {@see OnConflictWhere} builder for multi-condition + * AND/OR groups: `->targetWhere(fn(OnConflictWhere $w) => $w->where(...)->orWhere(...))`. + * + * Identifiers in every non-raw-string form are quoted; values are bound as parameters. + * + * @param mixed ...$args See {@see \Cycle\Database\Query\Traits\WhereTrait::where()}. + */ + public function targetWhere(mixed ...$args): static + { + $args === [] and throw new BuilderException('targetWhere() requires at least one argument.'); + + $clone = clone $this; + + // A single string is not a valid builder condition — treat it as raw SQL. + if (\count($args) === 1 && \is_string($args[0])) { + $args[0] === '' and throw new BuilderException('Index predicate must not be empty.'); + + $clone->indexPredicate = [['AND', new Fragment($args[0])]]; + return $clone; + } + + $builder = new OnConflictWhere(); + if (\count($args) === 1 && $args[0] instanceof \Closure) { + ($args[0])($builder); + } else { + $builder->where(...$args); + } + + $clone->indexPredicate = $builder->getWhereTokens(); + return $clone; + } + + /** + * Where tokens of the index-inference predicate (empty when none was set). + */ + public function getIndexPredicate(): array + { + return $this->indexPredicate; + } +} diff --git a/tests/Database/Functional/Driver/Common/Query/UpsertQueryTest.php b/tests/Database/Functional/Driver/Common/Query/UpsertQueryTest.php index 9e930d72..886351d5 100644 --- a/tests/Database/Functional/Driver/Common/Query/UpsertQueryTest.php +++ b/tests/Database/Functional/Driver/Common/Query/UpsertQueryTest.php @@ -5,6 +5,7 @@ namespace Cycle\Database\Tests\Functional\Driver\Common\Query; use Cycle\Database\Driver\CompilerInterface; +use Cycle\Database\Driver\Handler; use Cycle\Database\Exception\CompilerException; use Cycle\Database\Query\InsertQuery; use Cycle\Database\Query\OnConflict; @@ -60,4 +61,77 @@ public function testEmptyValuesRejected(): void $this->expectException(CompilerException::class); (string) $q; } + + // --- Runtime (execution against a live database) --- + + public function testRuntimeDoUpdateUpdatesExistingRow(): void + { + $this->makeUpsertUsersTable(); + $this->database->insert('upsert_users')->values(['email' => 'a@b.c', 'name' => 'Old'])->run(); + + $this->database->insert('upsert_users') + ->values(['email' => 'a@b.c', 'name' => 'New']) + ->onConflict('email') + ->run(); + + $rows = $this->database->select()->from('upsert_users')->fetchAll(); + $this->assertCount(1, $rows, 'Conflicting row must be updated, not duplicated.'); + $this->assertSame('New', $rows[0]['name']); + } + + public function testRuntimeDoUpdateSelectiveColumns(): void + { + $this->makeUpsertUsersTable(); + $this->database->insert('upsert_users') + ->values(['email' => 'a@b.c', 'name' => 'Old', 'tag' => 'keep'])->run(); + + $this->database->insert('upsert_users') + ->values(['email' => 'a@b.c', 'name' => 'New', 'tag' => 'drop']) + ->onConflict(OnConflict::target('email')->doUpdate(['name'])) + ->run(); + + $rows = $this->database->select()->from('upsert_users')->fetchAll(); + $this->assertCount(1, $rows); + $this->assertSame('New', $rows[0]['name']); + $this->assertSame('keep', $rows[0]['tag'], 'Columns outside the update list must be preserved.'); + } + + public function testRuntimeDoNothingPreservesExistingRow(): void + { + $this->makeUpsertUsersTable(); + $this->database->insert('upsert_users')->values(['email' => 'a@b.c', 'name' => 'Old'])->run(); + + $this->database->insert('upsert_users') + ->values(['email' => 'a@b.c', 'name' => 'New']) + ->onConflict(OnConflict::target('email')->doNothing()) + ->run(); + + $rows = $this->database->select()->from('upsert_users')->fetchAll(); + $this->assertCount(1, $rows); + $this->assertSame('Old', $rows[0]['name'], 'DO NOTHING must keep the original row.'); + } + + public function testRuntimeInsertsWhenNoConflict(): void + { + $this->makeUpsertUsersTable(); + $this->database->insert('upsert_users')->values(['email' => 'a@b.c', 'name' => 'Old'])->run(); + + $this->database->insert('upsert_users') + ->values(['email' => 'x@y.z', 'name' => 'New']) + ->onConflict('email') + ->run(); + + $this->assertCount(2, $this->database->select()->from('upsert_users')->fetchAll()); + } + + private function makeUpsertUsersTable(): void + { + $schema = $this->schema('upsert_users'); + $schema->primary('id'); + $schema->string('email'); + $schema->string('name'); + $schema->string('tag')->nullable(true); + $schema->index(['email'])->unique(true); + $schema->save(Handler::DO_ALL); + } } diff --git a/tests/Database/Functional/Driver/MySQL/Query/UpsertQueryTest.php b/tests/Database/Functional/Driver/MySQL/Query/UpsertQueryTest.php index 6dd23ddc..f3ecf04f 100644 --- a/tests/Database/Functional/Driver/MySQL/Query/UpsertQueryTest.php +++ b/tests/Database/Functional/Driver/MySQL/Query/UpsertQueryTest.php @@ -66,7 +66,7 @@ public function testDoNothingEmulated(): void ->onConflict(OnConflict::target('request_id')->doNothing()); $this->assertSameQuery( - 'INSERT INTO {logs} ({request_id}, {payload}) VALUES (?, ?) AS {new_row} ' + 'INSERT INTO {logs} ({request_id}, {payload}) VALUES (?, ?) ' . 'ON DUPLICATE KEY UPDATE {request_id} = {request_id}', $q, ); @@ -82,7 +82,7 @@ public function testDoNothingUsesTargetWhenItIsNotFirstColumn(): void ->onConflict(OnConflict::target('request_id')->doNothing()); $this->assertSameQuery( - 'INSERT INTO {logs} ({payload}, {request_id}) VALUES (?, ?) AS {new_row} ' + 'INSERT INTO {logs} ({payload}, {request_id}) VALUES (?, ?) ' . 'ON DUPLICATE KEY UPDATE {request_id} = {request_id}', $q, ); diff --git a/tests/Database/Functional/Driver/Postgres/Query/UpsertQueryTest.php b/tests/Database/Functional/Driver/Postgres/Query/UpsertQueryTest.php index 0f68ef2e..2e8fe81c 100644 --- a/tests/Database/Functional/Driver/Postgres/Query/UpsertQueryTest.php +++ b/tests/Database/Functional/Driver/Postgres/Query/UpsertQueryTest.php @@ -4,9 +4,12 @@ namespace Cycle\Database\Tests\Functional\Driver\Postgres\Query; +use Cycle\Database\Driver\Handler; use Cycle\Database\Driver\Postgres\PostgresOnConflict; -use Cycle\Database\Injection\Expression; +use Cycle\Database\Exception\CompilerException; +use Cycle\Database\Injection\Fragment; use Cycle\Database\Query\OnConflict; +use Cycle\Database\Query\OnConflictWhere; use Cycle\Database\Tests\Functional\Driver\Common\Query\UpsertQueryTest as CommonClass; /** @@ -25,7 +28,7 @@ public function testShorthandSingleColumn(): void $this->assertSameQuery( 'INSERT INTO {users} ({email}, {name}) VALUES (?, ?) ' - . 'ON CONFLICT ({email}) DO UPDATE SET {name} = {EXCLUDED}.{name}', + . 'ON CONFLICT ({email}) DO UPDATE SET {name} = EXCLUDED.{name}', $q, ); } @@ -38,22 +41,24 @@ public function testDoUpdateExplicitColumns(): void $this->assertSameQuery( 'INSERT INTO {users} ({email}, {name}, {visits}) VALUES (?, ?, ?) ' - . 'ON CONFLICT ({email}) DO UPDATE SET {name} = {EXCLUDED}.{name}, {visits} = {EXCLUDED}.{visits}', + . 'ON CONFLICT ({email}) DO UPDATE SET {name} = EXCLUDED.{name}, {visits} = EXCLUDED.{visits}', $q, ); } - public function testDoUpdateWithExpression(): void + public function testDoUpdateWithFragmentReferencingExcluded(): void { + // EXCLUDED must stay unquoted, so reference it with a raw Fragment (not an + // Expression, which would quote it and break at runtime on Postgres). $q = $this->database->insert('counters') ->values(['key' => 'x', 'n' => 1]) ->onConflict(OnConflict::target('key')->doUpdate([ - 'n' => new Expression('counters.n + EXCLUDED.n'), + 'n' => new Fragment('counters.n + EXCLUDED.n'), ])); $this->assertSameQuery( 'INSERT INTO {counters} ({key}, {n}) VALUES (?, ?) ' - . 'ON CONFLICT ({key}) DO UPDATE SET {n} = {counters}.{n} + {EXCLUDED}.{n}', + . 'ON CONFLICT ({key}) DO UPDATE SET {n} = counters.n + EXCLUDED.n', $q, ); } @@ -78,8 +83,162 @@ public function testConstraintTarget(): void $this->assertSameQuery( 'INSERT INTO {users} ({email}, {name}) VALUES (?, ?) ' - . 'ON CONFLICT ON CONSTRAINT {users_email_unique} DO UPDATE SET {name} = {EXCLUDED}.{name}', + . 'ON CONFLICT ON CONSTRAINT {users_email_unique} DO UPDATE SET {name} = EXCLUDED.{name}', $q, ); } + + public function testTargetWherePartialIndex(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'b']) + ->onConflict( + PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['body']), + ); + + $this->assertSameQuery( + 'INSERT INTO {routes} ({resource_key}, {route}, {body}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE resource_key IS NOT NULL ' + . 'DO UPDATE SET {body} = EXCLUDED.{body}', + $q, + ); + } + + public function testTargetWhereVariadicCondition(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'priority' => 1]) + ->onConflict( + PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('priority', '>', 5) + ->doUpdate(['priority']), + ); + + $this->assertSameQueryWithParameters( + 'INSERT INTO {routes} ({resource_key}, {route}, {priority}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE {priority} > ? ' + . 'DO UPDATE SET {priority} = EXCLUDED.{priority}', + ['k', '/r', 1, 5], + $q, + ); + } + + public function testTargetWhereClosureBuilder(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'b']) + ->onConflict( + PostgresOnConflict::target('resource_key', 'route') + ->targetWhere(static fn(OnConflictWhere $w) => $w->where('resource_key', '!=', null)) + ->doUpdate(['body']), + ); + + $this->assertSameQuery( + 'INSERT INTO {routes} ({resource_key}, {route}, {body}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE {resource_key} IS NOT NULL ' + . 'DO UPDATE SET {body} = EXCLUDED.{body}', + $q, + ); + } + + public function testTargetWhereMultiRow(): void + { + // Multi-row upsert is not cached, so the predicate is rendered on the + // direct compile path (bypassing the cache's where-hasher). + $q = $this->database->insert('routes') + ->values([ + ['resource_key' => 'k1', 'route' => '/a', 'body' => 'b1'], + ['resource_key' => 'k2', 'route' => '/b', 'body' => 'b2'], + ]) + ->onConflict( + PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('priority', '>', 5) + ->doUpdate(['body']), + ); + + $this->assertSameQueryWithParameters( + 'INSERT INTO {routes} ({resource_key}, {route}, {body}) VALUES (?, ?, ?), (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE {priority} > ? ' + . 'DO UPDATE SET {body} = EXCLUDED.{body}', + ['k1', '/a', 'b1', 'k2', '/b', 'b2', 5], + $q, + ); + } + + public function testConstraintWithPredicateRejected(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r']) + ->onConflict( + PostgresOnConflict::onConstraint('routes_unique') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['route']), + ); + + $this->expectException(CompilerException::class); + (string) $q; + } + + public function testTargetWherePartialIndexRuntime(): void + { + $schema = $this->schema('upsert_routes'); + $schema->primary('id'); + $schema->string('resource_key')->nullable(true); + $schema->string('route'); + $schema->string('body'); + $schema->save(Handler::DO_ALL); + + $this->database->execute( + 'CREATE UNIQUE INDEX upsert_routes_partial ON upsert_routes (resource_key, route) ' + . 'WHERE resource_key IS NOT NULL', + ); + + $this->database->insert('upsert_routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'old'])->run(); + + $upsert = fn(string $key, string $body) => $this->database->insert('upsert_routes') + ->values(['resource_key' => $key, 'route' => '/r', 'body' => $body]) + ->onConflict( + PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['body']), + )->run(); + + // Matching partial index → conflict → update in place. + $upsert('k', 'new'); + $rows = $this->database->select()->from('upsert_routes')->fetchAll(); + $this->assertCount(1, $rows); + $this->assertSame('new', $rows[0]['body']); + + // Different key → no conflict → new row inserted. + $upsert('k2', 'second'); + $this->assertCount(2, $this->database->select()->from('upsert_routes')->fetchAll()); + } + + public function testDoUpdateFragmentExcludedRuntime(): void + { + // Locks the documented EXCLUDED-via-Fragment path: an Expression here would + // emit a quoted "EXCLUDED" and fail on Postgres at runtime. + $schema = $this->schema('upsert_counters'); + $schema->primary('id'); + $schema->string('k'); + $schema->integer('n'); + $schema->index(['k'])->unique(true); + $schema->save(Handler::DO_ALL); + + $this->database->insert('upsert_counters')->values(['k' => 'x', 'n' => 10])->run(); + + $this->database->insert('upsert_counters') + ->values(['k' => 'x', 'n' => 5]) + ->onConflict(OnConflict::target('k')->doUpdate([ + 'n' => new Fragment('upsert_counters.n + EXCLUDED.n'), + ])) + ->run(); + + $rows = $this->database->select()->from('upsert_counters')->fetchAll(); + $this->assertCount(1, $rows); + $this->assertSame(15, (int) $rows[0]['n']); + } } diff --git a/tests/Database/Functional/Driver/SQLite/Query/UpsertQueryTest.php b/tests/Database/Functional/Driver/SQLite/Query/UpsertQueryTest.php index f7f30c3d..4f087b63 100644 --- a/tests/Database/Functional/Driver/SQLite/Query/UpsertQueryTest.php +++ b/tests/Database/Functional/Driver/SQLite/Query/UpsertQueryTest.php @@ -4,8 +4,12 @@ namespace Cycle\Database\Tests\Functional\Driver\SQLite\Query; +use Cycle\Database\Driver\Handler; +use Cycle\Database\Driver\SQLite\SQLiteOnConflict; use Cycle\Database\Injection\Expression; +use Cycle\Database\Injection\Fragment; use Cycle\Database\Query\OnConflict; +use Cycle\Database\Query\OnConflictWhere; use Cycle\Database\Tests\Functional\Driver\Common\Query\UpsertQueryTest as CommonClass; /** @@ -23,7 +27,7 @@ public function testShorthandSingleColumn(): void ->onConflict('email'); $this->assertSameQuery( - 'INSERT INTO {users} ({email}, {name}) VALUES (?, ?) ON CONFLICT ({email}) DO UPDATE SET {name} = {EXCLUDED}.{name}', + 'INSERT INTO {users} ({email}, {name}) VALUES (?, ?) ON CONFLICT ({email}) DO UPDATE SET {name} = EXCLUDED.{name}', $q, ); } @@ -36,22 +40,24 @@ public function testDoUpdateExplicitColumns(): void $this->assertSameQuery( 'INSERT INTO {users} ({email}, {name}, {visits}) VALUES (?, ?, ?) ' - . 'ON CONFLICT ({email}) DO UPDATE SET {name} = {EXCLUDED}.{name}, {visits} = {EXCLUDED}.{visits}', + . 'ON CONFLICT ({email}) DO UPDATE SET {name} = EXCLUDED.{name}, {visits} = EXCLUDED.{visits}', $q, ); } - public function testDoUpdateWithExpression(): void + public function testDoUpdateWithFragmentReferencingExcluded(): void { + // EXCLUDED must stay unquoted, so reference it with a raw Fragment (not an + // Expression, which would quote it — tolerated by SQLite but broken on Postgres). $q = $this->database->insert('counters') ->values(['key' => 'x', 'n' => 1]) ->onConflict(OnConflict::target('key')->doUpdate([ - 'n' => new Expression('counters.n + EXCLUDED.n'), + 'n' => new Fragment('counters.n + EXCLUDED.n'), ])); $this->assertSameQuery( 'INSERT INTO {counters} ({key}, {n}) VALUES (?, ?) ' - . 'ON CONFLICT ({key}) DO UPDATE SET {n} = {counters}.{n} + {EXCLUDED}.{n}', + . 'ON CONFLICT ({key}) DO UPDATE SET {n} = counters.n + EXCLUDED.n', $q, ); } @@ -79,8 +85,107 @@ public function testMultiRowUpsert(): void $this->assertSameQuery( 'INSERT INTO {users} ({email}, {name}) VALUES (?, ?), (?, ?) ' - . 'ON CONFLICT ({email}) DO UPDATE SET {name} = {EXCLUDED}.{name}', + . 'ON CONFLICT ({email}) DO UPDATE SET {name} = EXCLUDED.{name}', $q, ); } + + public function testTargetWherePartialIndex(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'b']) + ->onConflict( + SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['body']), + ); + + $this->assertSameQuery( + 'INSERT INTO {routes} ({resource_key}, {route}, {body}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE resource_key IS NOT NULL ' + . 'DO UPDATE SET {body} = EXCLUDED.{body}', + $q, + ); + } + + public function testTargetWhereClosureBuilder(): void + { + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'b']) + ->onConflict( + SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere(static fn(OnConflictWhere $w) => $w->where('resource_key', '!=', null)) + ->doUpdate(['body']), + ); + + $this->assertSameQuery( + 'INSERT INTO {routes} ({resource_key}, {route}, {body}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE {resource_key} IS NOT NULL ' + . 'DO UPDATE SET {body} = EXCLUDED.{body}', + $q, + ); + } + + public function testTargetWhereParameterOrder(): void + { + // Values, then the predicate parameter, then the DO UPDATE parameter — the + // predicate is rendered between the conflict target and DO UPDATE, so its + // parameter must bind before the update parameter (also on the cached path). + $q = $this->database->insert('routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'priority' => 1]) + ->onConflict( + SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere(static fn(OnConflictWhere $w) => $w->where('priority', '>', 5)) + ->doUpdate(['priority' => new Expression('routes.priority + ?', 10)]), + ); + + $this->assertSameQueryWithParameters( + 'INSERT INTO {routes} ({resource_key}, {route}, {priority}) VALUES (?, ?, ?) ' + . 'ON CONFLICT ({resource_key}, {route}) WHERE {priority} > ? ' + . 'DO UPDATE SET {priority} = {routes}.{priority} + ?', + ['k', '/r', 1, 5, 10], + $q, + ); + } + + public function testTargetWherePartialIndexRuntime(): void + { + $this->makePartialIndexTable(); + + $this->database->insert('upsert_routes') + ->values(['resource_key' => 'k', 'route' => '/r', 'body' => 'old'])->run(); + + $upsert = fn(string $key, string $body) => $this->database->insert('upsert_routes') + ->values(['resource_key' => $key, 'route' => '/r', 'body' => $body]) + ->onConflict( + SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['body']), + )->run(); + + // Matching partial index → conflict → update in place. + $upsert('k', 'new'); + $rows = $this->database->select()->from('upsert_routes')->fetchAll(); + $this->assertCount(1, $rows); + $this->assertSame('new', $rows[0]['body']); + + // Different key → no conflict → new row inserted. + $upsert('k2', 'second'); + $this->assertCount(2, $this->database->select()->from('upsert_routes')->fetchAll()); + } + + private function makePartialIndexTable(): void + { + $schema = $this->schema('upsert_routes'); + $schema->primary('id'); + $schema->string('resource_key')->nullable(true); + $schema->string('route'); + $schema->string('body'); + $schema->save(Handler::DO_ALL); + + $this->database->execute( + 'CREATE UNIQUE INDEX upsert_routes_partial ON upsert_routes (resource_key, route) ' + . 'WHERE resource_key IS NOT NULL', + ); + } } diff --git a/tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php b/tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php index 07d95445..7e681644 100644 --- a/tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php +++ b/tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php @@ -6,9 +6,13 @@ use Cycle\Database\Driver\MySQL\MySQLOnConflict; use Cycle\Database\Driver\Postgres\PostgresOnConflict; +use Cycle\Database\Driver\SQLite\SQLiteOnConflict; use Cycle\Database\Exception\BuilderException; +use Cycle\Database\Injection\Expression; +use Cycle\Database\Injection\Fragment; use Cycle\Database\Query\ConflictAction; use Cycle\Database\Query\OnConflict; +use Cycle\Database\Query\OnConflictWhere; use Cycle\Database\Query\QueryParameters; use PHPUnit\Framework\TestCase; @@ -80,4 +84,91 @@ public function testCacheKeyIncludesConstraint(): void $b->getCacheKey(new QueryParameters()), ); } + + public function testTargetWhereWrapsString(): void + { + $c = PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL'); + + $tokens = $c->getIndexPredicate(); + $this->assertCount(1, $tokens); + $this->assertSame('AND', $tokens[0][0]); + $this->assertInstanceOf(Fragment::class, $tokens[0][1]); + $this->assertSame('resource_key IS NOT NULL', (string) $tokens[0][1]); + } + + public function testTargetWhereAcceptsFragmentInterface(): void + { + // Expression is a FragmentInterface (not a Fragment) — it is stored as-is. + $expr = new Expression('resource_key IS NOT NULL'); + $c = PostgresOnConflict::target('resource_key')->targetWhere($expr); + + $this->assertSame($expr, $c->getIndexPredicate()[0][1]); + } + + public function testTargetWhereAcceptsVariadicCondition(): void + { + $c = PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('priority', '>', 5); + + $tokens = $c->getIndexPredicate(); + $this->assertNotSame([], $tokens); + $this->assertSame('priority', $tokens[0][1][0]); + $this->assertSame('>', $tokens[0][1][1]); + } + + public function testTargetWhereAcceptsArrayForm(): void + { + $c = PostgresOnConflict::target('resource_key') + ->targetWhere(['resource_key' => ['!=' => null]]); + + $this->assertNotSame([], $c->getIndexPredicate()); + } + + public function testTargetWhereAcceptsClosureBuilder(): void + { + $c = PostgresOnConflict::target('resource_key', 'route') + ->targetWhere(static fn(OnConflictWhere $w) => $w->where('resource_key', '!=', null)); + + $tokens = $c->getIndexPredicate(); + $this->assertNotSame([], $tokens); + $this->assertSame('resource_key', $tokens[0][1][0]); + $this->assertSame('!=', $tokens[0][1][1]); + } + + public function testTargetWhereNoArgsRejected(): void + { + $this->expectException(BuilderException::class); + PostgresOnConflict::target('resource_key')->targetWhere(); + } + + public function testTargetWhereEmptyStringRejected(): void + { + $this->expectException(BuilderException::class); + PostgresOnConflict::target('resource_key')->targetWhere(''); + } + + public function testTargetWhereIsImmutable(): void + { + $base = PostgresOnConflict::target('resource_key'); + $withWhere = $base->targetWhere('resource_key IS NOT NULL'); + + $this->assertSame([], $base->getIndexPredicate()); + $this->assertNotSame($base, $withWhere); + } + + public function testFromSqliteSiblingCarriesPredicate(): void + { + $sqlite = SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['route']); + + $pg = PostgresOnConflict::from($sqlite); + + $this->assertInstanceOf(PostgresOnConflict::class, $pg); + $this->assertSame(['resource_key', 'route'], $pg->getTarget()); + $this->assertSame(['route'], $pg->getUpdate()); + $this->assertSame('resource_key IS NOT NULL', (string) $pg->getIndexPredicate()[0][1]); + $this->assertNull($pg->getConstraint()); + } } diff --git a/tests/Database/Unit/Driver/SQLite/SQLiteOnConflictTest.php b/tests/Database/Unit/Driver/SQLite/SQLiteOnConflictTest.php new file mode 100644 index 00000000..650f052a --- /dev/null +++ b/tests/Database/Unit/Driver/SQLite/SQLiteOnConflictTest.php @@ -0,0 +1,90 @@ +assertInstanceOf(SQLiteOnConflict::class, $c); + } + + public function testTargetWhereWrapsString(): void + { + $c = SQLiteOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL'); + + $tokens = $c->getIndexPredicate(); + $this->assertCount(1, $tokens); + $this->assertInstanceOf(Fragment::class, $tokens[0][1]); + $this->assertSame('resource_key IS NOT NULL', (string) $tokens[0][1]); + } + + public function testTargetWhereAcceptsClosureBuilder(): void + { + $c = SQLiteOnConflict::target('resource_key') + ->targetWhere(static fn(OnConflictWhere $w) => $w->where('resource_key', '!=', null)); + + $this->assertNotSame([], $c->getIndexPredicate()); + } + + public function testFromBase(): void + { + $base = OnConflict::target('email')->doUpdate(['name']); + $sqlite = SQLiteOnConflict::from($base); + + $this->assertInstanceOf(SQLiteOnConflict::class, $sqlite); + $this->assertSame(['email'], $sqlite->getTarget()); + $this->assertSame(['name'], $sqlite->getUpdate()); + $this->assertSame([], $sqlite->getIndexPredicate()); + } + + public function testFromSelfReturnsSameInstance(): void + { + $sqlite = SQLiteOnConflict::target('email'); + + $this->assertSame($sqlite, SQLiteOnConflict::from($sqlite)); + } + + public function testFromPostgresSiblingCarriesPredicate(): void + { + $pg = PostgresOnConflict::target('resource_key', 'route') + ->targetWhere('resource_key IS NOT NULL') + ->doUpdate(['route']); + + $sqlite = SQLiteOnConflict::from($pg); + + $this->assertInstanceOf(SQLiteOnConflict::class, $sqlite); + $this->assertSame(['resource_key', 'route'], $sqlite->getTarget()); + $this->assertSame('resource_key IS NOT NULL', (string) $sqlite->getIndexPredicate()[0][1]); + } + + public function testFromPostgresConstraintRejected(): void + { + $pg = PostgresOnConflict::onConstraint('users_email_unique')->doUpdate(['name']); + + $this->expectException(BuilderException::class); + SQLiteOnConflict::from($pg); + } + + public function testFromOtherDriverSubclassRejected(): void + { + $mysql = MySQLOnConflict::target('email'); + + $this->expectException(BuilderException::class); + SQLiteOnConflict::from($mysql); + } +} diff --git a/tests/Database/Unit/Query/OnConflictTest.php b/tests/Database/Unit/Query/OnConflictTest.php index 14460365..bda7e7a7 100644 --- a/tests/Database/Unit/Query/OnConflictTest.php +++ b/tests/Database/Unit/Query/OnConflictTest.php @@ -6,6 +6,7 @@ use Cycle\Database\Exception\BuilderException; use Cycle\Database\Injection\Expression; +use Cycle\Database\Injection\Fragment; use Cycle\Database\Injection\Parameter; use Cycle\Database\Query\ConflictAction; use Cycle\Database\Query\OnConflict; @@ -117,7 +118,8 @@ public function testDoUpdateColumnList(): void public function testDoUpdateColumnMap(): void { - $expr = new Expression('counters.n + EXCLUDED.n'); + // EXCLUDED references use a raw Fragment (Expression would quote the keyword). + $expr = new Fragment('counters.n + EXCLUDED.n'); $c = OnConflict::target('key')->doUpdate(['n' => $expr]); $this->assertSame(['n' => $expr], $c->getUpdate());