diff --git a/.claude/plans/query-builder-raw-select-where/001-interface-declarations.md b/.claude/plans/query-builder-raw-select-where/001-interface-declarations.md new file mode 100644 index 00000000..5af2e4d2 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/001-interface-declarations.md @@ -0,0 +1,136 @@ +# Task 001: QueryBuilderInterface — Add `selectRaw` and `whereRaw` Declarations + Update All Stubs + +**Status**: completed +**Depends on**: none +**Retry count**: 0 + +## Description +Add `selectRaw` and `whereRaw` method declarations (signatures + PHPDoc) to `Marko\Database\Query\QueryBuilderInterface`. **Critical**: update every anonymous-class stub of `QueryBuilderInterface` (and the one `EntityQueryBuilderInterface` stub) in `packages/database/tests/` with no-op implementations so the test suite still loads after the interface changes. Add reflection-based contract tests in `QueryBuilderInterfaceTest.php` asserting the new methods are declared with the expected signatures. + +This task introduces the contract only. Behavior implementations land in tasks 002 (MySql), 003 (PgSql), and 004 (Repository wrapper). Behavior tests for the denylist live in tasks 002 and 003 against real driver implementations — NOT in this task, because `QueryBuilderInterfaceTest.php` is reflection-only (no inline stub-impl pattern exists in the file today). + +## Context + +### Files to edit + +**Interface itself:** +- `packages/database/src/Query/QueryBuilderInterface.php` (read first — the file ends at the `raw()` method declaration around line 376; the interface imports `Marko\Database\Exceptions\InvalidColumnException` at line 7 — no new imports needed). + +**Interface contract tests:** +- `packages/database/tests/Query/QueryBuilderInterfaceTest.php` (read first — every existing test uses `new ReflectionClass(QueryBuilderInterface::class)` and reflection assertions. NO stub-impl pattern is used. Mirror that style exactly). + +**Anonymous-class stubs that implement `QueryBuilderInterface` (every one MUST be updated with no-op `selectRaw` and `whereRaw` methods that return `$this`):** +- `packages/database/tests/Repository/RepositoryTest.php` (line ~1091) +- `packages/database/tests/Repository/RepositoryMatchingTest.php` (line ~50) +- `packages/database/tests/Repository/StringPrimaryKeyTest.php` (line ~153) +- `packages/database/tests/Repository/RepositoryWithTest.php` (lines ~104, ~551, ~800, ~1039 — four separate stubs in this one file) +- `packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php` (line ~78 — `makeRqbStubBuilder`) +- `packages/database/tests/Entity/RelationshipLoaderNestedTest.php` (line ~345) +- `packages/database/tests/Entity/RelationshipLoaderTest.php` (lines ~246, ~482) +- `packages/database/tests/Entity/RelationshipLoaderBelongsToManyTest.php` (lines ~200, ~448) +- `packages/database/tests/Entity/RelationshipValidationTest.php` (line ~49) +- `packages/database/tests/Query/QuerySpecificationTest.php` (lines ~37, ~255) +- `packages/database/tests/Query/SpecEagerLoadCompositionTest.php` (lines ~107, ~293) — plus an `EntityQueryBuilderInterface` stub at line ~565 that also needs both methods + +Use `grep -rn 'implements QueryBuilderInterface' packages/database/tests/` to verify the full list is covered before submitting. Any missed stub causes a PHP fatal error at test load time (`Class contains 2 abstract methods and must therefore be declared abstract`). + +### Method signatures + +```php +public function selectRaw( + string $expression, + array $bindings = [], +): static; + +public function whereRaw( + string $expression, + array $bindings = [], +): static; +``` + +### PHPDoc verbatim + +```php +/** + * Add a raw SQL expression to the SELECT list. + * + * The expression is appended after any columns added via select(). Include + * "AS alias" in the expression if you need a column alias. If no select() + * call precedes this, the default '*' column is preserved (emitting + * `SELECT *, `). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * Note: aggregate methods (count, min, max, sum, avg) build their own + * SELECT list and ignore selectRaw additions. + * + * @param string $expression Raw SQL select expression (e.g. "COALESCE(a, b) AS resolved") + * @param array $bindings Positional bindings for ? placeholders in the expression + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ +public function selectRaw( + string $expression, + array $bindings = [], +): static; + +/** + * Add a raw SQL WHERE condition with optional positional bindings. + * + * The expression is AND-combined with any other where conditions, in call + * order, after the regular where*/whereIn/whereNull/etc. conditions. + * + * Aggregate methods (count, min, max, sum, avg) honor whereRaw conditions + * the same way they honor where(). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * @param string $expression Raw SQL WHERE expression (e.g. "COALESCE(price, base) > ?") + * @param array $bindings Positional bindings for ? placeholders + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ +public function whereRaw( + string $expression, + array $bindings = [], +): static; +``` + +### Declaration ordering in `QueryBuilderInterface.php` + +- Place `selectRaw` immediately after `select()` (around line 31, before `distinct()`). This groups SELECT-shaping methods together at the top of the interface. +- Place `whereRaw` immediately after `orWhere` (after line 134). This groups WHERE-shaping methods together. + +### No `EntityQueryBuilderInterface` change required +`EntityQueryBuilderInterface extends QueryBuilderInterface` (verified — see `packages/database/src/Query/EntityQueryBuilderInterface.php`). Additions to the parent propagate automatically. But the stub implementation of `EntityQueryBuilderInterface` in `SpecEagerLoadCompositionTest.php` (line ~565) DOES need updating — see the file list above. + +## Requirements (Test Descriptions) + +### Reflection-based interface assertions (in `QueryBuilderInterfaceTest.php`, matching existing reflection-only style) +- [ ] `QueryBuilderInterface declares a selectRaw method` +- [ ] `QueryBuilderInterface::selectRaw takes a string expression as the first parameter named "expression"` +- [ ] `QueryBuilderInterface::selectRaw takes an array bindings as the second parameter named "bindings" with default []` +- [ ] `QueryBuilderInterface::selectRaw returns static` +- [ ] `QueryBuilderInterface declares a whereRaw method` +- [ ] `QueryBuilderInterface::whereRaw takes a string expression as the first parameter named "expression"` +- [ ] `QueryBuilderInterface::whereRaw takes an array bindings as the second parameter named "bindings" with default []` +- [ ] `QueryBuilderInterface::whereRaw returns static` + +(Denylist behavior tests live in tasks 002 and 003 against real drivers — NOT here.) + +### Stub-cascade smoke test +- [ ] `composer test for packages/database is fully green after the interface change` (this requirement alone catches any missed stub — a PHP fatal at load time would block all tests) + +## Acceptance Criteria +- Both methods declared on the interface with the exact signatures and PHPDoc from above. +- Every stub listed in the Context section has no-op `selectRaw(string $expression, array $bindings = []): static { return $this; }` and `whereRaw(string $expression, array $bindings = []): static { return $this; }` methods. (For the `EntityQueryBuilderInterface` stub in `SpecEagerLoadCompositionTest.php`, both methods plus inheritance through the parent.) +- The reflection tests in `QueryBuilderInterfaceTest.php` pass. +- `composer test` for `packages/database` is fully green (no PHP fatals from missed stubs). +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +(Left blank — filled in by programmer during implementation) diff --git a/.claude/plans/query-builder-raw-select-where/002-mysql-implementation.md b/.claude/plans/query-builder-raw-select-where/002-mysql-implementation.md new file mode 100644 index 00000000..69fcb93c --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/002-mysql-implementation.md @@ -0,0 +1,187 @@ +# Task 002: MySqlQueryBuilder — Implement `selectRaw` and `whereRaw` + +**Status**: completed +**Depends on**: 001 +**Retry count**: 0 + +## Description +Implement `selectRaw` and `whereRaw` in `MySqlQueryBuilder`. Both methods append to internal state (raw-select expressions list, raw-where expressions list with their bindings), and the SQL-compilation step weaves them into the emitted SQL with correct binding order. A NEW private denylist helper rejects `;`, `--`, `/*`, `*/`, and backticks. (There is no pre-existing `orderByRaw` to refactor — that method does not exist in the codebase. The new helper is genuinely new.) + +The aggregate code path (`runAggregate`, used by `count`/`min`/`max`/`sum`/`avg`) must also honor `whereRaw` conditions, since aggregates build their own SELECT and bypass `buildSelectSql()`. `selectRaw` is documented as ignored by aggregates (their SELECT list is fully determined by the aggregate function). + +## Context + +### Files to edit +- `packages/database-mysql/src/Query/MySqlQueryBuilder.php` (read first — 881 lines, `class MySqlQueryBuilder implements QueryBuilderInterface`). +- `packages/database-mysql/tests/Query/MySqlQueryBuilderTest.php` (read first — uses SQLite-backed PDO via `createPdo()` override for real query-result assertions). + +### Verified file landmarks (read these before coding) +- `having()` at line 269 — uses inline denylist `;`, `--`, `/*`, `*/` (no backtick). DO NOT touch this method. +- `orderBy()` at line 341 — followed directly by `limit()` at 358. There is NO `orderByRaw`. The plan's earlier draft incorrectly claimed otherwise. +- `get()` at line 374 — resets `$this->bindings = []` at line 380, THEN calls `buildSelectSql()`. This is important for the `selectRaw` bindings insertion site (see below). +- `runAggregate()` at line 533 — resets `$this->bindings = []`, builds its own SELECT, then concatenates `buildWhereClause()`. This is the path that must be updated to honor `whereRaw`. +- `buildSelectSql()` at line 665 — does NOT reset `$this->bindings`. Calls `buildJoinClause()`, `buildWhereClause()`, `buildGroupByClause()`, `buildHavingClause()`, `buildOrderByClause()`, `buildLimitOffsetClause()` in that order. +- `buildWhereClause()` at line 740 — iterates `$this->wheres`, `$this->whereIns`, `$this->whereNulls`, `$this->whereNotNulls`, `$this->whereJsonContains`, `$this->whereJsonPaths` and appends to `$this->bindings`. THIS is where the new `$this->rawWheres` iteration must be added (in call order, after the other where types, AND-combined with the running condition list). +- `compileSubquery()` at line 151 — saves bindings, resets, calls `buildSelectSql()`, captures bindings, merges into the outer bindings, restores. `selectRaw`/`whereRaw` bindings flow through this correctly as long as `buildSelectSql()`/`buildWhereClause()` handle them. + +### New state fields on the builder + +```php +/** @var list */ +private array $rawSelects = []; + +/** @var list */ +private array $rawSelectBindings = []; + +/** @var list */ +private array $rawWheres = []; +``` + +### Method implementations + +```php +public function selectRaw(string $expression, array $bindings = []): static +{ + $this->assertNoDangerousPatterns($expression); + $this->rawSelects[] = $expression; + array_push($this->rawSelectBindings, ...$bindings); + return $this; +} + +public function whereRaw(string $expression, array $bindings = []): static +{ + $this->assertNoDangerousPatterns($expression); + $this->rawWheres[] = ['expression' => $expression, 'bindings' => $bindings]; + return $this; +} +``` + +### New private denylist helper + +```php +/** + * @throws InvalidColumnException + */ +private function assertNoDangerousPatterns(string $expression): void +{ + if ( + str_contains($expression, ';') + || str_contains($expression, '--') + || str_contains($expression, '/*') + || str_contains($expression, '*/') + || str_contains($expression, '`') + ) { + throw InvalidColumnException::invalidColumn($expression); + } +} +``` + +(This is a NEW private method. There is no pre-existing inline denylist outside of `having()` to refactor. `having()` is intentionally left alone — see `_plan.md` Risks & Mitigations.) + +### SELECT compilation — exact insertion site (MySql-specific) + +`MySqlQueryBuilder::buildSelectSql()` (line 665) does NOT reset `$this->bindings`. The caller (`get()` at line 380, `compileSubquery()` at 154–155) handles the reset. Therefore: + +- At the top of `buildSelectSql()` (after the `$quotedColumns` array is built but BEFORE `$sql .= $this->buildWhereClause()`), append `$this->rawSelects` items to the SELECT-list rendering. +- ALSO at the top of `buildSelectSql()` (before `buildWhereClause()` runs), append `$this->rawSelectBindings` to `$this->bindings`. This places SELECT bindings before WHERE bindings in the final array. + +Concretely the rendering changes around line 667–682: + +```php +$quotedColumns = array_map(function (string $col): string { + if ($col === '*') { + return '*'; + } + return $this->compileColumnExpression($col); +}, $this->columns); + +// NEW: append raw select expressions verbatim +$selectParts = array_merge($quotedColumns, $this->rawSelects); + +// NEW: append raw-select bindings to the bindings stream BEFORE WHERE compiles +foreach ($this->rawSelectBindings as $binding) { + $this->bindings[] = $binding; +} + +$keyword = $this->distinct ? 'SELECT DISTINCT' : 'SELECT'; + +$sql = sprintf( + '%s %s FROM %s', + $keyword, + implode(', ', $selectParts), + $this->quoteIdentifier($this->table), +); +``` + +### WHERE compilation — extend `buildWhereClause()` + +After the existing `whereJsonPaths` loop (around line 820) and BEFORE the empty-conditions early-return, add a new loop iterating `$this->rawWheres`. Each item appends `$item['expression']` as a condition (prefixed with `AND ` if `$conditions` is non-empty) and appends `$item['bindings']` to `$this->bindings`. + +### Aggregate path — extend `runAggregate()` + +`runAggregate()` (line 533) currently does: +```php +$this->bindings = []; +$sql = sprintf('SELECT %s FROM %s', $aggregateExpr, $this->quoteIdentifier($this->table)); +$sql .= $this->buildWhereClause(); +``` + +Since `buildWhereClause()` is the shared method, and the change above adds `rawWheres` handling INSIDE `buildWhereClause()`, aggregates automatically honor `whereRaw`. **Verify** that no extra change is needed in `runAggregate()` itself — but add an explicit test (see below) to lock this in. (`selectRaw` is correctly ignored because `runAggregate()` builds its own SELECT and never reads `$this->rawSelects`.) + +### Repeated `get()` calls must produce identical bindings +Critical: `$this->rawSelectBindings` and `$this->rawWheres` MUST NOT be mutated during compile. The compile step reads them and appends to `$this->bindings` (which IS reset between calls). Re-running `->get()` on the same builder twice must produce identical SQL and bindings. + +## Requirements (Test Descriptions) + +### selectRaw — SELECT compilation +- [ ] `selectRaw appends the expression to the SELECT list after regular columns` +- [ ] `selectRaw alone (no prior select call) emits "SELECT *, " preserving the default *` +- [ ] `selectRaw can be called multiple times; expressions appear in call order in the SELECT list` +- [ ] `selectRaw together with select() emits select() columns first then selectRaw expressions` + +### selectRaw — bindings position +- [ ] `selectRaw with bindings places the bindings BEFORE WHERE bindings in the compiled bindings array (assert against a captured stub-connection bindings array)` +- [ ] `selectRaw bindings from multiple calls concatenate in call order` +- [ ] `running ->get() twice on the same builder produces identical SQL and bindings (no mutation of internal raw state)` + +### selectRaw — denylist +- [ ] `selectRaw throws InvalidColumnException when the expression contains a semicolon` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a -- comment marker` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a backtick` +- [ ] `selectRaw returns the builder for fluent chaining` + +### whereRaw — WHERE compilation +- [ ] `whereRaw appends the expression to the WHERE clause AND-combined with other conditions` +- [ ] `whereRaw used alone (no prior where) emits "WHERE " with no leading AND` +- [ ] `whereRaw can be called multiple times; expressions appear in call order, AND-combined` +- [ ] `whereRaw together with where() emits the regular where condition first then the raw expression, AND-combined` + +### whereRaw — bindings position +- [ ] `whereRaw with bindings places its bindings in the WHERE position of the bindings array (after selectRaw bindings, after regular where bindings if both exist)` +- [ ] `whereRaw bindings from multiple calls concatenate in call order` + +### whereRaw — aggregate path +- [ ] `count() honors whereRaw conditions (filtered count, not full-table count)` +- [ ] `min() / max() / sum() / avg() honor whereRaw conditions` + +### whereRaw — denylist +- [ ] `whereRaw throws InvalidColumnException when the expression contains a semicolon` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a -- comment marker` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a backtick` +- [ ] `whereRaw returns the builder for fluent chaining` + +### Combined selectRaw + whereRaw +- [ ] `selectRaw and whereRaw used together produce bindings in [select-bindings..., where-bindings...] order in the final bindings array` +- [ ] `selectRaw and whereRaw flow correctly through compileSubquery() when this builder is used as a UNION right-hand side` + +## Acceptance Criteria +- All requirements have passing tests. +- The new private `assertNoDangerousPatterns()` helper is referenced by `selectRaw()` and `whereRaw()` only — `having()` is NOT modified. +- Bindings inspection in tests uses a stub `ConnectionInterface` that records the `(sql, bindings)` from `query()`/`execute()`, OR uses real SQLite query results to verify behavior end-to-end. Either approach is acceptable; pick whichever matches the existing file's pattern. +- `composer test` for `packages/database-mysql` is green. +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +(Left blank — filled in by programmer during implementation) diff --git a/.claude/plans/query-builder-raw-select-where/003-pgsql-implementation.md b/.claude/plans/query-builder-raw-select-where/003-pgsql-implementation.md new file mode 100644 index 00000000..db742812 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/003-pgsql-implementation.md @@ -0,0 +1,123 @@ +# Task 003: PgSqlQueryBuilder — Implement `selectRaw` and `whereRaw` + +**Status**: completed +**Depends on**: 001 +**Retry count**: 0 + +## Description +Mirror task 002 for `PgSqlQueryBuilder`. Per the sibling-modules rule (`.claude/sibling-modules.md`), method names, visibilities, parameter ordering, and internal-state-field names MUST match the MySql implementation exactly. The only legitimate driver-specific difference here is the `selectRaw` bindings insertion site — `PgSqlQueryBuilder::buildSelectSql()` resets `$this->bindings = []` at its top (line 662), whereas MySql's `buildSelectSql()` does not. This is pre-existing sibling drift, not something this plan introduces; the workaround is documented below. + +The aggregate code path (`runAggregate` or equivalent) must also honor `whereRaw` conditions — same as task 002. + +## Context + +### Files to edit +- `packages/database-pgsql/src/Query/PgSqlQueryBuilder.php` (read first — 886 lines, sibling of `MySqlQueryBuilder`). +- `packages/database-pgsql/tests/Query/PgSqlQueryBuilderTest.php` (read first to understand the existing test pattern). + +### Verified file landmarks (read these before coding) +- `having()` at line 268 — same denylist as MySql's `having()`: `;`, `--`, `/*`, `*/` (no backtick). DO NOT touch this method. +- `orderBy()` at line 340 — there is NO `orderByRaw`. The plan's earlier draft incorrectly claimed otherwise. +- `buildSelectSql()` at line 660 — **resets `$this->bindings = []` at line 662** (unlike MySql). Calls `buildJoinClause()`, `buildWhereClause()`, etc. in the same order as MySql. +- `buildHavingClause()` at line 707 — appends `havingClause['bindings']` to `$this->bindings`. Same shape as MySql. +- `buildWhereClause()` — same structure as MySql; iterates `wheres`, `whereIns`, `whereNulls`, etc. Find the equivalent insertion point for the new `rawWheres` loop. +- Use SQL identifier quoting via `"` (Postgres) — not backticks. But raw expressions are passed through verbatim so no quote-aware compilation is needed. + +### Sibling-module rule + +State fields, method bodies, and the new private denylist helper must match `MySqlQueryBuilder` line-for-line (same names, same shape, same visibility). Tests should be near-identical mirrors of the MySql tests — same fixture inputs, same expectations modulo Postgres identifier quoting (`"` vs `` ` ``). + +### Per-driver `selectRaw` bindings insertion site (PgSql-specific) + +PgSql's `buildSelectSql()` already resets `$this->bindings = []` at line 662. Therefore in PgSql: + +- Insert `$this->rawSelectBindings` into `$this->bindings` AFTER the reset at line 662, BEFORE `buildWhereClause()` is called. +- ALSO append `$this->rawSelects` to the SELECT-list rendering at the same point where the regular `$this->columns` are rendered. + +```php +private function buildSelectSql(): string +{ + $this->bindings = []; + + // NEW: append raw-select bindings BEFORE WHERE compiles + foreach ($this->rawSelectBindings as $binding) { + $this->bindings[] = $binding; + } + + $regularColumns = $this->columns[0] === '*' + ? ['*'] + : array_map( + fn (string $col): string => $this->compileColumnExpression($col), + $this->columns, + ); + + // NEW: append raw select expressions verbatim + $selectParts = array_merge($regularColumns, $this->rawSelects); + $columns = implode(', ', $selectParts); + + // ... rest unchanged +} +``` + +### Aggregate path (`runAggregate` or equivalent) + +Same as task 002: identify the aggregate code path in PgSql. If it calls `buildWhereClause()`, and `buildWhereClause()` is extended to handle `rawWheres`, then aggregates automatically honor `whereRaw` with no further change. If PgSql's aggregate path is structured differently, add the equivalent handling. + +### **Important**: do NOT diverge from MySql in observable behavior + +The two drivers may have slightly different insertion sites internally (because of the `buildSelectSql()` reset drift), but their observable behavior — emitted SQL skeleton and final bindings array — MUST be identical for the same input. Task 005's cross-driver test enforces this. If the PgSql implementation seems to need observable divergence, that's a sign of a planning gap — flag it rather than silently diverging. + +## Requirements (Test Descriptions) + +### selectRaw — SELECT compilation +- [ ] `selectRaw appends the expression to the SELECT list after regular columns` +- [ ] `selectRaw alone (no prior select call) emits "SELECT *, " preserving the default *` +- [ ] `selectRaw can be called multiple times; expressions appear in call order in the SELECT list` +- [ ] `selectRaw together with select() emits select() columns first then selectRaw expressions` + +### selectRaw — bindings position +- [ ] `selectRaw with bindings places the bindings BEFORE WHERE bindings in the compiled bindings array (assert against a captured stub-connection bindings array)` +- [ ] `selectRaw bindings from multiple calls concatenate in call order` +- [ ] `running ->get() twice on the same builder produces identical SQL and bindings (no mutation of internal raw state)` + +### selectRaw — denylist +- [ ] `selectRaw throws InvalidColumnException when the expression contains a semicolon` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a -- comment marker` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker` +- [ ] `selectRaw throws InvalidColumnException when the expression contains a backtick` +- [ ] `selectRaw returns the builder for fluent chaining` + +### whereRaw — WHERE compilation +- [ ] `whereRaw appends the expression to the WHERE clause AND-combined with other conditions` +- [ ] `whereRaw used alone (no prior where) emits "WHERE " with no leading AND` +- [ ] `whereRaw can be called multiple times; expressions appear in call order, AND-combined` +- [ ] `whereRaw together with where() emits the regular where condition first then the raw expression, AND-combined` + +### whereRaw — bindings position +- [ ] `whereRaw with bindings places its bindings in the WHERE position of the bindings array (after selectRaw bindings, after regular where bindings if both exist)` +- [ ] `whereRaw bindings from multiple calls concatenate in call order` + +### whereRaw — aggregate path +- [ ] `count() honors whereRaw conditions (filtered count, not full-table count)` +- [ ] `min() / max() / sum() / avg() honor whereRaw conditions` + +### whereRaw — denylist +- [ ] `whereRaw throws InvalidColumnException when the expression contains a semicolon` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a -- comment marker` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker` +- [ ] `whereRaw throws InvalidColumnException when the expression contains a backtick` +- [ ] `whereRaw returns the builder for fluent chaining` + +### Combined selectRaw + whereRaw +- [ ] `selectRaw and whereRaw used together produce bindings in [select-bindings..., where-bindings...] order in the final bindings array` +- [ ] `selectRaw and whereRaw flow correctly through compileSubquery() when this builder is used as a UNION right-hand side` + +## Acceptance Criteria +- All requirements have passing tests. +- The PgSql implementation is structurally a mirror of the MySql implementation — same state-field names, same method names, same visibilities, same denylist helper signature. The only internal difference is the `selectRaw` bindings insertion site (because of the pre-existing `buildSelectSql()` reset drift in PgSql). +- `having()` is NOT modified. +- `composer test` for `packages/database-pgsql` is green. +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +(Left blank — filled in by programmer during implementation) diff --git a/.claude/plans/query-builder-raw-select-where/004-repository-passthrough.md b/.claude/plans/query-builder-raw-select-where/004-repository-passthrough.md new file mode 100644 index 00000000..56cf08f6 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/004-repository-passthrough.md @@ -0,0 +1,87 @@ +# Task 004: RepositoryQueryBuilder — Passthrough `selectRaw` and `whereRaw` + +**Status**: complete +**Depends on**: 001 +**Retry count**: 0 + +## Description +Add `selectRaw` and `whereRaw` to the delegating `RepositoryQueryBuilder` — same pattern as the existing `having()` delegation. Methods discard the inner builder's `static` return value and return `$this` so chaining works through the wrapper itself. + +## Context + +### Files to edit +- `packages/database/src/Repository/RepositoryQueryBuilder.php` (read first — 394 lines, delegates everything to an internal `private readonly QueryBuilderInterface $queryBuilder`). +- `packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php` (extend; OR create a sibling `RepositoryQueryBuilderRawTest.php` if the existing file becomes unwieldy — programmer's call). + +### Verified prior art (the closest existing delegation pattern) + +Lines 170–175 of `RepositoryQueryBuilder.php` — the `having()` delegation: + +```php +public function having(string $expression, array $bindings = []): static +{ + $this->queryBuilder->having($expression, $bindings); + return $this; +} +``` + +(Note: the plan's earlier draft incorrectly described an `orderByRaw` delegation at lines 232–242. That method does NOT exist in `RepositoryQueryBuilder` or anywhere else in the codebase. Use the `having()` delegation as the template instead.) + +### New methods to add + +Place adjacent to the corresponding non-raw siblings: +- `selectRaw` immediately after `select()` (around line 55). +- `whereRaw` immediately after `having()` (around line 175) — keeping all raw-expression delegations together — OR after `orWhere()` (around line 121) to keep WHERE-shaping methods together. Programmer's call; either placement is fine, but document the choice. + +```php +public function selectRaw( + string $expression, + array $bindings = [], +): static { + $this->queryBuilder->selectRaw($expression, $bindings); + return $this; +} + +public function whereRaw( + string $expression, + array $bindings = [], +): static { + $this->queryBuilder->whereRaw($expression, $bindings); + return $this; +} +``` + +### Test setup considerations +The existing `RepositoryQueryBuilderEnhancedTest.php` file uses `makeRqbStubBuilder()` (line ~78), an anonymous class implementing `QueryBuilderInterface`. Task 001 already added no-op `selectRaw`/`whereRaw` methods to this stub. For tests in THIS task that need to assert delegation behavior (e.g., bindings forwarded correctly, exceptions propagated), extend the stub to record `selectRaw`/`whereRaw` call arguments — same pattern the stub already uses for `wheresCalled` and `orderByCalled`. + +The denylist enforcement lives on the inner builder; the wrapper relies on it. Tests assert the wrapper propagates exceptions correctly (no swallowing). + +## Requirements (Test Descriptions) + +### Basic delegation +- [x] `selectRaw delegates to the inner query builder with the same expression and bindings` +- [x] `selectRaw returns the wrapper for fluent chaining` +- [x] `selectRaw propagates InvalidColumnException from the inner builder` +- [x] `whereRaw delegates to the inner query builder with the same expression and bindings` +- [x] `whereRaw returns the wrapper for fluent chaining` +- [x] `whereRaw propagates InvalidColumnException from the inner builder` + +### Chaining through the wrapper +- [x] `the wrapper can chain selectRaw and whereRaw alongside the existing methods (e.g. select.selectRaw.where.whereRaw.orderBy)` + +### QuerySpecification composition (smoke test for the real-world use case) +- [x] `a QuerySpecification that calls $builder->selectRaw(...) inside its apply() method works correctly when invoked via matching()` +- [x] `a QuerySpecification that calls $builder->whereRaw(...) inside its apply() method works correctly when invoked via matching()` + +## Acceptance Criteria +- All requirements have passing tests. +- Implementation is the minimal 4-line delegation per method, matching the `having()` shape. +- No mutation to the `having()` delegation or other unrelated methods. +- `composer test` for `packages/database` is green. +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +- Both `selectRaw` and `whereRaw` were already implemented in `RepositoryQueryBuilder.php` (lines 57–64 and 132–139) as part of Task 001 parallel work. No implementation changes were needed. +- The stub in `RepositoryQueryBuilderEnhancedTest.php` was extended to record `selectRaw`/`whereRaw` calls (added `$selectRawCalled`, `$whereRawCalled`, `$selectRawShouldThrow`, `$whereRawShouldThrow` tracking properties) following the existing `wheresCalled`/`orderByCalled` pattern. +- 9 new tests were added directly to the existing `RepositoryQueryBuilderEnhancedTest.php` file (file is not unwieldy, so no sibling file was needed). +- All 9 new tests passed immediately because the implementation was already in place. diff --git a/.claude/plans/query-builder-raw-select-where/005-cross-driver-consistency.md b/.claude/plans/query-builder-raw-select-where/005-cross-driver-consistency.md new file mode 100644 index 00000000..cc8a92ec --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/005-cross-driver-consistency.md @@ -0,0 +1,106 @@ +# Task 005: Cross-Driver Consistency Tests + +**Status**: completed +**Depends on**: 002, 003 +**Retry count**: 0 + +## Description +Add a Pest test that runs the same fixture input through `MySqlQueryBuilder` and `PgSqlQueryBuilder` for both `selectRaw` and `whereRaw`, then asserts: +1. The SQL skeleton matches the documented shape (modulo identifier quoting which is naturally driver-specific: `` ` `` vs `"`). +2. The final bindings array is identical across both drivers — SELECT-raw bindings come first, then WHERE-clause bindings (regular `where()` + `whereRaw` in call order). + +This is the lockdown against silent driver drift, per the plan's "Risks & Mitigations". + +## Context + +### Test file location + +**`tests/Integration/QueryBuilderRawConsistencyTest.php`** at the marko monorepo root (not inside `packages/database/tests/` — that package does not depend on either driver, so cross-driver tests cannot live there without inverting the dependency direction). + +The monorepo `phpunit.xml` defines a "Monorepo" testsuite at `tests`, so any file under the top-level `tests/` directory is auto-discovered by `composer test`. Create the `tests/Integration/` subdirectory if it does not already exist. + +The monorepo `composer.json` already requires both `marko/database-mysql` and `marko/database-pgsql` at `self.version`, so both driver classes are autoloadable from the root. + +### Recording stub `ConnectionInterface` + +To inspect the bindings array (which is `private` on both builders), the test uses a stub `ConnectionInterface` that records the `(sql, bindings)` pair from each `query()`/`execute()` call. Pattern: + +```php +function createRecordingConnection(): object +{ + return new class implements ConnectionInterface + { + public ?string $lastSql = null; + /** @var array */ + public array $lastBindings = []; + + public function query(string $sql, array $bindings = []): array + { + $this->lastSql = $sql; + $this->lastBindings = $bindings; + return []; // empty result — we only care about the captured pair + } + + public function execute(string $sql, array $bindings = []): int + { + $this->lastSql = $sql; + $this->lastBindings = $bindings; + return 0; + } + + // ... other ConnectionInterface methods: stub as needed (lastInsertId, connect, etc.) + }; +} +``` + +(Read `packages/database/src/Connection/ConnectionInterface.php` first to enumerate every method that must be implemented — missing methods = PHP fatal at test load.) + +### Fixture pattern + +```php +function applyFixture(QueryBuilderInterface $builder): void +{ + $builder + ->table('products') + ->select('id') + ->selectRaw('COALESCE(?, ?) AS resolved', ['a', 'b']) + ->where('active', '=', true) + ->whereRaw('price > ?', [100]); +} +// Expected bindings (both drivers): ['a', 'b', true, 100] +``` + +After applying the fixture and calling `->get()`, read `$connection->lastBindings` and assert against `['a', 'b', true, 100]` for BOTH drivers. + +### SQL skeleton assertion + +The emitted SQL differs between drivers in identifier quoting only: +- MySql: `` SELECT `id`, COALESCE(?, ?) AS resolved FROM `products` WHERE `active` = ? AND price > ? `` +- PgSql: `SELECT "id", COALESCE(?, ?) AS resolved FROM "products" WHERE "active" = ? AND price > ?` + +Strategy: normalize both SQLs by stripping `` ` `` and `"` characters before comparing. Or assert on substrings (e.g. `str_contains($sql, 'COALESCE(?, ?) AS resolved')`, `str_contains($sql, 'price > ?')`, `str_contains($sql, 'AND')`). + +### Do NOT touch `RepositoryQueryBuilder` in this task +That's task 004's territory. This test exercises the two driver builders directly. + +## Requirements (Test Descriptions) + +- [ ] `MySqlQueryBuilder and PgSqlQueryBuilder produce identical bindings array for the selectRaw + where + whereRaw fixture` +- [ ] `the bindings array order is exactly [select-raw-bindings..., where-bindings..., where-raw-bindings...] in both drivers` +- [ ] `both drivers include the raw select expression in the SELECT list, after the regular columns` +- [ ] `both drivers AND-combine the raw where expression with the regular where condition` +- [ ] `both drivers throw InvalidColumnException for each denylist input (semicolon, --, /*, */, backtick) — parameterized via Pest's it(...)->with([...])` +- [ ] `the emitted SQL contains the same SELECT-list ordering across both drivers (raw expression appended after regular columns)` +- [ ] `the emitted SQL contains the same WHERE-clause ordering across both drivers (regular where first, then whereRaw, AND-combined)` + +## Acceptance Criteria +- All requirements have passing tests. +- Test file lives at `tests/Integration/QueryBuilderRawConsistencyTest.php`. +- Test does NOT depend on a real database connection — uses the recording stub above. +- Denylist test is parameterized (`it(...)->with([...])` pattern) to avoid copy-paste across 5 patterns × 2 methods × 2 drivers. +- `composer test` is fully green in the monorepo root. +- The test is in the default suite (NOT tagged `integration-destructive`, since it uses stub connections — `composer test` excludes only `integration-destructive` per the root `composer.json`). +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +(Left blank — filled in by programmer during implementation) diff --git a/.claude/plans/query-builder-raw-select-where/006-changelogs.md b/.claude/plans/query-builder-raw-select-where/006-changelogs.md new file mode 100644 index 00000000..a7f353e5 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/006-changelogs.md @@ -0,0 +1,62 @@ +# Task 006: CHANGELOG Entry (root monorepo `/CHANGELOG.md`) + +**Status**: completed +**Depends on**: 005 +**Retry count**: 0 + +## Description +Add an entry to the root monorepo `/CHANGELOG.md` under its existing `## [Unreleased]` section. The marko monorepo uses a single root CHANGELOG (auto-generated from merged PR titles by `bin/release.sh` for tagged releases; manual entries permitted under `## [Unreleased]`). + +There are **no per-package CHANGELOG files** in `packages/database/`, `packages/database-mysql/`, or `packages/database-pgsql/` — earlier drafts of this plan incorrectly assumed otherwise. Verified via `find` across the monorepo: only `/CHANGELOG.md` exists at the root (excluding files inside `vendor/` directories). + +## Context + +### Files to edit +- `/CHANGELOG.md` at the marko monorepo root (read first to confirm the existing `## [Unreleased]` section and the `### New Features` style). + +### Existing root CHANGELOG style (verified by reading the file) + +The root CHANGELOG uses Keep-a-Changelog as the format reference but its actual section headings are: +- `### New Features` +- `### Bug Fixes` +- `### Maintenance` + +(NOT `### Added` / `### Changed` / `### Fixed`.) + +Entries are written as PR-title-style bullets, e.g.: +``` +* feat(page-cache): full-page HTTP cache with file driver, tag invalidation, and entity bridge by @michalbiarda in https://github.com/marko-php/marko/pull/58 +``` + +### Entry to add under `## [Unreleased]` → `### New Features` + +``` +* feat(database): add `QueryBuilderInterface::selectRaw(string, array)` and `::whereRaw(string, array)` with positional bindings, denylist validation, and `MySqlQueryBuilder` / `PgSqlQueryBuilder` / `RepositoryQueryBuilder` implementations +``` + +If a PR URL is known by the time this task runs, append `by @ in ` to match the file's style. + +### Out of scope for this task +- Per-package CHANGELOG files (they don't exist; do NOT create them). +- README updates (handled by `doc-updater` agent per `.claude/pipeline.md`). +- Mentioning `orWhereRaw` or `havingRaw` (those are explicitly deferred per `_plan.md` "Deferred"). +- Mentioning a "Changed" entry about refactoring `orderByRaw` — `orderByRaw` does not exist in the codebase and is not modified by this plan. + +## Requirements (Test Descriptions) + +There is no precedent for `Unit/ChangelogTest.php` in this monorepo (verified by `find . -name 'ChangelogTest.php'`). Do NOT invent one. The verification for this task is manual / human-reviewable: + +- [ ] The root `/CHANGELOG.md` has an `## [Unreleased]` section. +- [ ] Under `## [Unreleased]` → `### New Features` there is a bullet mentioning `QueryBuilderInterface::selectRaw` and `QueryBuilderInterface::whereRaw`. +- [ ] No bullet mentions `orWhereRaw`, `havingRaw`, or `orderByRaw` (those are explicitly deferred / non-existent). + +These are validated as part of the PR review, not as automated tests. + +## Acceptance Criteria +- The root `/CHANGELOG.md` has the entry above (or an equivalent paraphrase matching the file's existing style). +- No new files are created in `packages/database*/` for CHANGELOG purposes. +- `composer test` is fully green (this task doesn't add tests, but the previous tasks' tests must still pass). +- `phpcs`, `php-cs-fixer --dry-run`, `phpstan` clean. + +## Implementation Notes +(Left blank — filled in by programmer during implementation) diff --git a/.claude/plans/query-builder-raw-select-where/_devils_advocate.md b/.claude/plans/query-builder-raw-select-where/_devils_advocate.md new file mode 100644 index 00000000..4bc37ce1 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/_devils_advocate.md @@ -0,0 +1,184 @@ +# Devil's Advocate Review: query-builder-raw-select-where + +## Critical (Must fix before building) + +### C1. `orderByRaw` does NOT exist in the codebase — entire "refactor" premise is false +**Affected**: `_plan.md`, tasks 002, 003, 004, 006. + +The plan claims `orderByRaw` exists on `QueryBuilderInterface`, in `MySqlQueryBuilder` (with denylist at ~line 384), in `PgSqlQueryBuilder` (with denylist at ~line 384), and in `RepositoryQueryBuilder` (with delegation at lines 232–242). I verified via grep across the entire marko codebase: **zero matches** for `orderByRaw`. The interface ends with `raw()` at line 376. `MySqlQueryBuilder::orderBy` is at line 341 followed by `limit()` at 358 — there is no `orderByRaw`. `RepositoryQueryBuilder` lines 193–200 contain `orderBy()` followed directly by `limit()`. + +Consequences: +- Task 002's "extract denylist from `orderByRaw`" cannot be done — there's no inline denylist to extract. +- Task 003's mirror refactor is similarly impossible. +- Task 004's "mirror the existing `orderByRaw` delegation" has no template to mirror — the closest pattern is the existing `having()` delegation at lines 170–175. +- Task 006's CHANGELOG `## Changed` entries claiming `orderByRaw` was refactored are false. +- The plan's Origin/Discovery Notes claim "The scope `ScopedOrderBy` already works against `orderByRaw`" — but the markommerce scope plan (task 013) was written against an `orderByRaw` that doesn't actually exist either. That's a separate problem for the markommerce/scope plan, but our plan should not perpetuate the false premise. + +**Fix applied**: Removed all `orderByRaw` references, framed the denylist helper as NEW (not a refactor), removed the "shared with orderByRaw" requirement, and updated CHANGELOG entries to drop the bogus "Changed" sections. + +### C2. Per-package CHANGELOG files do not exist; the project uses a single root CHANGELOG +**Affected**: task 006. + +I verified: the only CHANGELOG.md in the marko monorepo is `/home/michal/www/marko/marko/CHANGELOG.md`. No CHANGELOG exists in `packages/database/`, `packages/database-mysql/`, or `packages/database-pgsql/`. The root CHANGELOG explicitly says "Entries from `0.4.0` onward are generated automatically by `bin/release.sh` from merged PR titles and labels". This means: +- The plan's directive to add entries to three per-package CHANGELOG files is incorrect. +- Creating per-package CHANGELOGs from scratch would diverge from the project's established release model. +- The acceptance criteria for a `Unit/ChangelogTest.php` would test files that shouldn't exist. + +**Fix applied**: Rewrote task 006 to update only the root monorepo `CHANGELOG.md` under its existing `## [Unreleased]` section using the project's actual format (`### New Features`, `feat(database): …` style). Removed the `ChangelogTest.php` requirement (no precedent in the repo). + +### C3. Adding methods to `QueryBuilderInterface` breaks ~15 anonymous-class stubs in the test suite +**Affected**: task 001, all downstream tasks (cascade test failures). + +`QueryBuilderInterface` is implemented by anonymous-class stubs in at least these test files (all in `packages/database/tests/`): +- `Repository/RepositoryTest.php` (line 1091) +- `Repository/RepositoryMatchingTest.php` (line 50) +- `Repository/StringPrimaryKeyTest.php` (line 153) +- `Repository/RepositoryWithTest.php` (lines 104, 551, 800, 1039) +- `Repository/RepositoryQueryBuilderEnhancedTest.php` (line 78) +- `Entity/RelationshipLoaderNestedTest.php` (line 345) +- `Entity/RelationshipLoaderTest.php` (lines 246, 482) +- `Entity/RelationshipLoaderBelongsToManyTest.php` (lines 200, 448) +- `Entity/RelationshipValidationTest.php` (line 49) +- `Query/QuerySpecificationTest.php` (lines 37, 255) +- `Query/SpecEagerLoadCompositionTest.php` (lines 107, 293) — plus an `EntityQueryBuilderInterface` stub at line 565 + +Each of these stubs implements every method of the interface (PHP fatal error if any is missing). Adding `selectRaw` and `whereRaw` to the interface in task 001 will cause every one of these tests to fail with `Class contains 2 abstract methods and must therefore be declared abstract`. The plan does not mention these stubs at all, let alone include them in task 001's work. + +**Fix applied**: Added an explicit requirement to task 001 to update every stub-impl of `QueryBuilderInterface` in `packages/database/tests/` (and the `EntityQueryBuilderInterface` stub in `SpecEagerLoadCompositionTest.php`) with no-op implementations that return `$this`. Added the full list of files to the task context. Added an acceptance criterion that `composer test` for `packages/database` passes after the change (the only way to confirm all stubs are caught). + +### C4. `QueryBuilderInterfaceTest.php` is reflection-only — no stub-impl pattern to mirror, and denylist behavior CANNOT be asserted at the contract level +**Affected**: task 001 (test requirements). + +I read the entire `QueryBuilderInterfaceTest.php` (307 lines). Every test uses `new ReflectionClass(QueryBuilderInterface::class)` to assert method presence, parameter names/types, and return types. There is **no** anonymous-class stub used to assert runtime behavior. The plan's task 001 says "Contract tests pass against a minimal stub implementation declared inline in the test (mirroring the existing test pattern)" — but there is no such pattern to mirror in this file. + +Furthermore, "denylist throws InvalidColumnException" is a behavior contract that can only be tested against an implementation. Forcing it at the interface level would either (a) invent a new stub pattern just for these two methods (inconsistent with the rest of the file), or (b) duplicate work that tasks 002/003 already do against real implementations. + +**Fix applied**: Removed the "denylist via inline stub" requirements from task 001. Kept only reflection-based assertions (method presence, parameter names/types, return type, PHPDoc tag presence via reflection). Behavior tests for the denylist live in tasks 002 and 003 against real driver implementations. + +### C5. Cross-driver test in `packages/database` is architecturally impossible +**Affected**: task 005. + +`packages/database/composer.json` requires only `marko/core` (no driver dependency — by design, per the interface/driver split). The plan's task 005 puts the cross-driver test in `packages/database/tests/Query/RawSelectWhereConsistencyTest.php` and instantiates both `MySqlQueryBuilder` and `PgSqlQueryBuilder` directly. This requires `packages/database` to depend on `marko/database-mysql` and `marko/database-pgsql`, which inverts the dependency direction and creates a circular dependency (drivers already depend on the interface package). + +The plan's claim that `QueryBuilderInterfaceTest.php` is precedent for cross-driver tests in `packages/database` is wrong — that file uses pure reflection and doesn't instantiate either driver. + +**Fix applied**: Moved the cross-driver consistency test to a new top-level integration directory: `tests/Integration/QueryBuilderRawConsistencyTest.php` (the marko monorepo's top-level tests directory, accessible via the monorepo `composer test`). If that directory does not exist, the task creates it. The task references the monorepo-level `composer.json` and verifies the test is registered in the monorepo Pest configuration. Alternative fallback documented in the task: parallel symmetric tests in each driver's own test suite asserting the same fixture against its own driver, plus a comment in each cross-referencing the other (lockdown via convention rather than a single test). + +### C6. Sibling drift in `buildSelectSql()` between MySql and PgSql changes where `selectRaw` bindings must be inserted +**Affected**: tasks 002, 003. + +`PgSqlQueryBuilder::buildSelectSql()` starts with `$this->bindings = []` (line 662). `MySqlQueryBuilder::buildSelectSql()` does NOT reset — it expects the caller (`get()`, `insert()`, etc.) to reset. This is pre-existing sibling drift, not introduced by this plan. + +Consequence: the implementation of "prepend `$rawSelectBindings` before WHERE bindings" must use different insertion sites per driver: +- **MySql**: caller (`get()` etc.) resets `$this->bindings = []`, then `buildSelectSql()` is called. Inside `buildSelectSql()`, before `buildWhereClause()` runs, append `$this->rawSelectBindings` to `$this->bindings`. +- **PgSql**: `buildSelectSql()` resets `$this->bindings = []` at line 662, then must append `$this->rawSelectBindings` BEFORE `buildWhereClause()` is invoked. + +The plan's task 002/003 instructions don't acknowledge this divergence and just say "verify exact position by tracing the existing binding flow" — leaving each worker to discover this independently and risk getting it wrong. + +**Fix applied**: Added explicit per-driver guidance in tasks 002 and 003 describing where the `$rawSelectBindings` insertion must occur, referencing the actual line numbers and the reset behavior in each file. Also added a test requirement that asserts `selectRaw` bindings appear before `where()` bindings in the final array (and that the bindings array is correctly ordered even after re-running the query — i.e., calling `get()` twice doesn't double-append). + +## Important (Should fix before building) + +### I1. `selectRaw` and `whereRaw` behavior in aggregate code paths (`count`, `min`, `max`, `sum`, `avg`) is undefined +**Affected**: tasks 002, 003. + +`MySqlQueryBuilder::runAggregate()` (line 533) and the PgSql equivalent reset `$this->bindings = []` and build a single-column SELECT directly, bypassing `buildSelectSql()` entirely. If a caller does `->selectRaw(...)->whereRaw(...)->count()`: +- `selectRaw` is silently ignored (aggregate replaces the entire SELECT list). +- `whereRaw` is silently ignored (`runAggregate` uses `buildWhereClause()` which only handles regular `where*` state). + +This is a foot-gun. Plan should either: +- Document the limitation in the interface PHPDoc ("raw select/where do not affect aggregate methods"). +- Make `runAggregate` honor `whereRaw` (selectRaw is meaningless for an aggregate, but whereRaw is meaningful). + +**Fix applied**: Updated tasks 002 and 003 to require `runAggregate()` (or whichever method builds the aggregate WHERE clause) to also include `whereRaw` conditions. Added a test requirement: "count()/min()/max() honor whereRaw conditions". Added a PHPDoc note on the interface `selectRaw` method that selectRaw is ignored when aggregate methods (`count`, `min`, etc.) are called, since those replace the SELECT list entirely. + +### I2. `compileSubquery` (UNION subquery path) — verify `selectRaw` and `whereRaw` flow through +**Affected**: tasks 002, 003. + +`compileSubquery` is called when this builder is used as the right-hand side of a UNION. It calls `buildSelectSql()`, captures `$this->bindings`, merges, restores. As long as `buildSelectSql()` correctly handles raw selects/wheres internally, this works — but it's worth an explicit test. + +**Fix applied**: Added a test requirement in tasks 002 and 003: "selectRaw and whereRaw bindings appear correctly in a UNION subquery's binding stream". + +### I3. Calling `selectRaw` or `whereRaw` multiple times — order preservation and re-call semantics +**Affected**: tasks 002, 003. + +The plan defines state as `private array $rawSelects = []` and `$rawWheres = []` — append-only — but doesn't add tests that: +- Multiple `selectRaw` calls produce SELECT list in call order. +- Multiple `whereRaw` calls produce WHERE conditions in call order, AND-combined. +- Bindings for multiple `selectRaw` calls are concatenated in call order. +- Bindings for multiple `whereRaw` calls are concatenated in call order. + +**Fix applied**: Added explicit test requirements for multi-call ordering to both tasks 002 and 003. + +### I4. `selectRaw` with empty `$this->columns` (default `['*']`) — what does the emitted SELECT look like? +**Affected**: tasks 002, 003. + +`$columns` defaults to `['*']`. If a user calls `->selectRaw('COUNT(*)')` without first calling `->select(...)`, the emitted SQL would be `SELECT *, COUNT(*) FROM ...` — almost certainly not what the user wants. Behavior should be documented and tested: +- Option A: appending `selectRaw` removes the implicit `*` if it's the only column. +- Option B: leave it alone, document that callers should pair `selectRaw` with `select()` explicitly. + +The plan picks neither. Laravel/Doctrine's convention is Option B (leave the `*`). For consistency with the plan's stated "Combined select: `select()` + `selectRaw()` together — raw selects are appended to the regular column list in compile order", Option B is the implicit choice but should be explicit. + +**Fix applied**: Added explicit test cases to tasks 002 and 003: "selectRaw with no prior select() emits `SELECT *, ` (the default `*` is preserved)". Documented this in the plan's Architecture Notes and in the method's PHPDoc. + +### I5. Repository wrapper test should also assert `selectRaw`/`whereRaw` work alongside `with()` and `matching()` +**Affected**: task 004. + +`RepositoryQueryBuilder` is the wrapper used by Repositories. The most likely real-world use of `selectRaw`/`whereRaw` is inside a `QuerySpecification` that calls them via the wrapper. Task 004 covers basic delegation but not the spec composition path. Worth a smoke test. + +**Fix applied**: Added a requirement to task 004: "selectRaw/whereRaw work when called from within a QuerySpecification applied via matching()". + +### I6. Task 001 declaration ordering — `selectRaw` placement and parameter naming convention +**Affected**: task 001. + +The plan says "place `selectRaw` immediately after the `distinct()` declaration" and "place `whereRaw` immediately after `orWhere`". Reading the interface, this is reasonable but worth noting: +- `select()` is at line 30, `distinct()` at line 43. Placing `selectRaw` after `distinct()` means readers see `select()` (line 30), then a bunch of where-methods (lines 44–134), then come back up mentally to find `selectRaw`. A clearer placement is **immediately after `select()`** so the SELECT-shaping methods sit together at the top. +- For `whereRaw` after `orWhere` (line 130–134) — that's fine. + +**Fix applied**: Updated task 001 to place `selectRaw` immediately after `select()` (around line 31) rather than after `distinct()`. Kept `whereRaw` after `orWhere`. + +### I7. Task 005's "stub ConnectionInterface" needs concrete shape +**Affected**: task 005. + +The bindings array on `MySqlQueryBuilder` is `private`. The only way to inspect the bindings that get sent to the connection is via a stub `ConnectionInterface` that records `($sql, $bindings)` from each `query()`/`execute()` call. Task 005 hints at this but doesn't specify the stub shape, the recording mechanism, or how to assert against the captured pair. + +**Fix applied**: Added concrete guidance to task 005: the stub records the last `($sql, $bindings)` pair from `query()` and exposes them via public properties; assertions read those properties after calling `->get()` on the builder. + +## Minor (Nice to address) + +### M1. The `having()` denylist drift is now permanent +The plan notes that `having()` doesn't block backticks today and intentionally leaves it that way. After this plan ships, the codebase will have: +- `having()` blocks `;`, `--`, `/*`, `*/` (no backtick). +- `selectRaw()` and `whereRaw()` block all 5 patterns. + +This is fine for now but worth a follow-up plan to unify the denylist into a shared `RawExpressionValidator` (already in the plan's "Deferred" section — good). + +### M2. PgSql's `buildSelectSql()` resets `$this->bindings` inside; MySql does not +This is pre-existing sibling drift, not caused by this plan, but worth a future cleanup plan. The two `buildSelectSql()` methods should have the same contract about who resets bindings. + +### M3. The exception factory `InvalidColumnException::invalidColumn()` returns a message that says "Validating SELECT column expression" — when used for `whereRaw`, the context will read awkwardly +The exception's `context` field hardcodes "Validating SELECT column expression `$column`". For `whereRaw` this will be misleading. A clean fix is to add a dedicated factory like `InvalidColumnException::invalidRawExpression(string $expression, string $clauseType)` so the error context says "Validating WHERE raw expression" or "Validating SELECT raw expression". This is a small improvement but not blocking. + +### M4. Task 006's verbatim CHANGELOG block uses an `Added`/`Changed` Keep-a-Changelog format +Even after fixing the per-package CHANGELOG issue, the entry style in the rewritten task should match the root CHANGELOG's actual style (which uses `### New Features` / `### Bug Fixes` / `### Maintenance`, not `### Added` / `### Changed`). + +**Fix applied** in C2 above. + +## Questions for the Team + +### Q1. `orderByRaw` clearly does not exist — what is the markommerce/scope team's actual plan? +The markommerce `scope-composite-overrides` plan (task 013) emits SQL like `builder.orderByRaw(sql, strtoupper($direction))`. That call will throw `BadMethodCallException` at runtime against any current `QueryBuilderInterface` impl. Three options: +1. Add `orderByRaw` to `QueryBuilderInterface` (and to MySql/PgSql/RepositoryQueryBuilder) as part of THIS plan, since adding raw select/where without raw order-by leaves the scope team blocked. +2. File a separate plan for `orderByRaw` and accept that markommerce/scope task 013 is blocked until then. +3. Have markommerce/scope rewrite `ScopedOrderBy` to use the escape-hatch `raw()` method or some other workaround. + +The user's framing of this review explicitly says "make sure additions are strictly additive — no signature changes on existing methods, no breaking behavior on `having()` / `orderByRaw()`", which implies the user believes `orderByRaw` exists. This is a Question for the team — the architecture review can flag it but not resolve it. + +### Q2. Should the denylist also include `\x00` (NUL byte) and `'` (single quote)? +Several SQL-injection cheat-sheets recommend rejecting NUL bytes (terminate query early in some parsers) and unescaped single quotes (string literal escape attempts). The plan only mirrors the existing `having()` denylist. Worth a future review pass, but out of scope here. + +### Q3. Should `selectRaw` validate that the expression contains at least one identifier-like token? +A defensive check ("expression must contain at least one of `[a-zA-Z_]` or `*`") would catch empty/whitespace-only strings. Currently the denylist passes those silently. Out of scope but worth considering. + +### Q4. Should `selectRaw` and `whereRaw` track which bindings came from raw clauses for debugging/profiling? +A future SQL debug logger might want to highlight which bindings originated from `selectRaw` vs `where()` vs `whereRaw`. The current `$bindings` array is a flat list. Not blocking — worth a follow-up. diff --git a/.claude/plans/query-builder-raw-select-where/_plan.md b/.claude/plans/query-builder-raw-select-where/_plan.md new file mode 100644 index 00000000..b34edd00 --- /dev/null +++ b/.claude/plans/query-builder-raw-select-where/_plan.md @@ -0,0 +1,230 @@ +# Plan: QueryBuilderInterface — `selectRaw` and `whereRaw` + +## Created +2026-05-19 + +## Status +completed + +## Objective +Add `selectRaw(string $expression, array $bindings = []): static` and `whereRaw(string $expression, array $bindings = []): static` to `Marko\Database\Query\QueryBuilderInterface`. Implement in `MySqlQueryBuilder`, `PgSqlQueryBuilder`, and the delegating `RepositoryQueryBuilder`. Sibling-module rule: identical method names, visibilities, and patterns across the two drivers. + +## Related Issues +none + +## Origin +Filed as a follow-up dependency of `markommerce/scope`'s `scope-composite-overrides` plan, which deferred `ScopedSelect` and `ScopedWhere` query specifications because the methods they'd compose against do not exist on `QueryBuilderInterface` today (only the escape-hatch `raw()` is exposed). Shipping `selectRaw` and `whereRaw` adds the symmetric SELECT and WHERE primitives needed by those specs. (`orderByRaw` — also needed by the markommerce scope work — is being added in a separate, already-open marko PR; see "Related work" below.) + +## Discovery Notes + +### Current state (verified against source) + +- `QueryBuilderInterface` defines `select(string ...$columns)`, `where(column, operator, value)`, `whereIn`, `whereNull`, `whereNotNull`, `whereJsonContains/Exists/Missing`, `orWhere`, `having(expression, bindings)` (already a raw form for HAVING), `orderBy`, and the escape-hatch `raw(sql, bindings)` — no `selectRaw`, no `whereRaw`, **and no `orderByRaw`**. +- `EntityQueryBuilderInterface` extends `QueryBuilderInterface` — additions to the parent propagate automatically. +- Two driver implementations: `MySqlQueryBuilder` (881 lines) and `PgSqlQueryBuilder` (886 lines). Both `implements QueryBuilderInterface` directly. Neither contains an `orderByRaw` method. +- `RepositoryQueryBuilder` is 394 lines, a delegating wrapper around an internal `$queryBuilder`. Adding the methods there is a straight passthrough mirroring the existing `having()` delegation at lines 170–175. +- Existing prior art for raw-with-bindings: `having(string $expression, array $bindings = [])` in both drivers. Its inline denylist rejects `;`, `--`, `/*`, `*/` (NO backtick). This plan deliberately uses a slightly stricter denylist on the new methods (adds backtick) but does NOT touch `having()`. +- Test layout: each driver has a `tests/Query/{Driver}QueryBuilderTest.php` plus topical files. The interface itself has `packages/database/tests/Query/QueryBuilderInterfaceTest.php` (reflection-only contract tests — no inline stub-impl pattern). The repository wrapper has `RepositoryQueryBuilderEnhancedTest.php`. +- The `feature/scope` branch in marko already merged some work; this plan branches off `develop` per the marko PR conventions. +- **The interface has ~15 anonymous-class stub implementations across `packages/database/tests/`** (full list in task 001). Adding methods to the interface forces every stub to be updated; otherwise PHP fatal errors at test load time. Plus one `EntityQueryBuilderInterface` stub in `SpecEagerLoadCompositionTest.php`. +- **No per-package CHANGELOG files exist.** The marko monorepo has a single root `CHANGELOG.md` populated by `bin/release.sh` from PR titles/labels. CHANGELOG updates in this plan target the root file under `## [Unreleased]`. +- **Sibling drift in `buildSelectSql()`**: `PgSqlQueryBuilder::buildSelectSql()` resets `$this->bindings = []` at its top (line 662); `MySqlQueryBuilder::buildSelectSql()` does NOT (the caller — `get()` etc. — resets). The `selectRaw` bindings insertion point therefore differs between the two drivers. +- **Aggregate paths bypass `buildSelectSql()`**: `count()`, `min()`, `max()`, `sum()`, `avg()` all call `runAggregate()` which builds its own SELECT and shares only `buildWhereClause()`. So `selectRaw` is meaningless during aggregates, and `whereRaw` must be wired into the aggregate WHERE path explicitly (see task 002/003 requirements). + +### Assumptions resolved up-front + +1. **API shape**: `selectRaw($expression, $bindings = [])` and `whereRaw($expression, $bindings = [])` — both return `static` for chaining. Bindings are positional `?` placeholders, mirroring `having()`. **No** `?$alias` parameter on `selectRaw` — the caller embeds `AS alias` in the expression if needed (Laravel/Doctrine convention; keeps the surface minimal). +2. **YAGNI on `orWhereRaw` and `havingRaw`**: this plan ships only `selectRaw` + `whereRaw`. `orWhereRaw` and `havingRaw` (or rather, `having` already takes a raw expression) can be added when a real consumer needs them. +3. **Combined select**: `select()` + `selectRaw()` together — raw selects are appended to the regular column list in compile order. The default `['*']` column list is preserved when only `selectRaw()` is called (caller must explicitly `select(...)` if they don't want `*`). Documented and tested. +4. **Validation**: same denylist as `having()` — block `;`, `--`, `/*`, `*/`. **Add backtick blocking too** as a defensive measure (raw expressions should not contain user-supplied identifier-quoting). Single rule across both new raw methods. The plan explicitly does NOT touch `having()`'s existing denylist (which is missing the backtick rule); unifying the two is deferred. +5. **Bindings flow**: `whereRaw`'s bindings are appended to the WHERE-clause bindings list in compile order. `selectRaw`'s bindings are prepended to the bindings stream at SELECT-list compile time (SELECT comes before WHERE in the SQL). Per-driver insertion point differs because of `buildSelectSql()` sibling drift — tasks 002 and 003 each document the exact insertion site. +6. **Aggregate methods**: `selectRaw` is documented as ignored when `count()`/`min()`/`max()`/`sum()`/`avg()` is called (those replace the SELECT list entirely). `whereRaw` IS honored by aggregate paths — `runAggregate()` is updated to include raw-where conditions alongside the regular WHERE clause. +7. **No `EntityQueryBuilderInterface` change**: the parent interface gets the methods; `EntityQueryBuilderInterface` inherits them. No separate change needed. +8. **Stub-impl cascade**: adding two methods to `QueryBuilderInterface` breaks every anonymous-class stub that implements it (~15 files in `packages/database/tests/`). Task 001 is responsible for updating all of them with no-op implementations that return `$this`. + +## Scope + +### In Scope +- `selectRaw(string $expression, array $bindings = []): static` on `QueryBuilderInterface`. +- `whereRaw(string $expression, array $bindings = []): static` on `QueryBuilderInterface`. +- `MySqlQueryBuilder` implementations (SQL compilation + tests). +- `PgSqlQueryBuilder` implementations (mirror — sibling rule). +- `RepositoryQueryBuilder` passthrough delegations (+ tests in `RepositoryQueryBuilderEnhancedTest.php`). +- Reflection-based contract tests in `packages/database/tests/Query/QueryBuilderInterfaceTest.php` asserting both methods are declared, return `static`, and take the expected params with expected names and defaults. (The file is reflection-only — there is no inline stub-impl pattern to mirror; behavior tests for the denylist live in tasks 002/003.) +- Updating every existing anonymous-class stub of `QueryBuilderInterface` (and the one `EntityQueryBuilderInterface` stub) with no-op `selectRaw` / `whereRaw` methods so the test suite still loads (see task 001 for the full file list). +- CHANGELOG entry in the root monorepo `/CHANGELOG.md` under `## [Unreleased]` (per-package CHANGELOGs do not exist in this monorepo). + +### Out of Scope +- `orWhereRaw`, `andWhereRaw`, `havingRaw`. (HAVING is already raw-by-default via the existing `having()` signature.) +- `selectRaw` with a dedicated `?string $alias` parameter — embed `AS alias` in the expression. +- Updating Marko's docs site (`docs/src/content/docs/`) — handled by the post-implementation `doc-updater` agent (per `.claude/pipeline.md`). +- Adding parsing or validation of the SQL inside the raw expression beyond the denylist — caller is responsible. +- Any change to `EntityQueryBuilderInterface` (inherits automatically). +- Any consumer wiring in markommerce — that's the follow-up `markommerce/scope` work (filing a `ScopedSelect`/`ScopedWhere` plan there will happen separately after this lands). + +## Success Criteria +- [ ] `QueryBuilderInterface::selectRaw` and `::whereRaw` are declared with the expected signatures and PHPDoc (including `@throws InvalidColumnException`). +- [ ] Every existing stub implementation of `QueryBuilderInterface` (and the one `EntityQueryBuilderInterface` stub) in `packages/database/tests/` has no-op `selectRaw` / `whereRaw` methods so the test suite loads. +- [ ] `MySqlQueryBuilder::selectRaw` and `::whereRaw` compile correctly into the emitted SQL with bindings in the right order. +- [ ] `PgSqlQueryBuilder::selectRaw` and `::whereRaw` ditto — identical observable behavior to MySql; per-driver internal insertion point for `selectRaw` bindings differs because of pre-existing `buildSelectSql()` sibling drift. +- [ ] `RepositoryQueryBuilder::selectRaw` and `::whereRaw` delegate cleanly. +- [ ] All denylist patterns (`;`, `--`, `/*`, `*/`, backtick) throw `InvalidColumnException` on both methods in both drivers. +- [ ] `select() + selectRaw()` combined emits both column sets in order with correct binding placement; `selectRaw()` alone (no prior `select()`) emits `SELECT *, `. +- [ ] `whereRaw` is honored by aggregate methods (`count`, `min`, `max`, `sum`, `avg`); `selectRaw` is documented as ignored by aggregates. +- [ ] `composer test` is fully green in the marko monorepo (this is the only way to confirm the stub cascade was caught and bindings/SQL flow works end-to-end). +- [ ] `./vendor/bin/phpcs`, `./vendor/bin/php-cs-fixer fix --dry-run`, `./vendor/bin/phpstan analyse` all clean. +- [ ] The root monorepo `/CHANGELOG.md` has a `## [Unreleased]` entry mentioning `selectRaw` and `whereRaw` in the project's actual changelog style (`### New Features`). + +## Task Overview + +| Task | Description | Depends on | Status | +|------|-------------|------------|--------| +| 001 | Add `selectRaw` and `whereRaw` declarations + PHPDoc to `QueryBuilderInterface`; update all ~15 anonymous-class stub implementations in `packages/database/tests/` + the one `EntityQueryBuilderInterface` stub with no-op methods; add reflection-based contract tests to `QueryBuilderInterfaceTest.php` asserting signatures | — | completed | +| 002 | Implement `selectRaw` and `whereRaw` in `MySqlQueryBuilder` — compile-time integration with select-list and where-clause builders, bindings ordering, denylist enforcement (new private helper, no refactor), aggregate WHERE path also honors `whereRaw`; tests in `MySqlQueryBuilderTest.php` | 001 | completed | +| 003 | Implement `selectRaw` and `whereRaw` in `PgSqlQueryBuilder` — mirror MySql impl per sibling rule (modulo the pre-existing `buildSelectSql()` reset drift, which forces a different bindings insertion point); tests in `PgSqlQueryBuilderTest.php` | 001 | completed | +| 004 | Implement passthrough `selectRaw` and `whereRaw` in `RepositoryQueryBuilder` (mirror the existing `having()` delegation at lines 170–175); tests in `RepositoryQueryBuilderEnhancedTest.php` covering basic delegation and use from within a `QuerySpecification` | 001 | completed | +| 005 | Cross-driver consistency: a test that runs the same input through `MySqlQueryBuilder` and `PgSqlQueryBuilder` via a recording stub `ConnectionInterface`, asserts the captured SQL skeleton + bindings array match between the drivers. Lives at the monorepo top-level `tests/Integration/` to avoid creating a circular dep on the driver packages from `packages/database` | 002, 003 | completed | +| 006 | One CHANGELOG entry in the root `/CHANGELOG.md` under `## [Unreleased]` (in the existing `### New Features` style) mentioning `selectRaw` and `whereRaw` on `QueryBuilderInterface`. No per-package CHANGELOG files; no README updates (handled by `doc-updater` per `.claude/pipeline.md`) | 005 | completed | + +Parallel batches: +``` +Batch 1: 001 +Batch 2: 002, 003, 004 (all ←001) +Batch 3: 005 (←002, 003) +Batch 4: 006 (←005) +``` + +## Architecture Notes + +### Method signatures (final) + +```php +/** + * Add a raw SQL expression to the SELECT list. + * + * The expression is appended after any columns added via select(). Include + * "AS alias" in the expression if you need a column alias. If no select() + * call precedes this, the default '*' column is preserved (emitting + * `SELECT *, `). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * Note: aggregate methods (count, min, max, sum, avg) build their own + * SELECT list and ignore selectRaw additions. + * + * @param string $expression Raw SQL select expression (e.g. "COALESCE(a, b) AS resolved") + * @param array $bindings Positional bindings for ? placeholders in the expression + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ +public function selectRaw( + string $expression, + array $bindings = [], +): static; + +/** + * Add a raw SQL WHERE condition with optional positional bindings. + * + * The expression is AND-combined with any other where conditions, in call + * order, after the regular where*/whereIn/whereNull/etc. conditions. + * + * Aggregate methods (count, min, max, sum, avg) honor whereRaw conditions + * the same way they honor where(). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * @param string $expression Raw SQL WHERE expression (e.g. "COALESCE(price, base) > ?") + * @param array $bindings Positional bindings for ? placeholders + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ +public function whereRaw( + string $expression, + array $bindings = [], +): static; +``` + +### Binding order + +- SELECT bindings come BEFORE WHERE bindings in the compiled SQL (per SQL standard). +- `selectRaw($expr, $bindings)` appends `$bindings` to a `private array $rawSelectBindings = []` member; the compile step appends them to `$this->bindings` BEFORE `buildWhereClause()` runs. +- `whereRaw($expr, $bindings)` appends to a `private array $rawWheres` list which `buildWhereClause()` consumes after the regular `where*` state, in call order, AND-combined. +- The cross-driver test (task 005) asserts both binding-order invariants with a concrete fixture (e.g. `select` + `selectRaw('… ?, ?', ['a','b'])` + `where('x','=',true)` + `whereRaw('y > ?', [100])` should produce bindings `['a','b',true,100]` in BOTH drivers). + +### Per-driver `selectRaw` bindings insertion site (sibling drift) + +- **MySql**: `buildSelectSql()` does NOT reset `$this->bindings`. Caller (`get()` line 380, etc.) resets first. Insert `$this->rawSelectBindings` into `$this->bindings` at the top of `buildSelectSql()`, before `buildWhereClause()` is called. +- **PgSql**: `buildSelectSql()` resets `$this->bindings = []` at line 662. Insert `$this->rawSelectBindings` into `$this->bindings` AFTER that reset, before `buildWhereClause()` is called. +- Both drivers must NOT mutate `$this->rawSelectBindings` during compile (so repeated `get()` calls produce identical bindings). + +### Denylist (security) + +Identical across `selectRaw` and `whereRaw` (NEW private helper per driver — there is no pre-existing helper to refactor): +- `;` (statement chaining) +- `--` (line comment) +- `/*` and `*/` (block comment) +- `` ` `` (backtick — MySQL identifier quote; raw expressions should use the driver's own quoting, not user-supplied backticks) + +The existing `having()` denylist does NOT block backticks today (verified via Read). This plan **does NOT change `having()`** — touching it would be scope creep. Future cleanup could unify the denylist into a shared `RawExpressionValidator`; deferred. + +### `RepositoryQueryBuilder` passthrough pattern + +Mirror the existing `having()` delegation at `packages/database/src/Repository/RepositoryQueryBuilder.php` lines 170–175: + +```php +public function selectRaw( + string $expression, + array $bindings = [], +): static { + $this->queryBuilder->selectRaw($expression, $bindings); + return $this; +} + +public function whereRaw( + string $expression, + array $bindings = [], +): static { + $this->queryBuilder->whereRaw($expression, $bindings); + return $this; +} +``` + +### Sibling-module rule + +`MySqlQueryBuilder` and `PgSqlQueryBuilder` must declare the methods with **identical** public-API parameter names, visibilities, and parameter ordering. The new state-field names (`$rawSelects`, `$rawSelectBindings`, `$rawWheres`) and the new private denylist helper (`assertNoDangerousPatterns`) MUST also match across both classes line-for-line. + +Observable behavior must be identical: same emitted SQL skeleton (modulo identifier quoting `` ` `` vs `"`) and same final bindings array for the same input. Task 005 enforces this. + +**The one legitimate internal divergence**: the `selectRaw` bindings insertion site differs between the two drivers because of pre-existing sibling drift in `buildSelectSql()` — PgSql resets `$this->bindings` at the top of that method while MySql relies on the caller. Both task 002 and task 003 document the per-driver site explicitly. This is not a new drift introduced by this plan and is captured in `## Deferred` for a future cleanup. + +Tests in 002 and 003 should be near-mirror images. + +## Risks & Mitigations + +- **Risk**: bindings get ordered wrong relative to SELECT/WHERE positions → silent SQL errors at runtime. **Mitigation**: task 005 is a dedicated cross-driver test with a fixture that uses both `selectRaw` and `whereRaw` together AND has a `where()` call between them, asserting the final bindings array order against a documented expectation. +- **Risk**: per-driver `buildSelectSql()` reset drift causes `selectRaw` bindings to land at different positions. **Mitigation**: the architecture note above documents the exact insertion site per driver; both task 002 and task 003 require explicit tests that assert the final bindings array shape. +- **Risk**: denylist drift (`having()` doesn't block backticks today; future maintainer adds them inconsistently). **Mitigation**: explicit "do NOT touch `having()`" note in this plan + a TODO captured in `## Deferred` below for a future denylist-unification plan. +- **Risk**: `RepositoryQueryBuilder` passthrough silently swallows the return value if `$this->queryBuilder->...` returns a new instance rather than `static`. **Mitigation**: existing `having()` delegation (lines 170–175) already follows the discard-and-`return $this` pattern; both drivers return `static`. Task 004's test asserts fluent chaining works through the wrapper. +- **Risk**: adding methods to `QueryBuilderInterface` breaks ~15 stub implementations in the existing test suite. **Mitigation**: task 001 explicitly lists every affected file and requires updating each one. The success criterion "`composer test` is fully green" cannot pass without all stubs being updated. +- **Risk**: aggregate methods silently ignore `whereRaw` conditions, producing wrong counts. **Mitigation**: tasks 002 and 003 explicitly require `runAggregate()` (or the equivalent path) to honor `whereRaw`. Tests assert `count()` with a `whereRaw` filter returns the filtered count, not the full-table count. +- **Risk**: docs page on `marko.build` for the database package becomes out of sync. **Mitigation**: marko's `post-implementation` pipeline runs `doc-updater` which handles this. No manual docs work in this plan. + +## Related work (resolved — not blocking) + +`orderByRaw` is being added in a **separate PR** that is already open against marko and awaiting maintainer review. That PR is the canonical home for the `orderByRaw` addition (interface + both drivers + repository delegation). This plan deliberately scopes itself to `selectRaw` + `whereRaw` only and does NOT add `orderByRaw`. + +Merge-order interaction: +- If the `orderByRaw` PR merges FIRST: when this plan lands, the new private `assertNoDangerousPatterns()` helper introduced here will be a duplicate of `orderByRaw`'s inline denylist. A small follow-up cleanup can unify them. Not a blocker for either PR. +- If this plan merges FIRST: the `orderByRaw` PR can either inline its own denylist (matching its existing style) or share `assertNoDangerousPatterns()` once rebased. +- Either order is safe — the PRs are functionally orthogonal. + +## Deferred (for a future plan) +- `orWhereRaw`, `andWhereRaw` — add when a real consumer needs OR-combined raw conditions. +- `havingRaw` — `having()` is already raw; rename + alias may be cleaner symmetry, but not urgent. +- Unify the raw-expression denylist into a single `RawExpressionValidator` class shared by `selectRaw`, `whereRaw`, `orderByRaw`, and `having`. After both this PR and the separate `orderByRaw` PR land, the denylist will be duplicated in three places. Worth a small cleanup plan then. +- Fix the `buildSelectSql()` sibling drift in `MySql` vs `PgSql` (one resets `$this->bindings`, the other doesn't) — pre-existing, not caused by this plan. +- Driver-specific raw-expression sanitizers (Postgres dollar-quoted strings, etc.) — out of scope; raw means raw. diff --git a/CHANGELOG.md b/CHANGELOG.md index 28abbd34..9261456a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Entries from `0.4.0` onward are generated automatically by `bin/release.sh` from ### New Features * feat: add marko/page-cache and marko/page-cache-file packages +* feat(database): add `selectRaw`, `whereRaw`, and `orderByRaw` to `QueryBuilderInterface` with positional bindings and denylist validation ## [0.5.0] - 2026-05-01 diff --git a/docs/src/content/docs/packages/database-mysql.md b/docs/src/content/docs/packages/database-mysql.md index 09f9eb28..8624018b 100644 --- a/docs/src/content/docs/packages/database-mysql.md +++ b/docs/src/content/docs/packages/database-mysql.md @@ -135,15 +135,18 @@ class MyService |---|---| | `table(string $table): static` | Set the target table | | `select(string ...$columns): static` | Choose columns to return | +| `selectRaw(string $expression, array $bindings = []): static` | Append a raw SQL expression to the SELECT list | | `where(string $column, string $operator, mixed $value): static` | Add a WHERE condition | | `orWhere(string $column, string $operator, mixed $value): static` | Add an OR WHERE condition | | `whereIn(string $column, array $values): static` | Add a WHERE IN condition | | `whereNull(string $column): static` | Add a WHERE IS NULL condition | | `whereNotNull(string $column): static` | Add a WHERE IS NOT NULL condition | +| `whereRaw(string $expression, array $bindings = []): static` | Add a raw SQL WHERE condition, AND-combined with other conditions | | `join(string $table, string $first, string $operator, string $second): static` | Inner join | | `leftJoin(string $table, string $first, string $operator, string $second): static` | Left join | | `rightJoin(string $table, string $first, string $operator, string $second): static` | Right join | | `orderBy(string $column, string $direction = 'ASC'): static` | Order results | +| `orderByRaw(string $expression, string $direction = 'ASC'): static` | Order by a raw SQL expression | | `limit(int $limit): static` | Limit result count | | `offset(int $offset): static` | Skip rows | | `get(): array` | Execute and return all matching rows | diff --git a/docs/src/content/docs/packages/database-pgsql.md b/docs/src/content/docs/packages/database-pgsql.md index d1bae90c..a4ddfa25 100644 --- a/docs/src/content/docs/packages/database-pgsql.md +++ b/docs/src/content/docs/packages/database-pgsql.md @@ -204,15 +204,18 @@ Implements `QueryBuilderInterface`. Fluent builder for PostgreSQL queries. |---|---| | `table(string $table): static` | Set the target table | | `select(string ...$columns): static` | Choose columns (defaults to `*`) | +| `selectRaw(string $expression, array $bindings = []): static` | Append a raw SQL expression to the SELECT list | | `where(string $column, string $operator, mixed $value): static` | Add a WHERE condition | | `orWhere(string $column, string $operator, mixed $value): static` | Add an OR WHERE condition | | `whereIn(string $column, array $values): static` | Add a WHERE IN condition | | `whereNull(string $column): static` | Add a WHERE IS NULL condition | | `whereNotNull(string $column): static` | Add a WHERE IS NOT NULL condition | +| `whereRaw(string $expression, array $bindings = []): static` | Add a raw SQL WHERE condition, AND-combined with other conditions | | `join(string $table, string $first, string $operator, string $second): static` | INNER JOIN | | `leftJoin(string $table, string $first, string $operator, string $second): static` | LEFT JOIN | | `rightJoin(string $table, string $first, string $operator, string $second): static` | RIGHT JOIN | | `orderBy(string $column, string $direction = 'ASC'): static` | Add ORDER BY clause | +| `orderByRaw(string $expression, string $direction = 'ASC'): static` | Order by a raw SQL expression | | `limit(int $limit): static` | Set LIMIT | | `offset(int $offset): static` | Set OFFSET | | `get(): array` | Execute SELECT and return all rows | diff --git a/docs/src/content/docs/packages/database.md b/docs/src/content/docs/packages/database.md index ec28c824..77619ff2 100644 --- a/docs/src/content/docs/packages/database.md +++ b/docs/src/content/docs/packages/database.md @@ -355,7 +355,35 @@ Use `getEntities()` / `firstEntity()` for typed domain objects. Drop to `get()` #### Available filters -`where`, `whereIn`, `whereNull`, `whereNotNull`, `orWhere`, `join`, `leftJoin`, `rightJoin`, `orderBy`, `limit`, `offset`, `select`. All return `static` for chaining. The escape hatch is `raw(string $sql, array $bindings = [])` for queries the builder can't express. +`where`, `whereIn`, `whereNull`, `whereNotNull`, `orWhere`, `whereRaw`, `join`, `leftJoin`, `rightJoin`, `orderBy`, `orderByRaw`, `limit`, `offset`, `select`, `selectRaw`. All return `static` for chaining. The escape hatch is `raw(string $sql, array $bindings = [])` for queries the builder can't express. + +#### Raw expressions + +Use `selectRaw` and `whereRaw` when the structured builder methods cannot express the SQL you need. Both accept a raw expression string and an optional array of positional `?` bindings. A denylist rejects expressions containing `;`, `--`, `/*`, `*/`, or backticks --- use `?` placeholders for user-supplied values instead of interpolating them directly. + +```php +// Compute a derived column inline +$rows = $this->query() + ->select('id', 'title') + ->selectRaw('COALESCE(published_at, created_at) AS display_date') + ->get(); + +// Filter on an expression that where() cannot express +$rows = $this->query() + ->whereRaw('COALESCE(price, base_price) > ?', [100]) + ->orderBy('title') + ->get(); + +// Both can be combined freely with structured methods +$rows = $this->query() + ->select('status') + ->selectRaw('COUNT(*) AS total') + ->whereRaw('EXTRACT(YEAR FROM created_at) = ?', [2024]) + ->groupBy('status') + ->get(); +``` + +`whereRaw` conditions are AND-combined with all other `where*` conditions and are also honoured by aggregate methods (`count`, `min`, `max`, `sum`, `avg`). #### Aggregate functions diff --git a/packages/database-mysql/src/Query/MySqlQueryBuilder.php b/packages/database-mysql/src/Query/MySqlQueryBuilder.php index 2b72bb0f..e4596eb7 100644 --- a/packages/database-mysql/src/Query/MySqlQueryBuilder.php +++ b/packages/database-mysql/src/Query/MySqlQueryBuilder.php @@ -81,6 +81,21 @@ class MySqlQueryBuilder implements QueryBuilderInterface private ?int $offsetValue = null; + /** + * @var list + */ + private array $rawSelects = []; + + /** + * @var list + */ + private array $rawSelectBindings = []; + + /** + * @var list}> + */ + private array $rawWheres = []; + /** * @var array */ @@ -106,6 +121,20 @@ public function select( return $this; } + /** + * @throws InvalidColumnException + */ + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + $this->assertNoDangerousPatterns($expression); + $this->rawSelects[] = $expression; + array_push($this->rawSelectBindings, ...$bindings); + + return $this; + } + public function distinct(): static { $this->distinct = true; @@ -252,11 +281,27 @@ public function orWhere( return $this; } + /** + * @throws InvalidColumnException + */ + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + $this->assertNoDangerousPatterns($expression); + $this->rawWheres[] = ['expression' => $expression, 'bindings' => $bindings]; + + return $this; + } + public function groupBy( string ...$columns, ): static { foreach ($columns as $column) { - if (!IdentifierValidator::isValidIdentifier($column) && !preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*\.[a-zA-Z_][a-zA-Z0-9_]*$/', $column)) { + if (!IdentifierValidator::isValidIdentifier($column) && !preg_match( + '/^[a-zA-Z_][a-zA-Z0-9_]*\.[a-zA-Z_][a-zA-Z0-9_]*$/', + $column + )) { throw InvalidColumnException::invalidColumn($column); } } @@ -350,6 +395,25 @@ public function orderBy( $this->orders[] = [ 'column' => $column, 'direction' => $direction, + 'raw' => false, + ]; + + return $this; + } + + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + $direction = strtoupper($direction); + if (!in_array($direction, ['ASC', 'DESC'], true)) { + $direction = 'ASC'; + } + + $this->orders[] = [ + 'column' => $expression, + 'direction' => $direction, + 'raw' => true, ]; return $this; @@ -672,12 +736,18 @@ private function buildSelectSql(): string return $this->compileColumnExpression($col); }, $this->columns); + $selectParts = array_merge($quotedColumns, $this->rawSelects); + + foreach ($this->rawSelectBindings as $binding) { + $this->bindings[] = $binding; + } + $keyword = $this->distinct ? 'SELECT DISTINCT' : 'SELECT'; $sql = sprintf( '%s %s FROM %s', $keyword, - implode(', ', $quotedColumns), + implode(', ', $selectParts), $this->quoteIdentifier($this->table), ); @@ -839,6 +909,17 @@ private function buildWhereClause(): string $conditions[] = $expr; } + foreach ($this->rawWheres as $item) { + $expr = $item['expression']; + $this->bindings = array_merge($this->bindings, $item['bindings']); + + if (!empty($conditions)) { + $expr = 'AND ' . $expr; + } + + $conditions[] = $expr; + } + if (empty($conditions)) { return ''; } @@ -855,7 +936,7 @@ private function buildOrderByClause(): string $orders = array_map( fn ($order) => sprintf( '%s %s', - $this->quoteIdentifier($order['column']), + $order['raw'] ? $order['column'] : $this->quoteIdentifier($order['column']), $order['direction'], ), $this->orders, @@ -878,4 +959,20 @@ private function buildLimitOffsetClause(): string return $sql; } + + /** + * @throws InvalidColumnException + */ + private function assertNoDangerousPatterns(string $expression): void + { + if ( + str_contains($expression, ';') + || str_contains($expression, '--') + || str_contains($expression, '/*') + || str_contains($expression, '*/') + || str_contains($expression, '`') + ) { + throw InvalidColumnException::invalidColumn($expression); + } + } } diff --git a/packages/database-mysql/tests/Query/MySqlQueryBuilderTest.php b/packages/database-mysql/tests/Query/MySqlQueryBuilderTest.php index 21a601a7..8f51a84a 100644 --- a/packages/database-mysql/tests/Query/MySqlQueryBuilderTest.php +++ b/packages/database-mysql/tests/Query/MySqlQueryBuilderTest.php @@ -6,7 +6,7 @@ use Marko\Core\Path\ProjectPaths; use Marko\Database\Config\DatabaseConfig; -use Marko\Database\Connection\ConnectionInterface; +use Marko\Database\Exceptions\InvalidColumnException; use Marko\Database\Exceptions\UnionShapeMismatchException; use Marko\Database\MySql\Connection\MySqlConnection; use Marko\Database\MySql\Query\MySqlQueryBuilder; @@ -331,17 +331,19 @@ public function lastInsertId(): int $recordedSql = ''; $recordedBindings = []; - $recordingConnection = new class ($this->connection, $recordedSql, $recordedBindings) extends MySqlConnection + $recordingConnection = new class ($recordedSql, $recordedBindings) extends MySqlConnection { public function __construct( - private readonly ConnectionInterface $inner, public string &$lastSql, public array &$lastBindings, ) {} public function connect(): void {} - public function query(string $sql, array $bindings = []): array + public function query( + string $sql, + array $bindings = [], + ): array { $this->lastSql = $sql; $this->lastBindings = $bindings; @@ -349,7 +351,10 @@ public function query(string $sql, array $bindings = []): array return []; } - public function execute(string $sql, array $bindings = []): int + public function execute( + string $sql, + array $bindings = [], + ): int { return 0; } @@ -373,18 +378,21 @@ public function execute(string $sql, array $bindings = []): int ->and($recordedBindings)->toBe(['active', 'inactive']); }); - it('throws UnionShapeMismatchException when the two queries select different numbers of columns', function (): void { - $left = (new MySqlQueryBuilder($this->connection)) - ->table('users') - ->select('name', 'email'); - - $right = (new MySqlQueryBuilder($this->connection)) - ->table('users') - ->select('name'); - - expect(fn () => $left->union($right)) - ->toThrow(UnionShapeMismatchException::class); - }); + it( + 'throws UnionShapeMismatchException when the two queries select different numbers of columns', + function (): void { + $left = (new MySqlQueryBuilder($this->connection)) + ->table('users') + ->select('name', 'email'); + + $right = (new MySqlQueryBuilder($this->connection)) + ->table('users') + ->select('name'); + + expect(fn () => $left->union($right)) + ->toThrow(UnionShapeMismatchException::class); + } + ); it('combines two queries with UNION ALL preserving duplicates', function (): void { $recordedSql = ''; @@ -399,7 +407,10 @@ public function __construct( public function connect(): void {} - public function query(string $sql, array $bindings = []): array + public function query( + string $sql, + array $bindings = [], + ): array { $this->lastSql = $sql; $this->lastBindings = $bindings; @@ -407,7 +418,10 @@ public function query(string $sql, array $bindings = []): array return []; } - public function execute(string $sql, array $bindings = []): int + public function execute( + string $sql, + array $bindings = [], + ): int { return 0; } @@ -439,14 +453,20 @@ public function __construct(public string &$lastSql) {} public function connect(): void {} - public function query(string $sql, array $bindings = []): array + public function query( + string $sql, + array $bindings = [], + ): array { $this->lastSql = $sql; return []; } - public function execute(string $sql, array $bindings = []): int + public function execute( + string $sql, + array $bindings = [], + ): int { return 0; } @@ -475,14 +495,20 @@ public function __construct(public string &$lastSql) {} public function connect(): void {} - public function query(string $sql, array $bindings = []): array + public function query( + string $sql, + array $bindings = [], + ): array { $this->lastSql = $sql; return []; } - public function execute(string $sql, array $bindings = []): int + public function execute( + string $sql, + array $bindings = [], + ): int { return 0; } @@ -516,7 +542,10 @@ public function __construct( public function connect(): void {} - public function query(string $sql, array $bindings = []): array + public function query( + string $sql, + array $bindings = [], + ): array { $this->lastSql = $sql; $this->lastBindings = $bindings; @@ -524,7 +553,10 @@ public function query(string $sql, array $bindings = []): array return []; } - public function execute(string $sql, array $bindings = []): int + public function execute( + string $sql, + array $bindings = [], + ): int { return 0; } @@ -597,4 +629,641 @@ public function execute(string $sql, array $bindings = []): int ->and($results[0]['author_name'])->toBe('Alice') ->and($results[0]['contact'])->toBe('alice@example.com'); }); + + describe('selectRaw', function (): void { + it('selectRaw appends the expression to the SELECT list after regular columns', function (): void { + $results = $this->builder + ->table('users') + ->select('name') + ->selectRaw("'static' AS extra") + ->get(); + + expect($results)->toHaveCount(3) + ->and($results[0])->toHaveKeys(['name', 'extra']) + ->and($results[0]['extra'])->toBe('static'); + }); + + it( + 'selectRaw alone (no prior select call) emits "SELECT *, " preserving the default *', + function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('1 AS one') + ->get(); + + expect($recordedSql)->toBe('SELECT *, 1 AS one FROM `users`'); + } + ); + + it( + 'selectRaw can be called multiple times; expressions appear in call order in the SELECT list', + function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->select('name') + ->selectRaw('1 AS first') + ->selectRaw('2 AS second') + ->get(); + + expect($recordedSql)->toBe('SELECT `name`, 1 AS first, 2 AS second FROM `users`'); + } + ); + + it( + 'selectRaw together with select() emits select() columns first then selectRaw expressions', + function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->select('name', 'email') + ->selectRaw('1 AS computed') + ->get(); + + expect($recordedSql)->toBe('SELECT `name`, `email`, 1 AS computed FROM `users`'); + } + ); + + it( + 'selectRaw with bindings places the bindings BEFORE WHERE bindings in the compiled bindings array', + function (): void { + $recordedBindings = []; + $recordingConnection = new class ($recordedBindings) extends MySqlConnection + { + public function __construct(public array &$lastBindings) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('? AS val', [42]) + ->where('status', '=', 'active') + ->get(); + + expect($recordedBindings)->toBe([42, 'active']); + } + ); + + it('selectRaw bindings from multiple calls concatenate in call order', function (): void { + $recordedBindings = []; + $recordingConnection = new class ($recordedBindings) extends MySqlConnection + { + public function __construct(public array &$lastBindings) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('? AS first', [1]) + ->selectRaw('? AS second', [2]) + ->get(); + + expect($recordedBindings)->toBe([1, 2]); + }); + + it( + 'running ->get() twice on the same builder produces identical SQL and bindings (no mutation of internal raw state)', + function (): void { + $sqls = []; + $bindingsList = []; + $recordingConnection = new class ($sqls, $bindingsList) extends MySqlConnection + { + public function __construct( + public array &$sqls, + public array &$bindingsList, + ) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->sqls[] = $sql; + $this->bindingsList[] = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + $builder = (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('? AS val', [99]) + ->where('status', '=', 'active'); + + $builder->get(); + $builder->get(); + + expect($sqls[0])->toBe($sqls[1]) + ->and($bindingsList[0])->toBe($bindingsList[1]); + } + ); + + it('selectRaw throws InvalidColumnException when the expression contains a semicolon', function (): void { + expect(fn () => $this->builder->selectRaw('1; DROP TABLE users')) + ->toThrow(InvalidColumnException::class); + }); + + it( + 'selectRaw throws InvalidColumnException when the expression contains a -- comment marker', + function (): void { + expect(fn () => $this->builder->selectRaw('1 -- comment')) + ->toThrow(InvalidColumnException::class); + } + ); + + it( + 'selectRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker', + function (): void { + expect(fn () => $this->builder->selectRaw('/* comment */ 1')) + ->toThrow(InvalidColumnException::class); + } + ); + + it('selectRaw throws InvalidColumnException when the expression contains a backtick', function (): void { + expect(fn () => $this->builder->selectRaw('`name`')) + ->toThrow(InvalidColumnException::class); + }); + + it('selectRaw returns the builder for fluent chaining', function (): void { + $result = $this->builder->selectRaw('1 AS one'); + expect($result)->toBe($this->builder); + }); + }); + + describe('whereRaw', function (): void { + it( + 'whereRaw appends the expression to the WHERE clause AND-combined with other conditions', + function (): void { + $results = $this->builder + ->table('users') + ->select('name') + ->where('status', '=', 'active') + ->whereRaw("name != 'Bob'") + ->get(); + + $names = array_column($results, 'name'); + expect($results)->toHaveCount(2) + ->and($names)->toContain('Alice') + ->and($names)->toContain('Charlie'); + } + ); + + it('whereRaw used alone (no prior where) emits "WHERE " with no leading AND', function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->whereRaw('status = ?', ['active']) + ->get(); + + expect($recordedSql)->toBe('SELECT * FROM `users` WHERE status = ?'); + }); + + it('whereRaw can be called multiple times; expressions appear in call order, AND-combined', function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->whereRaw('status = ?', ['active']) + ->whereRaw('name != ?', ['Bob']) + ->get(); + + expect($recordedSql)->toBe('SELECT * FROM `users` WHERE status = ? AND name != ?'); + }); + + it( + 'whereRaw together with where() emits the regular where condition first then the raw expression, AND-combined', + function (): void { + $recordedSql = ''; + $recordingConnection = new class ($recordedSql) extends MySqlConnection + { + public function __construct(public string &$lastSql) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->where('status', '=', 'active') + ->whereRaw('name != ?', ['Bob']) + ->get(); + + expect($recordedSql)->toBe('SELECT * FROM `users` WHERE `status` = ? AND name != ?'); + } + ); + + it( + 'whereRaw with bindings places its bindings in the WHERE position of the bindings array (after selectRaw bindings, after regular where bindings if both exist)', + function (): void { + $recordedBindings = []; + $recordingConnection = new class ($recordedBindings) extends MySqlConnection + { + public function __construct(public array &$lastBindings) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('? AS sel', ['sel_val']) + ->where('status', '=', 'active') + ->whereRaw('name != ?', ['Bob']) + ->get(); + + expect($recordedBindings)->toBe(['sel_val', 'active', 'Bob']); + } + ); + + it('whereRaw bindings from multiple calls concatenate in call order', function (): void { + $recordedBindings = []; + $recordingConnection = new class ($recordedBindings) extends MySqlConnection + { + public function __construct(public array &$lastBindings) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->whereRaw('a = ?', [1]) + ->whereRaw('b = ?', [2]) + ->get(); + + expect($recordedBindings)->toBe([1, 2]); + }); + + it('count() honors whereRaw conditions (filtered count, not full-table count)', function (): void { + $count = $this->builder + ->table('users') + ->whereRaw("status = 'active'") + ->count(); + + expect($count)->toBe(2); + }); + + it('min() / max() / sum() / avg() honor whereRaw conditions', function (): void { + $min = (new MySqlQueryBuilder($this->connection)) + ->table('users') + ->whereRaw("status = 'active'") + ->min('id'); + + $max = (new MySqlQueryBuilder($this->connection)) + ->table('users') + ->whereRaw("status = 'active'") + ->max('id'); + + expect($min)->toBe(1) + ->and($max)->toBe(3); + }); + + it('whereRaw throws InvalidColumnException when the expression contains a semicolon', function (): void { + expect(fn () => $this->builder->whereRaw('1; DROP TABLE users')) + ->toThrow(InvalidColumnException::class); + }); + + it( + 'whereRaw throws InvalidColumnException when the expression contains a -- comment marker', + function (): void { + expect(fn () => $this->builder->whereRaw('1 -- comment')) + ->toThrow(InvalidColumnException::class); + } + ); + + it( + 'whereRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker', + function (): void { + expect(fn () => $this->builder->whereRaw('/* comment */ 1')) + ->toThrow(InvalidColumnException::class); + } + ); + + it('whereRaw throws InvalidColumnException when the expression contains a backtick', function (): void { + expect(fn () => $this->builder->whereRaw('`name` = ?', ['Alice'])) + ->toThrow(InvalidColumnException::class); + }); + + it('whereRaw returns the builder for fluent chaining', function (): void { + $result = $this->builder->whereRaw('1 = 1'); + expect($result)->toBe($this->builder); + }); + }); + + describe('selectRaw and whereRaw combined', function (): void { + it( + 'selectRaw and whereRaw used together produce bindings in [select-bindings..., where-bindings...] order in the final bindings array', + function (): void { + $recordedBindings = []; + $recordingConnection = new class ($recordedBindings) extends MySqlConnection + { + public function __construct(public array &$lastBindings) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->selectRaw('? AS sel', ['sel_val']) + ->whereRaw('status = ?', ['active']) + ->get(); + + expect($recordedBindings)->toBe(['sel_val', 'active']); + } + ); + + it( + 'selectRaw and whereRaw flow correctly through compileSubquery() when this builder is used as a UNION right-hand side', + function (): void { + $recordedSql = ''; + $recordedBindings = []; + $recordingConnection = new class ($recordedSql, $recordedBindings) extends MySqlConnection + { + public function __construct( + public string &$lastSql, + public array &$lastBindings, + ) {} + + public function connect(): void {} + + public function query( + string $sql, + array $bindings = [], + ): array + { + $this->lastSql = $sql; + $this->lastBindings = $bindings; + + return []; + } + + public function execute( + string $sql, + array $bindings = [], + ): int + { + return 0; + } + }; + + $left = (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->select('name'); + + $right = (new MySqlQueryBuilder($recordingConnection)) + ->table('users') + ->select('name') + ->selectRaw('? AS sel', ['sel_val']) + ->whereRaw('status = ?', ['active']); + + $left->union($right)->get(); + + expect($recordedSql)->toContain('? AS sel') + ->and($recordedSql)->toContain('status = ?') + ->and($recordedBindings)->toBe(['sel_val', 'active']); + } + ); + }); }); diff --git a/packages/database-pgsql/src/Query/PgSqlQueryBuilder.php b/packages/database-pgsql/src/Query/PgSqlQueryBuilder.php index 6758d1bb..1eddc182 100644 --- a/packages/database-pgsql/src/Query/PgSqlQueryBuilder.php +++ b/packages/database-pgsql/src/Query/PgSqlQueryBuilder.php @@ -86,6 +86,21 @@ class PgSqlQueryBuilder implements QueryBuilderInterface */ private array $bindings = []; + /** + * @var list + */ + private array $rawSelects = []; + + /** + * @var list + */ + private array $rawSelectBindings = []; + + /** + * @var list}> + */ + private array $rawWheres = []; + public function __construct( private readonly ConnectionInterface $connection, ) {} @@ -106,6 +121,20 @@ public function select( return $this; } + /** + * @throws InvalidColumnException + */ + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + $this->assertNoDangerousPatterns($expression); + $this->rawSelects[] = $expression; + array_push($this->rawSelectBindings, ...$bindings); + + return $this; + } + public function distinct(): static { $this->distinct = true; @@ -251,11 +280,27 @@ public function orWhere( return $this; } + /** + * @throws InvalidColumnException + */ + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + $this->assertNoDangerousPatterns($expression); + $this->rawWheres[] = ['expression' => $expression, 'bindings' => $bindings]; + + return $this; + } + public function groupBy( string ...$columns, ): static { foreach ($columns as $column) { - if (!IdentifierValidator::isValidIdentifier($column) && !preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*\.[a-zA-Z_][a-zA-Z0-9_]*$/', $column)) { + if (!IdentifierValidator::isValidIdentifier($column) && !preg_match( + '/^[a-zA-Z_][a-zA-Z0-9_]*\.[a-zA-Z_][a-zA-Z0-9_]*$/', + $column + )) { throw InvalidColumnException::invalidColumn($column); } } @@ -349,6 +394,25 @@ public function orderBy( $this->orders[] = [ 'column' => $column, 'direction' => $direction, + 'raw' => false, + ]; + + return $this; + } + + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + $direction = strtoupper($direction); + if (!in_array($direction, ['ASC', 'DESC'], true)) { + $direction = 'ASC'; + } + + $this->orders[] = [ + 'column' => $expression, + 'direction' => $direction, + 'raw' => true, ]; return $this; @@ -607,7 +671,7 @@ private function compilePgJsonPath(string $expression): string $lastIndex = count($path->segments) - 1; foreach ($path->segments as $i => $segment) { - $op = ($i === $lastIndex) ? $path->operator : '->'; + $op = $i === $lastIndex ? $path->operator : '->'; $sql .= $op . "'" . $segment . "'"; } @@ -657,20 +721,40 @@ private function compileColumnExpression( return $compiledColumn; } + /** + * @throws InvalidColumnException + */ + private function assertNoDangerousPatterns(string $expression): void + { + if ( + str_contains($expression, ';') + || str_contains($expression, '--') + || str_contains($expression, '/*') + || str_contains($expression, '*/') + || str_contains($expression, '`') + ) { + throw InvalidColumnException::invalidColumn($expression); + } + } + private function buildSelectSql(): string { $this->bindings = []; - $columns = $this->columns[0] === '*' - ? '*' - : implode( - ', ', - array_map( - fn (string $col): string => $this->compileColumnExpression($col), - $this->columns, - ), + foreach ($this->rawSelectBindings as $binding) { + $this->bindings[] = $binding; + } + + $regularColumns = $this->columns[0] === '*' + ? ['*'] + : array_map( + fn (string $col): string => $this->compileColumnExpression($col), + $this->columns, ); + $selectParts = array_merge($regularColumns, $this->rawSelects); + $columns = implode(', ', $selectParts); + $keyword = $this->distinct ? 'SELECT DISTINCT' : 'SELECT'; $sql = sprintf( @@ -844,6 +928,19 @@ private function buildWhereClause(): string } } + // Raw WHERE conditions + foreach ($this->rawWheres as $rawWhere) { + if (empty($conditions)) { + $conditions[] = $rawWhere['expression']; + } else { + $conditions[] = 'AND ' . $rawWhere['expression']; + } + + foreach ($rawWhere['bindings'] as $binding) { + $this->bindings[] = $binding; + } + } + if (empty($conditions)) { return ''; } @@ -860,7 +957,7 @@ private function buildOrderByClause(): string $clauses = array_map( fn (array $order): string => sprintf( '%s %s', - $this->quoteIdentifier($order['column']), + $order['raw'] ?? false ? $order['column'] : $this->quoteIdentifier($order['column']), $order['direction'], ), $this->orders, diff --git a/packages/database-pgsql/tests/Query/PgSqlQueryBuilderSelectRawWhereRawTest.php b/packages/database-pgsql/tests/Query/PgSqlQueryBuilderSelectRawWhereRawTest.php new file mode 100644 index 00000000..3df19fc8 --- /dev/null +++ b/packages/database-pgsql/tests/Query/PgSqlQueryBuilderSelectRawWhereRawTest.php @@ -0,0 +1,361 @@ +table('users') + ->select('name') + ->selectRaw("'static' AS extra") + ->get(); + + expect($connection->lastQuerySql)->toBe("SELECT \"name\", 'static' AS extra FROM \"users\""); + }); + + it( + 'selectRaw alone (no prior select call) emits "SELECT *, " preserving the default *', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw("'hello' AS greeting") + ->get(); + + expect($connection->lastQuerySql)->toBe("SELECT *, 'hello' AS greeting FROM \"users\""); + } + ); + + it( + 'selectRaw can be called multiple times; expressions appear in call order in the SELECT list', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw("'first' AS one") + ->selectRaw("'second' AS two") + ->get(); + + expect($connection->lastQuerySql)->toBe("SELECT *, 'first' AS one, 'second' AS two FROM \"users\""); + } + ); + + it('selectRaw together with select() emits select() columns first then selectRaw expressions', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->select('id', 'name') + ->selectRaw("'extra' AS computed") + ->get(); + + expect($connection->lastQuerySql)->toBe("SELECT \"id\", \"name\", 'extra' AS computed FROM \"users\""); + }); + + it( + 'selectRaw with bindings places the bindings BEFORE WHERE bindings in the compiled bindings array', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw('? AS val', [42]) + ->where('id', '=', 1) + ->get(); + + expect($connection->lastQueryBindings)->toBe([42, 1]); + } + ); + + it('selectRaw bindings from multiple calls concatenate in call order', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw('? AS first', [10]) + ->selectRaw('? AS second', [20]) + ->get(); + + expect($connection->lastQueryBindings)->toBe([10, 20]); + }); + + it( + 'running ->get() twice on the same builder produces identical SQL and bindings (no mutation of internal raw state)', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw('? AS val', [42]) + ->where('id', '=', 1); + + $builder->get(); + $firstSql = $connection->lastQuerySql; + $firstBindings = $connection->lastQueryBindings; + + $builder->get(); + $secondSql = $connection->lastQuerySql; + $secondBindings = $connection->lastQueryBindings; + + expect($firstSql)->toBe($secondSql) + ->and($firstBindings)->toBe($secondBindings); + } + ); + + it('selectRaw throws InvalidColumnException when the expression contains a semicolon', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->selectRaw('1; DROP TABLE users--')) + ->toThrow(InvalidColumnException::class); + }); + + it('selectRaw throws InvalidColumnException when the expression contains a -- comment marker', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->selectRaw('1 -- comment')) + ->toThrow(InvalidColumnException::class); + }); + + it( + 'selectRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->selectRaw('/* comment */')) + ->toThrow(InvalidColumnException::class); + } + ); + + it('selectRaw throws InvalidColumnException when the expression contains a backtick', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->selectRaw('`name`')) + ->toThrow(InvalidColumnException::class); + }); + + it('selectRaw returns the builder for fluent chaining', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $result = $builder->table('users')->selectRaw("'x' AS col"); + + expect($result)->toBeInstanceOf(PgSqlQueryBuilder::class); + }); +}); + +describe('PgSqlQueryBuilder whereRaw', function (): void { + it('whereRaw appends the expression to the WHERE clause AND-combined with other conditions', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->where('status', '=', 'active') + ->whereRaw('LENGTH(name) > 3') + ->get(); + + expect($connection->lastQuerySql)->toBe('SELECT * FROM "users" WHERE "status" = ? AND LENGTH(name) > 3'); + }); + + it('whereRaw used alone (no prior where) emits "WHERE " with no leading AND', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->whereRaw('LENGTH(name) > 3') + ->get(); + + expect($connection->lastQuerySql)->toBe('SELECT * FROM "users" WHERE LENGTH(name) > 3'); + }); + + it('whereRaw can be called multiple times; expressions appear in call order, AND-combined', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->whereRaw('LENGTH(name) > 3') + ->whereRaw('LENGTH(email) > 5') + ->get(); + + expect($connection->lastQuerySql)->toBe('SELECT * FROM "users" WHERE LENGTH(name) > 3 AND LENGTH(email) > 5'); + }); + + it( + 'whereRaw together with where() emits the regular where condition first then the raw expression, AND-combined', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->where('id', '=', 1) + ->whereRaw('LENGTH(name) > 3') + ->get(); + + expect($connection->lastQuerySql)->toBe('SELECT * FROM "users" WHERE "id" = ? AND LENGTH(name) > 3'); + } + ); + + it( + 'whereRaw with bindings places its bindings in the WHERE position of the bindings array (after selectRaw bindings, after regular where bindings if both exist)', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->where('status', '=', 'active') + ->whereRaw('LENGTH(name) > ?', [3]) + ->get(); + + expect($connection->lastQueryBindings)->toBe(['active', 3]); + } + ); + + it('whereRaw bindings from multiple calls concatenate in call order', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->whereRaw('LENGTH(name) > ?', [3]) + ->whereRaw('LENGTH(email) > ?', [5]) + ->get(); + + expect($connection->lastQueryBindings)->toBe([3, 5]); + }); + + it('whereRaw throws InvalidColumnException when the expression contains a semicolon', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->whereRaw('1; DROP TABLE users--')) + ->toThrow(InvalidColumnException::class); + }); + + it('whereRaw throws InvalidColumnException when the expression contains a -- comment marker', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->whereRaw('1 -- comment')) + ->toThrow(InvalidColumnException::class); + }); + + it( + 'whereRaw throws InvalidColumnException when the expression contains a /* or */ block-comment marker', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->whereRaw('/* comment */')) + ->toThrow(InvalidColumnException::class); + } + ); + + it('whereRaw throws InvalidColumnException when the expression contains a backtick', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + expect(fn () => $builder->whereRaw('`name` = ?')) + ->toThrow(InvalidColumnException::class); + }); + + it('whereRaw returns the builder for fluent chaining', function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $result = $builder->table('users')->whereRaw('1 = 1'); + + expect($result)->toBeInstanceOf(PgSqlQueryBuilder::class); + }); + + it('count() honors whereRaw conditions (filtered count, not full-table count)', function (): void { + $connection = new MockConnection(queryReturn: [['aggregate' => 2]]); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->whereRaw('LENGTH(name) > ?', [3]) + ->count(); + + expect($connection->lastQuerySql)->toBe('SELECT COUNT(*) as aggregate FROM "users" WHERE LENGTH(name) > ?') + ->and($connection->lastQueryBindings)->toBe([3]); + }); + + it('min() / max() / sum() / avg() honor whereRaw conditions', function (): void { + $connection = new MockConnection(queryReturn: [['aggregate' => 10]]); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->whereRaw('LENGTH(name) > ?', [3]) + ->min('id'); + + expect($connection->lastQuerySql)->toBe('SELECT MIN("id") as aggregate FROM "users" WHERE LENGTH(name) > ?') + ->and($connection->lastQueryBindings)->toBe([3]); + }); +}); + +describe('PgSqlQueryBuilder selectRaw + whereRaw combined', function (): void { + it( + 'selectRaw and whereRaw used together produce bindings in [select-bindings..., where-bindings...] order in the final bindings array', + function (): void { + $connection = new MockConnection(); + $builder = new PgSqlQueryBuilder($connection); + + $builder + ->table('users') + ->selectRaw('? AS sel_val', [10]) + ->whereRaw('LENGTH(name) > ?', [3]) + ->get(); + + expect($connection->lastQueryBindings)->toBe([10, 3]); + } + ); + + it( + 'selectRaw and whereRaw flow correctly through compileSubquery() when this builder is used as a UNION right-hand side', + function (): void { + $leftConnection = new MockConnection(); + $rightConnection = new MockConnection(); + + $left = new PgSqlQueryBuilder($leftConnection); + $left->table('users')->select('name'); + + $right = new PgSqlQueryBuilder($rightConnection); + $right->table('admins') + ->select('name') + ->selectRaw('? AS extra', [99]) + ->whereRaw('LENGTH(name) > ?', [2]); + + $left->union($right)->get(); + + expect($leftConnection->lastQuerySql)->toBe( + '(SELECT "name" FROM "users") UNION (SELECT "name", ? AS extra FROM "admins" WHERE LENGTH(name) > ?)', + ) + ->and($leftConnection->lastQueryBindings)->toBe([99, 2]); + } + ); +}); diff --git a/packages/database/src/Query/QueryBuilderInterface.php b/packages/database/src/Query/QueryBuilderInterface.php index 1ece559b..4731a28f 100644 --- a/packages/database/src/Query/QueryBuilderInterface.php +++ b/packages/database/src/Query/QueryBuilderInterface.php @@ -29,6 +29,31 @@ public function table(string $table): static; */ public function select(string ...$columns): static; + /** + * Add a raw SQL expression to the SELECT list. + * + * The expression is appended after any columns added via select(). Include + * "AS alias" in the expression if you need a column alias. If no select() + * call precedes this, the default '*' column is preserved (emitting + * `SELECT *, `). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * Note: aggregate methods (count, min, max, sum, avg) build their own + * SELECT list and ignore selectRaw additions. + * + * @param string $expression Raw SQL select expression (e.g. "COALESCE(a, b) AS resolved") + * @param array $bindings Positional bindings for ? placeholders in the expression + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ + public function selectRaw( + string $expression, + array $bindings = [], + ): static; + /** * Enable DISTINCT selection to deduplicate result rows. * @@ -133,6 +158,29 @@ public function orWhere( mixed $value, ): static; + /** + * Add a raw SQL WHERE condition with optional positional bindings. + * + * The expression is AND-combined with any other where conditions, in call + * order, after the regular where()/whereIn/whereNull/etc. conditions. + * + * Aggregate methods (count, min, max, sum, avg) honor whereRaw conditions + * the same way they honor where(). + * + * Security: $expression must not contain semicolons, SQL comment markers, + * or backticks. Never interpolate user-supplied values directly — use ? + * placeholders and pass values via $bindings. + * + * @param string $expression Raw SQL WHERE expression (e.g. "COALESCE(price, base) > ?") + * @param array $bindings Positional bindings for ? placeholders + * @return static For fluent chaining + * @throws InvalidColumnException When the expression contains dangerous patterns + */ + public function whereRaw( + string $expression, + array $bindings = [], + ): static; + /** * Add an INNER JOIN clause. * @@ -218,6 +266,18 @@ public function orderBy( string $direction = 'ASC', ): static; + /** + * Add an ORDER BY clause with a raw SQL expression. + * + * @param string $expression The raw SQL expression to order by (e.g. a COALESCE expression) + * @param string $direction The sort direction (ASC or DESC) + * @return static For fluent chaining + */ + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static; + /** * Set the maximum number of rows to return. * diff --git a/packages/database/src/Repository/RepositoryQueryBuilder.php b/packages/database/src/Repository/RepositoryQueryBuilder.php index deb04d22..b739f252 100644 --- a/packages/database/src/Repository/RepositoryQueryBuilder.php +++ b/packages/database/src/Repository/RepositoryQueryBuilder.php @@ -9,6 +9,7 @@ use Marko\Database\Entity\EntityHydrator; use Marko\Database\Entity\EntityMetadata; use Marko\Database\Entity\RelationshipLoader; +use Marko\Database\Exceptions\InvalidColumnException; use Marko\Database\Exceptions\RepositoryException; use Marko\Database\Query\EntityQueryBuilderInterface; use Marko\Database\Query\QueryBuilderInterface; @@ -54,6 +55,18 @@ public function select( return $this; } + /** + * @throws InvalidColumnException + */ + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + $this->queryBuilder->selectRaw($expression, $bindings); + + return $this; + } + public function where( string $column, string $operator, @@ -120,6 +133,18 @@ public function orWhere( return $this; } + /** + * @throws InvalidColumnException + */ + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + $this->queryBuilder->whereRaw($expression, $bindings); + + return $this; + } + public function join( string $table, string $first, @@ -199,6 +224,15 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + $this->queryBuilder->orderByRaw($expression, $direction); + + return $this; + } + public function limit( int $limit, ): static { diff --git a/packages/database/tests/Entity/RelationshipLoaderBelongsToManyTest.php b/packages/database/tests/Entity/RelationshipLoaderBelongsToManyTest.php index e8e0e5bb..1ada1496 100644 --- a/packages/database/tests/Entity/RelationshipLoaderBelongsToManyTest.php +++ b/packages/database/tests/Entity/RelationshipLoaderBelongsToManyTest.php @@ -280,6 +280,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -535,6 +556,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -794,7 +836,7 @@ function makeBtmLoader(QueryBuilderFactoryInterface $factory, EntityMetadataFact $loader = makeBtmLoader($qbFactory, $metadataFactory); $loader->load([$post], $relationship, $postMeta); - expect($post->tags)->toBe([]); + expect($post->tags)->toBeEmpty(); }); it('resolves through pivot table using two queries', function (): void { @@ -1127,5 +1169,5 @@ function makeBtmLoader(QueryBuilderFactoryInterface $factory, EntityMetadataFact $loader->load([$post1, $post2], $relationship, $postMeta); expect($post1->tags)->toHaveCount(1) - ->and($post2->tags)->toBe([]); + ->and($post2->tags)->toBeEmpty(); }); diff --git a/packages/database/tests/Entity/RelationshipLoaderNestedTest.php b/packages/database/tests/Entity/RelationshipLoaderNestedTest.php index 95a02cbc..92a45c75 100644 --- a/packages/database/tests/Entity/RelationshipLoaderNestedTest.php +++ b/packages/database/tests/Entity/RelationshipLoaderNestedTest.php @@ -440,6 +440,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -924,5 +945,5 @@ public function create(): QueryBuilderInterface $tree = ['comments' => ['author' => []]]; $loader->loadNested([$post], $tree, $postMeta); - expect($post->comments)->toBe([]); + expect($post->comments)->toBeEmpty(); }); diff --git a/packages/database/tests/Entity/RelationshipLoaderTest.php b/packages/database/tests/Entity/RelationshipLoaderTest.php index 126c476a..b06aebe9 100644 --- a/packages/database/tests/Entity/RelationshipLoaderTest.php +++ b/packages/database/tests/Entity/RelationshipLoaderTest.php @@ -348,6 +348,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -581,6 +602,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -1059,7 +1101,7 @@ public function parse(string $entityClass): EntityMetadata $loader = makeLoader($qbFactory, $factory); $loader->load([$user], $relationship, $userMeta); - expect($user->posts)->toBe([]); + expect($user->posts)->toBeEmpty(); }); it('groups HasMany results by foreign key value', function (): void { @@ -1099,7 +1141,7 @@ public function parse(string $entityClass): EntityMetadata expect($user1->posts)->toHaveCount(3) ->and($user2->posts)->toHaveCount(1) - ->and($user3->posts)->toBe([]); + ->and($user3->posts)->toBeEmpty(); }); // ── Batch Query Optimization ─────────────────────────────────────────────────── @@ -1161,7 +1203,7 @@ public function parse(string $entityClass): EntityMetadata $loader = makeLoader($qbFactory, $factory); $loader->load([$user], $relationship, $userMeta); - expect($queries)->toHaveCount(0) + expect($queries)->toBeEmpty() ->and($user->country)->toBeNull(); }); @@ -1340,7 +1382,7 @@ function (): void { $loader->load([$author], $relationship, $authorMeta); expect($author->posts)->toBeInstanceOf(EntityCollection::class) - ->and($author->posts)->toHaveCount(0) + ->and($author->posts)->toBeEmpty() ->and($author->posts->isEmpty())->toBeTrue(); }, ); diff --git a/packages/database/tests/Entity/RelationshipValidationTest.php b/packages/database/tests/Entity/RelationshipValidationTest.php index 58233a20..4b2d4511 100644 --- a/packages/database/tests/Entity/RelationshipValidationTest.php +++ b/packages/database/tests/Entity/RelationshipValidationTest.php @@ -140,6 +140,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; diff --git a/packages/database/tests/Query/QueryBuilderInterfaceTest.php b/packages/database/tests/Query/QueryBuilderInterfaceTest.php index 74374315..63f43b63 100644 --- a/packages/database/tests/Query/QueryBuilderInterfaceTest.php +++ b/packages/database/tests/Query/QueryBuilderInterfaceTest.php @@ -278,6 +278,74 @@ expect($returnType?->getName())->toBe('array'); }); + it('QueryBuilderInterface declares a selectRaw method', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + + expect($reflection->hasMethod('selectRaw'))->toBeTrue(); + }); + + it('QueryBuilderInterface::selectRaw takes a string expression as the first parameter named "expression"', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('selectRaw'); + $params = $method->getParameters(); + + expect($params[0]->getName())->toBe('expression') + ->and($params[0]->getType()?->getName())->toBe('string'); + }); + + it('QueryBuilderInterface::selectRaw takes an array bindings as the second parameter named "bindings" with default []', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('selectRaw'); + $params = $method->getParameters(); + + expect($params[1]->getName())->toBe('bindings') + ->and($params[1]->getType()?->getName())->toBe('array') + ->and($params[1]->isDefaultValueAvailable())->toBeTrue() + ->and($params[1]->getDefaultValue())->toBeEmpty(); + }); + + it('QueryBuilderInterface::selectRaw returns static', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('selectRaw'); + $returnType = $method->getReturnType(); + + expect($returnType?->getName())->toBe('static'); + }); + + it('QueryBuilderInterface declares a whereRaw method', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + + expect($reflection->hasMethod('whereRaw'))->toBeTrue(); + }); + + it('QueryBuilderInterface::whereRaw takes a string expression as the first parameter named "expression"', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('whereRaw'); + $params = $method->getParameters(); + + expect($params[0]->getName())->toBe('expression') + ->and($params[0]->getType()?->getName())->toBe('string'); + }); + + it('QueryBuilderInterface::whereRaw takes an array bindings as the second parameter named "bindings" with default []', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('whereRaw'); + $params = $method->getParameters(); + + expect($params[1]->getName())->toBe('bindings') + ->and($params[1]->getType()?->getName())->toBe('array') + ->and($params[1]->isDefaultValueAvailable())->toBeTrue() + ->and($params[1]->getDefaultValue())->toBeEmpty(); + }); + + it('QueryBuilderInterface::whereRaw returns static', function (): void { + $reflection = new ReflectionClass(QueryBuilderInterface::class); + $method = $reflection->getMethod('whereRaw'); + $returnType = $method->getReturnType(); + + expect($returnType?->getName())->toBe('static'); + }); + it('defines min(), max(), sum(), avg() aggregate methods returning int|float|null', function (): void { $reflection = new ReflectionClass(QueryBuilderInterface::class); diff --git a/packages/database/tests/Query/QuerySpecificationTest.php b/packages/database/tests/Query/QuerySpecificationTest.php index 16fac6a7..43c842cb 100644 --- a/packages/database/tests/Query/QuerySpecificationTest.php +++ b/packages/database/tests/Query/QuerySpecificationTest.php @@ -132,6 +132,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -350,6 +371,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; diff --git a/packages/database/tests/Query/SpecEagerLoadCompositionTest.php b/packages/database/tests/Query/SpecEagerLoadCompositionTest.php index 27c7d362..526c3b46 100644 --- a/packages/database/tests/Query/SpecEagerLoadCompositionTest.php +++ b/packages/database/tests/Query/SpecEagerLoadCompositionTest.php @@ -188,6 +188,21 @@ public function orderBy(string $column, string $direction = 'ASC'): static return $this; } + public function orderByRaw(string $expression, string $direction = 'ASC'): static + { + return $this; + } + + public function selectRaw(string $expression, array $bindings = []): static + { + return $this; + } + + public function whereRaw(string $expression, array $bindings = []): static + { + return $this; + } + public function limit(int $limit): static { return $this; @@ -373,6 +388,21 @@ public function orderBy(string $column, string $direction = 'ASC'): static return $this; } + public function orderByRaw(string $expression, string $direction = 'ASC'): static + { + return $this; + } + + public function selectRaw(string $expression, array $bindings = []): static + { + return $this; + } + + public function whereRaw(string $expression, array $bindings = []): static + { + return $this; + } + public function limit(int $limit): static { return $this; @@ -648,6 +678,21 @@ public function orderBy(string $column, string $direction = 'ASC'): static return $this; } + public function orderByRaw(string $expression, string $direction = 'ASC'): static + { + return $this; + } + + public function selectRaw(string $expression, array $bindings = []): static + { + return $this; + } + + public function whereRaw(string $expression, array $bindings = []): static + { + return $this; + } + public function limit(int $limit): static { return $this; @@ -783,11 +828,9 @@ public function apply(EntityQueryBuilderInterface $builder): void it('eager-loads nested relationship paths declared by a spec (e.g. "author.profile")', function (): void { $postRows = [['id' => 1, 'title' => 'Hello', 'status' => 'published', 'author_id' => 5]]; $authorRows = [['id' => 5, 'name' => 'Alice', 'author_id' => 5]]; - $profileRows = [['id' => 10, 'bio' => 'Writer', 'author_id' => 5]]; $primaryBuilder = makeSpecStubBuilder($postRows); $authorBuilder = makeSpecStubBuilder($authorRows); - $profileBuilder = makeSpecStubBuilder($profileRows); // The loader uses a single factory — we use authorRows to cover the nested load $loader = makeSpecLoader($authorBuilder); diff --git a/packages/database/tests/Repository/RepositoryMatchingTest.php b/packages/database/tests/Repository/RepositoryMatchingTest.php index 3bb9c1da..44e73406 100644 --- a/packages/database/tests/Repository/RepositoryMatchingTest.php +++ b/packages/database/tests/Repository/RepositoryMatchingTest.php @@ -148,6 +148,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; diff --git a/packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php b/packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php index 714447c1..04017395 100644 --- a/packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php +++ b/packages/database/tests/Repository/RepositoryQueryBuilderEnhancedTest.php @@ -14,6 +14,7 @@ use Marko\Database\Entity\EntityMetadata; use Marko\Database\Entity\EntityMetadataFactory; use Marko\Database\Entity\RelationshipLoader; +use Marko\Database\Exceptions\InvalidColumnException; use Marko\Database\Query\QueryBuilderFactoryInterface; use Marko\Database\Query\QueryBuilderInterface; use Marko\Database\Query\QuerySpecification; @@ -83,6 +84,16 @@ function makeRqbStubBuilder(array $rows = []): QueryBuilderInterface /** @var array */ public array $orderByCalled = []; + /** @var array}> */ + public array $selectRawCalled = []; + + /** @var array}> */ + public array $whereRawCalled = []; + + public bool $selectRawShouldThrow = false; + + public bool $whereRawShouldThrow = false; + public function __construct(private readonly array $rows) {} public function table(string $table): static @@ -181,6 +192,39 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + if ($this->selectRawShouldThrow) { + throw InvalidColumnException::invalidColumn($expression); + } + + $this->selectRawCalled[] = ['expression' => $expression, 'bindings' => $bindings]; + + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + if ($this->whereRawShouldThrow) { + throw InvalidColumnException::invalidColumn($expression); + } + + $this->whereRawCalled[] = ['expression' => $expression, 'bindings' => $bindings]; + + return $this; + } + public function limit(int $limit): static { return $this; @@ -500,3 +544,125 @@ public function apply(QueryBuilderInterface $builder): void expect($result)->toBeInstanceOf(EntityCollection::class) ->and($result->isEmpty())->toBeTrue(); }); + +// ── selectRaw / whereRaw delegation ─────────────────────────────────────────── + +it('selectRaw delegates to the inner query builder with the same expression and bindings', function (): void { + $stub = makeRqbStubBuilder(); + $rqb = makeRqb($stub); + + $rqb->selectRaw('COALESCE(a, b) AS resolved', [1, 2]); + + expect($stub->selectRawCalled)->toBe([ + ['expression' => 'COALESCE(a, b) AS resolved', 'bindings' => [1, 2]], + ]); +}); + +it('selectRaw returns the wrapper for fluent chaining', function (): void { + $stub = makeRqbStubBuilder(); + $rqb = makeRqb($stub); + + $result = $rqb->selectRaw('COUNT(*) AS total'); + + expect($result)->toBeInstanceOf(RepositoryQueryBuilder::class); +}); + +it('selectRaw propagates InvalidColumnException from the inner builder', function (): void { + $stub = makeRqbStubBuilder(); + $stub->selectRawShouldThrow = true; + $rqb = makeRqb($stub); + + expect(fn () => $rqb->selectRaw('bad;expression'))->toThrow(InvalidColumnException::class); +}); + +it('whereRaw delegates to the inner query builder with the same expression and bindings', function (): void { + $stub = makeRqbStubBuilder(); + $rqb = makeRqb($stub); + + $rqb->whereRaw('COALESCE(price, base) > ?', [100]); + + expect($stub->whereRawCalled)->toBe([ + ['expression' => 'COALESCE(price, base) > ?', 'bindings' => [100]], + ]); +}); + +it('whereRaw returns the wrapper for fluent chaining', function (): void { + $stub = makeRqbStubBuilder(); + $rqb = makeRqb($stub); + + $result = $rqb->whereRaw('status = ?', ['active']); + + expect($result)->toBeInstanceOf(RepositoryQueryBuilder::class); +}); + +it('whereRaw propagates InvalidColumnException from the inner builder', function (): void { + $stub = makeRqbStubBuilder(); + $stub->whereRawShouldThrow = true; + $rqb = makeRqb($stub); + + expect(fn () => $rqb->whereRaw('bad;expression'))->toThrow(InvalidColumnException::class); +}); + +it('the wrapper can chain selectRaw and whereRaw alongside the existing methods (e.g. select.selectRaw.where.whereRaw.orderBy)', function (): void { + $stub = makeRqbStubBuilder([]); + $rqb = makeRqb($stub); + + $result = $rqb + ->select('id', 'name') + ->selectRaw('COALESCE(a, b) AS resolved', [1]) + ->where('status', '=', 'active') + ->whereRaw('score > ?', [50]) + ->orderBy('name', 'ASC'); + + expect($result)->toBeInstanceOf(RepositoryQueryBuilder::class) + ->and($stub->selectRawCalled)->toBe([ + ['expression' => 'COALESCE(a, b) AS resolved', 'bindings' => [1]], + ]) + ->and($stub->wheresCalled)->toBe(['status = active']) + ->and($stub->whereRawCalled)->toBe([ + ['expression' => 'score > ?', 'bindings' => [50]], + ]) + ->and($stub->orderByCalled)->toBe(['name ASC']); +}); + +it('a QuerySpecification that calls $builder->selectRaw(...) inside its apply() method works correctly when invoked via matching()', function (): void { + $rows = [['id' => 1, 'name' => 'Alice']]; + $stub = makeRqbStubBuilder($rows); + $rqb = makeRqb($stub); + + $spec = new class () implements QuerySpecification + { + public function apply(QueryBuilderInterface $builder): void + { + $builder->selectRaw('COALESCE(a, b) AS resolved', [42]); + } + }; + + $collection = $rqb->matching($spec); + + expect($collection)->toBeInstanceOf(EntityCollection::class) + ->and($stub->selectRawCalled)->toBe([ + ['expression' => 'COALESCE(a, b) AS resolved', 'bindings' => [42]], + ]); +}); + +it('a QuerySpecification that calls $builder->whereRaw(...) inside its apply() method works correctly when invoked via matching()', function (): void { + $rows = [['id' => 1, 'name' => 'Alice']]; + $stub = makeRqbStubBuilder($rows); + $rqb = makeRqb($stub); + + $spec = new class () implements QuerySpecification + { + public function apply(QueryBuilderInterface $builder): void + { + $builder->whereRaw('score > ?', [50]); + } + }; + + $collection = $rqb->matching($spec); + + expect($collection)->toBeInstanceOf(EntityCollection::class) + ->and($stub->whereRawCalled)->toBe([ + ['expression' => 'score > ?', 'bindings' => [50]], + ]); +}); diff --git a/packages/database/tests/Repository/RepositoryTest.php b/packages/database/tests/Repository/RepositoryTest.php index 917d0c94..23a0794b 100644 --- a/packages/database/tests/Repository/RepositoryTest.php +++ b/packages/database/tests/Repository/RepositoryTest.php @@ -129,7 +129,7 @@ class ExtenderRepository extends Repository $method = $reflection->getMethod('findAll'); $returnType = $method->getReturnType(); expect($method->isPublic())->toBeTrue() - ->and($method->getParameters())->toHaveCount(0) + ->and($method->getParameters())->toBeEmpty() ->and($returnType->getName())->toBe(EntityCollection::class); }); @@ -1200,6 +1200,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit( int $limit, ): static { diff --git a/packages/database/tests/Repository/RepositoryWithTest.php b/packages/database/tests/Repository/RepositoryWithTest.php index 48be297e..73a67872 100644 --- a/packages/database/tests/Repository/RepositoryWithTest.php +++ b/packages/database/tests/Repository/RepositoryWithTest.php @@ -197,6 +197,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -545,18 +566,11 @@ function makeWithRepository( describe('Eager Loading Integration', function (): void { it('passes loaded entities to RelationshipLoader', function (): void { $userRows = [['id' => 1, 'name' => 'Alice', 'country_id' => null]]; - // Track whether the query builder was used (indicating load was called) - $queryCalled = false; - $trackingBuilder = new class ($queryCalled) implements QueryBuilderInterface + $trackingBuilder = new class () implements QueryBuilderInterface { public bool $getCalled = false; - public function __construct(bool &$queryCalled) - { - // store reference not needed; use getCalled instead - } - public function table(string $table): static { return $this; @@ -651,6 +665,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -795,14 +830,10 @@ public function create(): QueryBuilderInterface }); it('returns null from find when entity not found without loading relationships', function (): void { - $queryCalled = false; - - $trackingBuilder = new class ($queryCalled) implements QueryBuilderInterface + $trackingBuilder = new class () implements QueryBuilderInterface { public bool $whereInCalled = false; - public function __construct(bool &$queryCalled) {} - public function table(string $table): static { return $this; @@ -897,6 +928,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; @@ -1034,14 +1086,10 @@ public function create(): QueryBuilderInterface it('loads multiple relationships when multiple names specified', function (): void { $userRows = [['id' => 1, 'name' => 'Alice', 'country_id' => null]]; - $callCount = 0; - - $countingBuilder = new class ($callCount) implements QueryBuilderInterface + $countingBuilder = new class () implements QueryBuilderInterface { public int $whereInCount = 0; - public function __construct(int &$callCount) {} - public function table(string $table): static { return $this; @@ -1136,6 +1184,27 @@ public function orderBy( return $this; } + public function orderByRaw( + string $expression, + string $direction = 'ASC', + ): static { + return $this; + } + + public function selectRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + + public function whereRaw( + string $expression, + array $bindings = [], + ): static { + return $this; + } + public function limit(int $limit): static { return $this; diff --git a/packages/database/tests/Repository/StringPrimaryKeyTest.php b/packages/database/tests/Repository/StringPrimaryKeyTest.php index faf30c33..3bfe7185 100644 --- a/packages/database/tests/Repository/StringPrimaryKeyTest.php +++ b/packages/database/tests/Repository/StringPrimaryKeyTest.php @@ -4,9 +4,7 @@ namespace Marko\Database\Tests\Repository; -use Marko\Database\Attributes\BelongsTo; use Marko\Database\Attributes\Column; -use Marko\Database\Attributes\HasMany; use Marko\Database\Attributes\Table; use Marko\Database\Connection\ConnectionInterface; use Marko\Database\Connection\StatementInterface; @@ -229,6 +227,21 @@ public function orderBy(string $column, string $direction = 'ASC'): static return $this; } + public function orderByRaw(string $expression, string $direction = 'ASC'): static + { + return $this; + } + + public function selectRaw(string $expression, array $bindings = []): static + { + return $this; + } + + public function whereRaw(string $expression, array $bindings = []): static + { + return $this; + } + public function limit(int $limit): static { return $this; diff --git a/tests/Integration/QueryBuilderRawConsistencyTest.php b/tests/Integration/QueryBuilderRawConsistencyTest.php new file mode 100644 index 00000000..78c7135f --- /dev/null +++ b/tests/Integration/QueryBuilderRawConsistencyTest.php @@ -0,0 +1,251 @@ + */ + public array $lastBindings = []; + + public function query(string $sql, array $bindings = []): array + { + $this->lastSql = $sql; + $this->lastBindings = $bindings; + + return []; + } + + public function execute(string $sql, array $bindings = []): int + { + $this->lastSql = $sql; + $this->lastBindings = $bindings; + + return 0; + } + + public function connect(): void {} + + public function disconnect(): void {} + + public function isConnected(): bool + { + return true; + } + + public function prepare(string $sql): StatementInterface + { + return new class () implements StatementInterface + { + public function execute(array $bindings = []): bool + { + return true; + } + + public function fetchAll(): array + { + return []; + } + + public function fetch(): ?array + { + return null; + } + + public function rowCount(): int + { + return 0; + } + }; + } + + public function lastInsertId(): int + { + return 0; + } + }; +} + +function applyFixture(QueryBuilderInterface $builder): void +{ + $builder + ->table('products') + ->select('id') + ->selectRaw('COALESCE(?, ?) AS resolved', ['a', 'b']) + ->where('active', '=', true) + ->whereRaw('price > ?', [100]); +} + +it('MySqlQueryBuilder and PgSqlQueryBuilder produce identical bindings array for the selectRaw + where + whereRaw fixture', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + expect($mysqlConnection->lastBindings) + ->toBe(['a', 'b', true, 100]) + ->and($pgsqlConnection->lastBindings) + ->toBe(['a', 'b', true, 100]); +}); + +it('the bindings array order is exactly [select-raw-bindings..., where-bindings..., where-raw-bindings...] in both drivers', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + $expected = ['a', 'b', true, 100]; + + expect($mysqlConnection->lastBindings[0])->toBe('a') + ->and($mysqlConnection->lastBindings[1])->toBe('b') + ->and($mysqlConnection->lastBindings[2])->toBeTrue() + ->and($mysqlConnection->lastBindings[3])->toBe(100) + ->and($pgsqlConnection->lastBindings[0])->toBe('a') + ->and($pgsqlConnection->lastBindings[1])->toBe('b') + ->and($pgsqlConnection->lastBindings[2])->toBeTrue() + ->and($pgsqlConnection->lastBindings[3])->toBe(100) + ->and($mysqlConnection->lastBindings)->toBe($expected) + ->and($pgsqlConnection->lastBindings)->toBe($expected); +}); + +it('both drivers include the raw select expression in the SELECT list, after the regular columns', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + $mysqlSql = $mysqlConnection->lastSql ?? ''; + $pgsqlSql = $pgsqlConnection->lastSql ?? ''; + + expect(str_contains($mysqlSql, 'COALESCE(?, ?) AS resolved'))->toBeTrue() + ->and(str_contains($pgsqlSql, 'COALESCE(?, ?) AS resolved'))->toBeTrue(); + + $mysqlIdPos = strpos($mysqlSql, 'id'); + $mysqlRawPos = strpos($mysqlSql, 'COALESCE'); + $pgsqlIdPos = strpos($pgsqlSql, 'id'); + $pgsqlRawPos = strpos($pgsqlSql, 'COALESCE'); + + expect($mysqlIdPos)->toBeLessThan($mysqlRawPos) + ->and($pgsqlIdPos)->toBeLessThan($pgsqlRawPos); +}); + +it('both drivers AND-combine the raw where expression with the regular where condition', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + $mysqlSql = $mysqlConnection->lastSql ?? ''; + $pgsqlSql = $pgsqlConnection->lastSql ?? ''; + + expect(str_contains($mysqlSql, 'AND price > ?'))->toBeTrue() + ->and(str_contains($pgsqlSql, 'AND price > ?'))->toBeTrue(); +}); + +it('both drivers throw InvalidColumnException for each denylist input (semicolon, --, /*, */, backtick) — parameterized via Pest\'s it(...)->with([...])', function (string $dangerous): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + expect(fn () => $mysql->selectRaw($dangerous))->toThrow(InvalidColumnException::class) + ->and(fn () => $pgsql->selectRaw($dangerous))->toThrow(InvalidColumnException::class); + + $mysql2 = new MySqlQueryBuilder($mysqlConnection); + $pgsql2 = new PgSqlQueryBuilder($pgsqlConnection); + + expect(fn () => $mysql2->whereRaw($dangerous))->toThrow(InvalidColumnException::class) + ->and(fn () => $pgsql2->whereRaw($dangerous))->toThrow(InvalidColumnException::class); +})->with([';', '--', '/*', '*/', '`']); + +it('the emitted SQL contains the same SELECT-list ordering across both drivers (raw expression appended after regular columns)', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + $mysqlSql = $mysqlConnection->lastSql ?? ''; + $pgsqlSql = $pgsqlConnection->lastSql ?? ''; + + $mysqlNormalized = str_replace(['`', '"'], '', $mysqlSql); + $pgsqlNormalized = str_replace(['`', '"'], '', $pgsqlSql); + + $mysqlSelectList = (string) preg_replace('/SELECT\s+(.*?)\s+FROM.*/s', '$1', $mysqlNormalized); + $pgsqlSelectList = (string) preg_replace('/SELECT\s+(.*?)\s+FROM.*/s', '$1', $pgsqlNormalized); + + expect($mysqlSelectList)->toBe($pgsqlSelectList); +}); + +it('the emitted SQL contains the same WHERE-clause ordering across both drivers (regular where first, then whereRaw, AND-combined)', function (): void { + $mysqlConnection = makeRecordingConnection(); + $pgsqlConnection = makeRecordingConnection(); + + $mysql = new MySqlQueryBuilder($mysqlConnection); + $pgsql = new PgSqlQueryBuilder($pgsqlConnection); + + applyFixture($mysql); + applyFixture($pgsql); + + $mysql->get(); + $pgsql->get(); + + $mysqlSql = $mysqlConnection->lastSql ?? ''; + $pgsqlSql = $pgsqlConnection->lastSql ?? ''; + + $mysqlNormalized = str_replace(['`', '"'], '', $mysqlSql); + $pgsqlNormalized = str_replace(['`', '"'], '', $pgsqlSql); + + $mysqlWherePos = strpos($mysqlNormalized, 'WHERE'); + $pgsqlWherePos = strpos($pgsqlNormalized, 'WHERE'); + + $mysqlWhere = $mysqlWherePos !== false ? substr($mysqlNormalized, $mysqlWherePos) : ''; + $pgsqlWhere = $pgsqlWherePos !== false ? substr($pgsqlNormalized, $pgsqlWherePos) : ''; + + expect($mysqlWhere)->toBe($pgsqlWhere); +});