Repository navigation
fix: let read-only users query Recall on ClickHouse and read each table once - #1218
Merged
Merged
Conversation
The ClickHouse invocations view read recall_events three times and joined four subqueries, so every lookup paid for three event reads and hash-join preallocation. It is now one UNION ALL of the context rows, heartbeats and lifecycle events folded by invocation, without joins. Rows and column types are unchanged; the Recall schema version rises to 3 so writers replace the view through the existing upgrade path. Refs: CHI-773
The statements view read the events twice, once for statement facts and once for resource names. It now folds one read by resource, which names the resource and gathers its statement events, then by statement. sql_available stays a lookup of the stored SQL, which is shared across invocations. Refs: CHI-773
A read connection sent SQLBuild's session settings and every recall query set readonly = 2, which a readonly = 1 user may not do. A read connection now applies the pinned session settings only when the session may change settings; a query sets readonly = 2 only when the session is not already read-only, sends the row limit only when the user may set it and otherwise stops reading at the cap, and a setting the user may not change fails with an error naming it and readonly = 2. output_invocations is folded without joins and artifacts makes its stored side nullable before the join, so no view depends on join_use_nulls or sets SETTINGS. Refs: CHI-772
Copies of a Recall key hold the same content, since a write with other content is refused as a conflict. The invocations and statements views only take the first, last, smallest or largest value of a key's copies, so they read without FINAL, as do the delivery and context branches of output_invocations. Views that list or count rows keep FINAL. Refs: CHI-777
A long command wrote about one small part per table every second. ClickHouse batches now wait up to five seconds; heartbeats still go out at once and a closing command sends everything it holds. A batch is also sent early once a quarter of the queue's byte capacity is ready, so a byte-heavy command fills the queue no sooner than before. Refs: CHI-777
kvlonge
enabled auto-merge (squash)
October 22, 2026 12:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Recall reads failed for ClickHouse
readonly = 1users.invocationsreadrecall_eventsthree times with memory-heavy joins,statementstwice; every view usedFINAL; long commands wrote many small inserts.Changes
readonly = 1).recall querysetsreadonly = 2only if the session is not read-only, and caps rows while reading when it may not setlimit; a setting it may not change fails naming it andreadonly = 2.invocationsandoutput_invocationsare oneUNION ALLfolded by invocation, without joins;statementsfolds one events read by resource, then by statement;artifactsnulls its stored side before the join. No view setsSETTINGSor depends onjoin_use_nulls.invocations,statementsand the delivery/context reads ofoutput_invocationsskipFINAL(their folds ignore repeated copies); listing and counting views keep it.--ddlprints them. Local SQLite stores recreate their unchanged views once, and new rows on every backend carryschema_version = 3.Closes CHI-772, CHI-773 and items 1-2 of CHI-777. Item 3 (LowCardinality) measured at 4% smaller tables, so not included.
Verification
readonly = 1,readonly = 2and normal readers (settings, every view, schema,--ddl, output); rewritten views equal the previous ones (EXCEPT both ways, same DESCRIBE) and are unchanged withjoin_use_nulls = 0and duplicated rows; each table read once by its key; upgrade from version 2.make check-ci.