fix(pipes): run write pipes every call, uncached and uncoalesced

[EricAndrechek]

Fixes #386. Part of #613.

Stacked on #614 (feat/cache-snapshot).

What changes

A pipe whose SQL is a write went to ClickHouse through Exec. Its [] was still cached, and identical calls in flight were coalesced. So a repeat within the TTL (10 s to 1 h) answered 200 without writing, and N concurrent identical calls became one write. With cache.backend: redis (#630), that cached [] would answer every instance.

PipesHandler.Execute now runs IsMutation on the bound SQL. A write branches off to executeWrite before the cache lookup, so it skips the lookup, the fill and singleflight. It takes the tenant's pool itself (no pool is still a 503 with Retry-After: 30, before anything is sent), executes on every call and answers X-Cache: BYPASS with Cache-Control: no-store, so an HTTP cache in front of a GET write pipe can't answer a repeat either. Reads are unchanged and keep #614's order: cache lookup, then the pool, whose absence answers 503 before a hit is served. The execute-and-marshal body moved into PipesHandler.run, which both paths share.

A failed write answers with #627's classification, status and code, but always retryable: false and with no Retry-After, 503 clickhouse.unavailable included (writeCHWriteError). Once the statement is sent it may have run, and the TS SDK retries both a 5xx with no retryable: false and any 503 that carries Retry-After.

How a pipe is identified as a write

The handler uses IsMutation, the classifier executeCHQuery already uses to send a statement to Exec instead of Query. It reads the leading keyword and skips whitespace, comments, string literals, quoted identifiers (curly-quoted ones included) and heredocs the way ClickHouse's lexer does. After a WITH list ClickHouse accepts only SELECT, a FROM-first SELECT or INSERT INTO (measured on 26.6.3.62: every other statement verb there is a SYNTAX_ERROR), so a WITH-led statement is a write exactly when it holds INSERT INTO outside parentheses. The classifier never tries to tell which other word starts the statement, because a WITH list's names can be spelled like any keyword. A number led by . ends at its digits and exponent, as ClickHouse's lexer ends it, so .5INSERT is .5 then INSERT. An EXECUTE AS <user> prefix is looked through, and the statement after it is classified by the same rules; a bare EXECUTE AS <user> returns no result set and goes through Exec.

• Why not a declared pipe property ("mutation": true): an operator who forgets it reintroduces bug(pipes): mutation pipe results are cached and coalesced — the write silently drops on repeat calls #386 without any error. Classifying automatically means nothing has to be declared.
• Why not ClickHouse's own classification (EXPLAIN, query_kind): that costs a round trip on every call, because the SQL has {{param}} placeholders until it is bound. query_kind in system.query_log is only known after the statement has run. Instead, the integration suite checks the classifier against ClickHouse's parser (EXPLAIN AST), so a disagreement fails CI rather than a call.
• Why this classifier and not a new parser: the statements that skip the cache are exactly the statements that run as writes, so the two cannot disagree. A write IsMutation misses is worse than uncached: it goes through conn.Query, which runs it and then fails the call (measured: clickhouse-go v2.48.0 native returns EOF after the row is inserted), and a client that retries the 5xx writes again.

Classification runs on the bound SQL, the same string Exec receives. A value bound to a bare placeholder is an escaped literal and cannot change the statement; a quoted placeholder is unsafe and is now documented as such (guard: #662).

Invalidating what a write pipe writes: left to #394

A write pipe still does not invalidate cached reads of the table it writes. This PR leaves that to #394 and does not fold #394 in:

• A pipe result has no table deps until feat(pipes): resolve table dependencies for cache invalidation #343. Bumping a table's version would not reach cached read pipes. It would reach only structured-query results, and those are bug(query): admin mutations via /v1/admin/query don't invalidate the structured-query cache — stale reads for up to an hour #394's surface.
• bug(query): admin mutations via /v1/admin/query don't invalidate the structured-query cache — stale reads for up to an hour #394 needs table extraction from arbitrary SQL for /v1/ops/query, the HTTP proxy that does not classify statements. A write pipe needs the same extraction. One fix should cover both, so I widened #394 to include the pipe surface.
• A tenant-wide InvalidateTenant on every write pipe would work, but it would empty the tenant's whole cache on every audit-log insert. That is a policy decision, not part of this bug fix.

Write-pipe rows also don't reach /v1/stream subscribers, because only /v1/ingest publishes to the queue. That is the same root cause as #362. The docs now say both.

Tests

• TestPipesHandler_Execute_MutationRunsEveryCall: INSERT, WITH … INSERT and ALTER … DELETE pipes, each called 3 times over a real LocalCache. Result: 3 Exec calls, 0 Query calls, X-Cache: BYPASS, Cache-Control: no-store and [] every time.
• TestPipesHandler_Execute_ConcurrentMutationsNotCoalesced: under synctest, 3 identical calls held inside Exec together. Result: 3 writes in flight.
• TestPipesHandler_Execute_ReadPipeStaysCached: SELECT * FROM insert_log …, whose table name starts with a write verb, gives MISS, HIT, HIT with 1 query.
• TestPipes_WriteClickHouseErrors: every row of fix(api): map ClickHouse query failures by class, not HTTP status #627's error table on a write pipe gives the read's status and code, retryable: false, no Retry-After, and exactly 1 Exec. TestClickHouseRoutes_NoPoolIs503/write_pipe_execute: no pool is a 503 with Retry-After: 30.
• TestIsMutation runs the 152 cases in internal/testutil/mutationtest, 88 of them new in this PR: writes and the reads they must not catch (backslash escapes in '…', "…" and backticks; curly quotes; nested block comments; // comments; \v, \f, NBSP and BOM; heredocs; $ in a bareword; _-led words; a leading bareword such as insert_log read whole; a WITH list's names spelled like keywords, as a CTE, an alias, a function, a lambda parameter, an operand, a qualified name's part, an array element or a bare element; a .-led number glued to INSERT; EXECUTE AS ahead of writes and reads, with a bare, quoted, backticked, curly-quoted or heredoc user and an @host). TestExecuteCHQuery_* sends EXECUTE AS … INSERT to Exec and EXECUTE AS … SELECT to Query. TestIsMutation_ClickHouseWhitespace covers every whitespace character ClickHouse accepts in four positions.
• TestIsMutation_AgreesWithClickHouseParser (integration): each case goes through EXPLAIN AST on the pinned ClickHouse, and its root node must map to the case's answer; an ExecuteAsQuery root is mapped by the statement it runs, and a bare one counts as a write. The 15 cases ClickHouse rejects (among them WITH … DELETE, WITH … ALTER, WITH … TRUNCATE and REPLACE INTO) are marked Unparsed and must still fail with SYNTAX_ERROR, so a ClickHouse that starts accepting one fails the test.
• TestIsMutation_KeywordNamesInWithList (integration): every word in system.keywords (498) in 12 WITH shapes, ahead of SELECT, FROM-first SELECT and INSERT INTO. That is 17,928 statements, of which 17,860 parse, in about 2.5 s under -race, and IsMutation must agree with the parser on each.
• Mutation checks:
  • disabling the bypass (if false && IsMutation(sql)) fails both write tests, while the read test still passes;
  • against an early version of the classifier, 18 of the first 19 new TestIsMutation rows (the 19th, a read, passes either way) and every non-ASCII-space case fail; against the classifier before the heredoc fix, all 6 heredoc/$ rows fail; against an earlier version of the classifier, 7 of its 8 rows fail (the 8th, a read, passes either way), and both leading-bareword rows fail against an earlier commit;
  • answering a write failure with the read path's writeCHError fails 7 subtests of TestPipes_WriteClickHouseErrors (the retryable ones), and a bare 500 fails all 15;
  • with the .-number case disabled, all 5 .-led rows fail (allowing no _ fails the separator row); with the EXECUTE AS look-through disabled, the 12 EXECUTE AS write rows and the Exec dispatch test fail, and classifying every EXECUTE AS as a write fails the 8 read rows and the Query dispatch test;
  • against an early version of the classifier, 26 TestIsMutation rows fail and TestIsMutation_KeywordNamesInWithList reports 6,066 disagreements (76 writes read as reads, 5,990 reads as writes); skipping only ASCII spaces between INSERT and INTO fails the whitespace test.
• make ci passed at every commit in this PR's history through the shared queue.

Docs

pipes.mdx has a "Pipes that write" section after the error table. It lists the write verbs the classifier recognizes, says a WITH list may lead only INSERT INTO as a write and that EXECUTE AS is looked through, and says what a write led by any other verb costs (#666): BACKUP and RESTORE return rows, so they are cached and coalesced; UNDROP and MOVE return none, so the call fails after the statement has run, and the SDK may retry it. It says that allowed_roles is a write pipe's only gate, default_role included, that a failed write is not retried automatically, since it may have run (with the SDK's remaining retries: a dropped connection, or a proxy's 502/503/504), and that no write invalidates a read pipe's cached result (#343). Its cached-path wording mentions the bypass. "How a value becomes SQL" now says to write each placeholder bare, never inside quotes. The pipe response in api.md covers X-Cache: BYPASS, and its ClickHouse-errors section and pipe error table cover the write answer, as do sdk/pipes.md, the SDK error reference and configuration.mdx. Four places said /v1/ops/query was the only way to run a non-insert write: the Insert-only note and the /v1/ops/query section in api.md, the two request-flow notes in architecture.md, and the ingest/ bullet in AGENTS.md. Each now also names an operator-authored write pipe. ingest-pipeline.md names write pipes as a mutation path too. settings-directory.mdx says clickhouse.query_timeout bounds write pipes and /v1/ops/query too. The pipes.go and ch_errors.go entries in architecture.md, the AGENTS.md api/ bullet, invariant 13 and Testing Conventions, and the CHANGELOG (Fixed, and the placeholder correction under Security) are updated too. make build-docs passes, links included.

Deliberately left to later PRs

• Invalidating the tables a write pipe writes (bug(query): admin mutations via /v1/admin/query don't invalidate the structured-query cache — stale reads for up to an hour #394, plus feat(pipes): resolve table dependencies for cache invalidation #343 for read pipes).
• Feeding write-pipe rows to /v1/stream (SSE/live-query yields no events for tables populated by a ClickHouse MATERIALIZED VIEW (only ingest-API writes republish to NATS) #362).
• A guard against quoted placeholders in BindParams (security(pipes): a quoted placeholder lets a value break out of its literal #662).
• Answering 405 to a GET on a write pipe (bug(pipes): a write pipe answers GET, which proxies and clients replay #663).
• Classifying BACKUP, RESTORE, UNDROP and MOVE pipes as writes (bug(pipes): a BACKUP/RESTORE/UNDROP pipe runs as a read and is cached #666).
• deployment.md's "Multiple instances and the shared cache" section in feat(config): cache.backend=redis selects the shared cache #630 describes this bug as open. Whichever of feat(config): cache.backend=redis selects the shared cache #630 and this PR merges second should update that paragraph.

🤖 Generated with Claude Code

https://claude.ai/code/session_017aS7rLrH1RKkUMem7X4ckd

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: aa8ac3c2-0697-4d47-af25-136f7c75f485

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}