Skip to content

feat: extend utopia-php/query, add Query::cursor and count($max)#113

Merged
abnegate merged 5 commits intomainfrom
feat-clickhouse-cursor-and-count-max
May 5, 2026
Merged

feat: extend utopia-php/query, add Query::cursor and count($max)#113
abnegate merged 5 commits intomainfrom
feat-clickhouse-cursor-and-count-max

Conversation

@lohanidamodar
Copy link
Copy Markdown
Contributor

Summary

  • The audit Query class becomes a thin extension of Utopia\Query\Query, so audit consumers share the canonical query type — including cursorAfter / cursorBefore — with the rest of the utopia ecosystem. Audit's lenient single-value factory signatures (equal, lessThan, greaterThan, between) are preserved as overrides so existing callers keep working with mixed values (including DateTime on the time column). The custom Query::in() alias is replaced by the inherited Query::contains().
  • The ClickHouse adapter gains keyset-pagination cursor support. Cursor values accept a Log/ArrayObject or a plain associative array; an id tiebreaker is auto-appended to ORDER BY for deterministic pagination on non-unique columns; cursorBefore flips direction at SQL build time and reverses results post-fetch.
  • count() and the four countBy* helpers (plus their abstract definitions and Audit facade wrappers) accept an optional ?int $max argument. When non-null, the ClickHouse path uses a bounded LIMIT subquery so very large tables can short-circuit; the Database path forwards $max to utopia-php/database's existing $max parameter. The countBy* helpers also stop loading rows just to count them — they call count() directly now.

Test plan

  • composer lint passes
  • composer check (PHPStan level max) passes
  • tests/Audit/Adapter/ClickHouseTest.php passes — 39/39, including 5 new cursor + count($max) tests
  • tests/Audit/QueryTest.php passes — 11/11
  • Database adapter tests verified in CI (local environment lacks PDO MySQL driver)

Notes

The transitive PHP requirement is now >=8.4 because utopia-php/query 0.1.x requires it; the explicit >=8.0 in composer.json is preserved for consistency with other utopia libraries (composer resolves the actual minimum from the transitive dep).

🤖 Generated with Claude Code

The audit Query class becomes a thin extension of `Utopia\Query\Query`,
which gives audit consumers a single canonical query type that includes
cursorAfter / cursorBefore. The custom `Query::in()` alias is removed in
favour of the inherited `Query::contains()`. Audit's lenient single-value
factory signatures for equal/lessThan/greaterThan/between are preserved by
overriding the base factories so existing callers keep working with mixed
values (including DateTime on the time column).

The ClickHouse adapter gains keyset-pagination cursor support: cursor rows
accept Log/ArrayObject or plain arrays, an `id` tiebreaker is auto-appended
to ORDER BY for deterministic pagination on non-unique columns, and
cursorBefore flips direction at SQL build time then reverses results.

count() and the four countBy* helpers (plus their Adapter abstracts and
Audit facade wrappers) accept an optional `?int $max` argument. When
non-null, ClickHouse uses a bounded subquery so very large tables can
short-circuit. The Database adapter forwards $max to utopia-php/database's
existing $max parameter. The countBy* helpers also stop loading rows just
to count them — they call count() directly now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 26, 2026

Greptile Summary

This PR makes Utopia\Audit\Query a thin extension of utopia-php/query, adds keyset cursor pagination (cursorAfter/cursorBefore) to the ClickHouse adapter, and introduces an optional ?int $max bound on all count / countBy* methods. The implementation is well-structured: a new getParamType/formatTypedValue layer centralises column-type–aware parameter binding, resolveCursorOrder auto-appends an id tiebreaker for deterministic pages, and cursorBefore correctly flips ORDER BY direction at SQL build time then reverses results post-fetch.

Confidence Score: 5/5

Safe to merge; only a P2 suggestion about guarding a negative $max value.

No P0/P1 findings. The single P2 (missing non-positive guard on $max) requires a deliberate bad call and results in a clear exception rather than silent data corruption. All previously flagged issues (null DateTime cursor, $id/id key cleanup, empty-values silent full-scan) have been addressed in this revision.

src/Audit/Adapter/ClickHouse.php — the new cursor and bounded-count paths are the largest surface area.

Important Files Changed

Filename Overview
src/Audit/Adapter/ClickHouse.php Adds cursor-based keyset pagination, bounded count($max), and a type-aware param-binding layer (getParamType/formatTypedValue). One minor issue: $max is not validated as positive before being bound to a UInt64 parameter.
src/Audit/Query.php Refactored to extend Utopia\Query\Query, preserving legacy mixed-value factory overrides for equal, lessThan, greaterThan, and between; removes the standalone in() alias.
src/Audit/Adapter/Database.php Adds ?int $max to all four countBy* methods and count(), forwarding it to utopia-php/database's count() max parameter; also skips cursor queries in count translation.
tests/Audit/Adapter/ClickHouseTest.php Adds 11 new test cases for cursor pagination, bounded count, and new query types; ClickHouse connection now env-configurable. Tests rely on setUp/cleanup isolation correctly inherited from AuditBase.

Reviews (5): Last reviewed commit: "fix: reject empty values for filter meth..." | Re-trigger Greptile

Comment thread src/Audit/Adapter/ClickHouse.php Outdated
Comment thread src/Audit/Adapter/ClickHouse.php Outdated
Comment thread src/Audit/Adapter/ClickHouse.php
Comment thread src/Audit/Adapter/ClickHouse.php Outdated
lohanidamodar and others added 3 commits April 26, 2026 05:12
utopia-php/query 0.1.x already requires PHP 8.4; making it explicit here
matches the actual transitive requirement and aligns with appwrite/appwrite
and cloud which already run on 8.4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four small hardening fixes from the greptile review:

- Drop the always-true `!empty($orderAttributes)` guard in the cursor
  branch — resolveCursorOrder() always appends an `id` tiebreaker, so the
  guard was dead code and could mislead future readers.
- normalizeCursorRow now removes `$id` after copying it to `id` so the
  cursor state no longer carries both keys.
- Throw an explicit Exception when a cursor value is null. The previous
  path silently routed null `time` cursors through formatDateTime(null)
  which returns the current timestamp, turning a misconfigured cursor into
  a silent correctness bug (filtering on `time < now()` and skipping rows).
- Replace the hard-coded `:String` type binding for non-`time` cursor
  attributes with a schema-driven getCursorParamType() helper that maps
  audit's VAR_STRING / VAR_DATETIME types to their ClickHouse equivalents
  and throws on unsupported types. This guards against silently misordering
  pages if a numeric column is added to the schema later — binding such a
  value as String would produce lexicographic comparisons ("9" > "10").

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds notEqual, lessThanEqual, greaterThanEqual, notContains, notBetween,
isNull, isNotNull, startsWith, endsWith — bringing the audit ClickHouse
adapter to parity with the usage adapter. startsWith / endsWith use
ClickHouse's built-in functions of the same name; isNull / isNotNull emit
`IS NULL` / `IS NOT NULL` without binding; the comparison helpers follow
the existing param-bound pattern.

Two structural changes make the shared parseQueries logic consistent
between this adapter and utopia-php/usage's ClickHouse adapter:

- getParamType() centralises the column → ClickHouse-type mapping
  (time → DateTime64(3), tenant → UInt64 when sharedTables, default →
  String). Previously TYPE_EQUAL and TYPE_CONTAINS hard-coded `:String`
  for every column — including `time` — so `Query::equal('time', ...)`
  bound a DateTime as String and would fail or produce wrong results.
- formatTypedValue() routes DateTime-typed values through formatDateTime
  and everything else through formatParamValue, so each case body has one
  code path.
- buildCursorWhere() uses the same dispatch and getCursorParamType is
  removed — getParamType is now strict-enough on its own (whitelist of
  known typed columns; everything else is String).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/Audit/Adapter/ClickHouse.php
Mirrors the validator pattern in utopia-php/database
(Validator/Query/Filter.php): contains/notContains/equal/etc. queries
must have at least one value; an empty values array is rejected up front
with `<Method> queries require at least one value.` instead of silently
producing a "no filter applied" WHERE clause.

Without the guard, `Query::contains('event', [])` would skip the IN
clause entirely and return all rows — exactly the opposite of the
intended IN () semantics, which should match nothing. This was the P1
flagged in greptile's review of cd5112f.

Defines a VALUE_REQUIRED_METHODS allow-list checked at the top of the
parseQueries loop, before the per-method switch. The same guard is being
applied to utopia-php/usage's ClickHouse adapter so both libraries
reject the same empty-value cases consistently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@abnegate abnegate merged commit 7c9f09e into main May 5, 2026
4 checks passed
@abnegate abnegate deleted the feat-clickhouse-cursor-and-count-max branch May 5, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants