Skip to content

feat(OnConflict): index-inference predicate for ON CONFLICT (Postgres/SQLite)#258

Merged
roxblnfk merged 2 commits into
2.xfrom
feature/upsert-on-conflict-predicate
Jun 24, 2026
Merged

feat(OnConflict): index-inference predicate for ON CONFLICT (Postgres/SQLite)#258
roxblnfk merged 2 commits into
2.xfrom
feature/upsert-on-conflict-predicate

Conversation

@roxblnfk

@roxblnfk roxblnfk commented Jun 23, 2026

Copy link
Copy Markdown
Member

🔍 What was changed

Adds support for the index-inference predicate on the conflict target — ON CONFLICT (cols) WHERE <predicate> — used to match a partial unique index. New method: OnConflict::targetWhere().

Usage

Given a partial unique index, e.g. CREATE UNIQUE INDEX ... ON routes (resource_key, route) WHERE resource_key IS NOT NULL:

use Cycle\Database\Driver\Postgres\PostgresOnConflict;

$db->insert('routes')
    ->values(['resource_key' => $key, 'route' => $route, 'body' => $body])
    ->onConflict(
        PostgresOnConflict::target('resource_key', 'route')
            ->targetWhere('resource_key IS NOT NULL')
            ->doUpdate(['body'])
    )
    ->run();
// INSERT INTO routes (...) VALUES (...)
// ON CONFLICT (resource_key, route) WHERE resource_key IS NOT NULL
// DO UPDATE SET body = EXCLUDED.body

targetWhere() accepts the same argument shapes as where(), so simple predicates need no closure:

use Cycle\Database\Query\OnConflictWhere;
use Cycle\Database\Injection\Fragment;

->targetWhere('resource_key IS NOT NULL')          // raw SQL, emitted verbatim
->targetWhere('priority', '>', 5)                  // condition: identifier quoted, value bound
->targetWhere('resource_key', '!=', null)          // null-aware -> "resource_key" IS NOT NULL
->targetWhere(['priority' => ['>' => 5]])          // array form
->targetWhere(fn(OnConflictWhere $w) =>            // closure builder for AND/OR groups, BETWEEN, IN
    $w->where('priority', '>', 5)->orWhere('resource_key', '!=', null))
->targetWhere(new Fragment('resource_key IS NOT NULL'))

The same works on SQLite via SQLiteOnConflict. A base OnConflict (or a SQLiteOnConflict) is converted to PostgresOnConflict and back without error, so portable code keeps working; MySQL / SQL Server have no equivalent clause, so the method is simply absent there (rejected at the type level, not at runtime).

Bugs fixed

These are pre-existing upsert bugs that the new runtime tests caught — both broke valid usage at execution time while passing the compile-time string assertions:

  • Postgres — every DO UPDATE upsert was broken at runtime. The generated SET col = "EXCLUDED".col quoted the EXCLUDED keyword; Postgres rejects it with missing FROM-clause entry for table "EXCLUDED". It is now emitted unquoted.
  • MySQL — DO NOTHING was broken at runtime. It emitted the AS new_row alias, making the no-op col = col ambiguous (1052 Column ... is ambiguous). The alias is now omitted on the DO NOTHING branch (it is only needed for col = new_row.col in DO UPDATE).

🤔 Why?

Partial unique indexes ("unique per key, but only when the key is set") are common, and Postgres/SQLite require the ON CONFLICT inference predicate to match such an index. Previously there was no way to express this through the query builder — you had to drop to raw SQL. SQLite copies the Postgres syntax, so the two share the implementation; MySQL and SQL Server have no equivalent, which is why the feature is a driver-specific subclass rather than a flag on the base DTO.

The two fixes are not cosmetic: the new execution tests fail without them, meaning the existing upsert paths were silently broken at runtime (compile-time tests only compared SQL strings and never ran them).

📝 Checklist

  • Follow-up to the InsertQuery::onConflict() upsert feature — no dedicated issue; please link one if applicable.
  • How was this tested:
    • Unit tests added — DTO semantics, every targetWhere() argument shape, immutability, from() conversion/rejection across drivers.
    • Compile-time SQL assertions per driver (Common, Postgres, SQLite, MySQL, SQL Server).
    • Runtime (execution) tests against live databases — cross-driver DO UPDATE / DO NOTHING / selective columns / insert-on-no-conflict; Postgres + SQLite partial-index predicate against a real CREATE UNIQUE INDEX ... WHERE ... (verifying update-vs-insert); Postgres EXCLUDED-via-Fragment counter increment.

📃 Documentation

OnConflict::doUpdate() PHPDoc now documents how to reference the inserted row inside a custom update expression: use a raw Fragment, not an Expression.

use Cycle\Database\Injection\Fragment;

// Postgres / SQLite
->doUpdate(['hits' => new Fragment('counters.hits + EXCLUDED.hits')])
// MySQL
->doUpdate(['hits' => new Fragment('counters.hits + new_row.hits')])

Expression quotes every identifier and Postgres rejects the quoted "EXCLUDED"; a Fragment is emitted verbatim. Use Expression only for expressions over real columns (which should be quoted), not for the excluded-row reference.

🤖 Generated with Claude Code

…/SQLite)

Add `OnConflict::targetWhere()` to express the `ON CONFLICT (cols) WHERE <predicate>`
index-inference form used to match a partial unique index. Supported only on the
drivers that inherit the Postgres inference clause (Postgres, SQLite); MySQL and
SQL Server reject it at the type level.

- New `OnConflictWithPredicate` middle class in the DTO tree holds the predicate;
  `PostgresOnConflict` and `SQLiteOnConflict` extend it. `from()` converts between
  these feature-compatible siblings without error (a Postgres constraint target
  cannot narrow to SQLite — the one rejection).
- `targetWhere()` accepts the same shapes as `where()` (variadic `('col','op',val)`,
  array form, FragmentInterface, Closure builder via `OnConflictWhere`) plus a raw
  string shorthand. Closures reuse TokenTrait/WhereTrait, so the full DSL works
  (null -> IS NOT NULL, AND/OR groups, BETWEEN, IN).
- Compilers render the predicate through the regular where-renderer via a new
  `Compiler::conflictTarget()` hook; the query cache hashes it through `hashWhere()`
  so predicate parameters bind before the DO UPDATE parameters.

Fixes found via the new runtime tests (pre-existing upsert bugs):
- Postgres: the auto-generated `DO UPDATE SET col = "EXCLUDED".col` quoted the
  EXCLUDED keyword, which Postgres rejects ("missing FROM-clause entry"). Emit it
  unquoted.
- MySQL: `DO NOTHING` emitted the `AS new_row` alias, making the no-op `col = col`
  ambiguous (error 1052). Omit the alias on the DO NOTHING branch.

Document that EXCLUDED/new_row references inside custom update expressions must use
a raw Fragment (not Expression, which quotes the keyword and breaks on Postgres).

Tests: unit coverage for the DTO/builder/conversions; per-driver compile-time
assertions; cross-driver runtime (execution) tests for DO UPDATE / DO NOTHING /
selective columns / insert-on-no-conflict; Postgres/SQLite runtime tests for the
partial-index predicate and the Fragment EXCLUDED path.
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.22222% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.69%. Comparing base (33cf5d5) to head (4110dbd).
⚠️ Report is 2 commits behind head on 2.x.

Files with missing lines Patch % Lines
src/Driver/Compiler.php 60.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x     #258      +/-   ##
============================================
+ Coverage     95.68%   95.69%   +0.01%     
- Complexity     2038     2067      +29     
============================================
  Files           137      140       +3     
  Lines          5775     5835      +60     
============================================
+ Hits           5526     5584      +58     
- Misses          249      251       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for Postgres/SQLite ON CONFLICT (cols) WHERE <predicate> (index-inference predicate) via a new targetWhere() API, while also fixing previously broken runtime upsert SQL generation for Postgres (EXCLUDED quoting) and MySQL (DO NOTHING alias ambiguity). The PR also expands test coverage with new unit tests and new live-runtime execution tests to validate actual database behavior.

Changes:

  • Introduce OnConflictWithPredicate + OnConflictWhere and driver-specific support (PostgresOnConflict, new SQLiteOnConflict) to compile an index-inference predicate on the conflict target.
  • Fix upsert SQL generation bugs: unquote EXCLUDED for Postgres/SQLite DO UPDATE, and omit MySQL row alias for DO NOTHING.
  • Add/extend unit + functional tests, including runtime execution tests against live databases and partial-index predicate coverage.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Database/Unit/Query/OnConflictTest.php Updates unit test to use Fragment for EXCLUDED references in custom update expressions.
tests/Database/Unit/Driver/SQLite/SQLiteOnConflictTest.php New unit tests covering SQLiteOnConflict behavior and from() conversions/rejections.
tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php Adds unit coverage for targetWhere() shapes, immutability, and sibling conversion with SQLite.
tests/Database/Functional/Driver/SQLite/Query/UpsertQueryTest.php Updates expected SQL (EXCLUDED unquoted), adds predicate compilation + runtime partial-index tests.
tests/Database/Functional/Driver/Postgres/Query/UpsertQueryTest.php Updates expected SQL (EXCLUDED unquoted), adds predicate compilation + runtime partial-index and fragment runtime tests.
tests/Database/Functional/Driver/MySQL/Query/UpsertQueryTest.php Updates expected SQL to remove row alias on DO NOTHING emulation.
tests/Database/Functional/Driver/Common/Query/UpsertQueryTest.php Adds cross-driver runtime upsert execution tests (update, selective columns, do-nothing, insert-on-no-conflict).
src/Query/OnConflictWithPredicate.php New shared base for Postgres/SQLite to store and build index-inference predicates via targetWhere().
src/Query/OnConflictWhere.php New lightweight WHERE-token builder for targetWhere() closure/DSL support.
src/Query/OnConflict.php Documentation updates describing driver-specific subclasses and Fragment vs Expression guidance for excluded-row references.
src/Driver/SQLite/SQLiteOnConflict.php New SQLite-specific OnConflict subclass supporting index-inference predicates and safe narrowing rules.
src/Driver/SQLite/SQLiteCompiler.php Overrides conflictTarget() to emit (cols) WHERE <predicate> for SQLite when present.
src/Driver/Postgres/PostgresOnConflict.php Extends the new predicate-enabled base and supports narrowing from SQLite/base while carrying predicates.
src/Driver/Postgres/PostgresCompiler.php Compiles conflict target with optional predicate; rejects predicate + constraint combination.
src/Driver/MySQL/MySQLCompiler.php Fixes DO NOTHING emulation by omitting row alias; still emits alias for DO UPDATE.
src/Driver/CompilerCache.php Includes predicate hashing (and parameter ordering) into the upsert cache key path.
src/Driver/Compiler.php Introduces conflictTarget() hook; fixes EXCLUDED quoting in upsert update clause generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Query/OnConflictWithPredicate.php Outdated
Comment thread src/Query/OnConflictWithPredicate.php Outdated
Comment thread tests/Database/Unit/Driver/Postgres/PostgresOnConflictTest.php Outdated
- Drop the docblock-only `FragmentInterface` import; reference it via FQN in the `targetWhere()` PHPDoc (consistent with the Expression reference on the same line).
- Rename `testTargetWhereAcceptsFragment` to `testTargetWhereAcceptsFragmentInterface`: it passes an Expression, which is a FragmentInterface but not a Fragment.
@roxblnfk roxblnfk merged commit bdd2cea into 2.x Jun 24, 2026
30 of 31 checks passed
@roxblnfk roxblnfk deleted the feature/upsert-on-conflict-predicate branch June 24, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants