Skip to content

fix: several bugs found while writing the docs - #57

Merged
kbond merged 6 commits into
zenstruck:1.xfrom
kbond:fix/tabled-bugs
Aug 20, 2026
Merged

kbond merged 6 commits into
zenstruck:1.xfrom
kbond:fix/tabled-bugs

Conversation

@kbond

@kbond kbond commented Aug 20, 2026

Copy link
Copy Markdown
Member

Fixes the bugs found while writing the README (#3), each with tests that fail without the fix.

  • readonly() was a no-op when iterating an EntityResult. getIterator() applied only the result modifier, not normalizeResult() - so entities were detached via first()/take()/eager() but not via foreach.
  • LIKE values weren't escaped. str_replace('%', '%%') isn't SQL escaping: the value is a bound parameter, so % and _ reached the database as wildcards (Spec::contains('value', '%') matched every row). They're now escaped with an ESCAPE clause, leaving * as the only wildcard.
  • DoctrineBridgeCollection::filter() checked the wrong variable ($this->inner instanceof Criteria rather than $specification), making the LogicException unreachable and surfacing a confusing InvalidSpecification instead.
  • EntityResult::count() memoization survived cloning, so every derived result inherited the original's count.
  • CriteriaInterpreter didn't guard the Spec::callback() return value. Returning the Criteria (as andWhere() does) violated transform()'s own return type. The callback may now either modify the Criteria or return an Expression; both are tested. This also adds the first coverage for Spec::callback().

The last commit documents the behavior these change.

Note: making _ literal is technically a behavior change for anyone relying on it as a single-character wildcard - it was undocumented and almost certainly unintended.

kbond added 6 commits August 20, 2026 13:26
`getIterator()` only applied the result modifier, so `readonly()` was a no-op for direct iteration (it worked via `first()`/`take()`/`eager()`).
`count()` memoizes, but `__clone()` only cloned the query builder - so every derived result inherited the original's count.
`%` and `_` in a `contains()`/`startsWith()`/`endsWith()` value were passed through to the LIKE pattern, where they acted as wildcards. They're now escaped via an ESCAPE clause, leaving `*` as the only wildcard.
… `Criteria`

The guard tested `$this->inner instanceof Criteria`, which is never true, so passing a `Criteria` to a non-`Selectable` collection threw `InvalidSpecification` instead of explaining the real problem.
The callback can modify the `Criteria` directly, but returning it (as `andWhere()` does) made `transform()` violate its own return type.
Covers literal `%`/`_` in LIKE specifications, `readonly()` when iterating, and modifying the `Criteria` from `Spec::callback()`.
@kbond
kbond marked this pull request as ready for review August 20, 2026 17:34
@kbond
kbond merged commit 9468f8a into zenstruck:1.x Aug 20, 2026
18 checks passed
@kbond
kbond deleted the fix/tabled-bugs branch August 20, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant