Split multi-statement SQL for cloud service query - #622
Conversation
ecc60f9 to
4305092
Compare
`cloud service query --queries-file` sent an entire `.sql` script as one `sql` body, and the Query API runs exactly one statement per request: ClickHouse answered a multi-statement script with error 62, `Multi-statements are not allowed`. The flag is plural and a `.sql` file is the obvious thing to hand it, so the CLI now splits the SQL instead of documenting the limitation. The Query API has no native multi-statement mode to prefer here — its body carries a single `sql` string. New `cloud::sql_statements` scans the script lexically for the `;` characters that really are separators: single-quoted literals, double-quoted and backtick-quoted identifiers (with `\'` and `''` escapes), `--` and `/* */` comments, and `$tag$` heredocs are all skipped over, a trailing `;` yields no statement, and whitespace/comment-only chunks are dropped. Statements are otherwise sent verbatim. `service_query` executes them in order and stops at the first failure, naming it (`statement 2 of 5 failed: ...`); single-statement runs report errors exactly as before. Credential resolution — including lazy Query API endpoint and key provisioning — happens once on the first statement and is reused for the rest, so a script cannot re-provision per statement. Splitting applies to `--query` and stdin too, so all three sources behave alike. Tests: 22 splitter unit tests (quotes, escapes, comments containing separators, heredocs, unterminated constructs, multibyte, trailing separator, a realistic schema script), unit tests for the statement error prefix, and wiremock subprocess tests covering one request per statement in order, stop-at-first-failure with the third statement never sent, no prefix for a single statement, and a comment-only script rejected before any network access. Fixes #610 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The splitter treated block comments as non-nesting and only knew the `--` line-comment introducer. ClickHouse's lexer nests /* */ comments per the SQL standard and also starts line comments with `//`, `# ` (hash followed by a space), and `#!`, so a `;` inside any of those was taken as a statement separator and commented-out SQL could be sent to the server as its own statement. skip_block_comment now tracks nesting depth, and the scanner recognizes the three additional line-comment introducers. A bare `#` followed by anything else is a lexer error in ClickHouse, not a comment, so it stays an ordinary character. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4305092 to
70f7ad5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 70f7ad5. Configure here.
| } | ||
|
|
||
| statements | ||
| } |
There was a problem hiding this comment.
FORMAT payloads split on inner semicolons
High Severity
The splitter treats every top-level ; as a statement boundary, including those in the raw payload after INSERT ... FORMAT (CSV, TSV, JSONEachRow, and similar). A single insert that previously went to the Query API as one body is now several requests, so the first fragment can apply truncated rows and later fragments run as SQL.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 70f7ad5. Configure here.


What
cloud service query --queries-filesent a whole.sqlscript as a singlesqlbody, so ClickHouse rejected it with error 62 (Multi-statements are not allowed) and only single-statement files worked.The Cloud Query API has no native multi-statement mode to prefer instead:
POST /service/{id}/runcarries onesqlstring, and the server-side parser is what rejects the second statement. So the CLI splits the script.crates/clickhousectl/src/cloud/sql_statements.rs— a lexical scanner that finds only the;characters that really are separators. It skips over single-quoted literals, double-quoted and backtick-quoted identifiers (handling both\'and''escapes),--line comments,/* */block comments, and$tag$heredocs. A trailing;produces no statement, whitespace/comment-only chunks are dropped, and everything else is sent verbatim (comments included). Unterminated quotes/comments/heredocs run to the end of input so the server reports the syntax error on the text the user actually wrote.service_queryexecutes the statements sequentially and stops at the first failure, naming it:statement 2 of 5 failed: .... A single-statement run's errors are unchanged (no prefix).--no-auto-enable— now happens once on the first statement and the resolved credential is reused for the rest, so a script can never re-probe authorization or re-provision a key per statement. This is the reason for theQueryTarget/ResolvedQueryAuthrefactor inservices.rs; the auth precedence itself is untouched.--queryand stdin as well, so all three SQL sources behave alike. The visible consequence is that a script's trailing newline/;is trimmed before it is sent.the SQL provided contains no statements to runbefore any network access.Why
The flag is plural and a
.sqlfile is the obvious thing to hand it; failing with a server-side parser error was the bug reported in #610. There is no transaction, so a failed script leaves earlier statements applied —--helpand the README now say so explicitly.Tests
cloud::sql_statements::tests): quotes and quoted identifiers, backslash and doubled-quote escapes, comments containing separators, heredocs and an unclosed$marker, unterminated literals, a trailing backslash (no panic), multibyte content, trailing/duplicate separators, comment-only input, and a realistic schema script.CloudErrorKind(so a 401 during a script still exits4).cli_request_shape_test.rs: one request per statement in order with the exact bodies; failure on statement 2 of 3 exits1, printsstatement 2 of 3 failed, and never sends statement 3; a single-statement failure carries no prefix; a comment-only script hits no mock at all. Updated the existing SQL-sources test for the now-trimmed bodies.cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,cargo test -p clickhousectlall clean.clickhouse-cloud-apiwas not touched.Docs
cloud service query --helpand README cover splitting, the quoting/comment rules, first-failure semantics, the absence of a transaction, and the per-statementOKacknowledgement for statements with no result body.Stacked PR
Part of a stacked chain: this PR is based on
fix/611-private-endpoint-id-validation, notmain.Fixes #610
🤖 Generated with Claude Code