Skip to content

docs(cubestore): add a reference page for Cube Store SQL commands - #11430

Open
igorlukanin wants to merge 4 commits into
masterfrom
igor/core-667-docs-cubestore-sql-commands
Open

docs(cubestore): add a reference page for Cube Store SQL commands#11430
igorlukanin wants to merge 4 commits into
masterfrom
igor/core-667-docs-cubestore-sql-commands

Conversation

@igorlukanin

@igorlukanin igorlukanin commented Jul 30, 2026

Copy link
Copy Markdown
Member

Cube Store has admin and recovery SQL commands but no page documenting any of them. This adds one, and moves the existing EXPLAIN material onto it so the commands live in a single place.

What's here

A new reference/cubestore/sql-commands page covering the whole command surface, grouped by how dangerous it is:

  • Diagnosticsinformation_schema.tables, EXPLAIN, EXPLAIN ANALYZE, EXPLAIN ANALYZE DETAILED, DUMP, plus the guidance on reading a query plan.
  • Cache and queue — the CACHE and QUEUE families.
  • Store maintenance and recoverySYS METASTORE, SYS CACHESTORE, and the remaining SYS commands.

Everything outside the diagnostics tier carries a warning. The names don't tell you what these do, so the page spells out the distinctions:

  • CACHE CLEAR / QUEUE CLEAR iterate and delete rows logically.
  • SYS METASTORE TRUNCATE / SYS CACHESTORE TRUNCATE do a single low-level RocksDB range delete with no per-row reads — which is why they still work when the store is too stuck to iterate.
  • SYS CACHESTORE WIPE destroys and reopens RocksDB; past the teardown the previous state is gone, and a teardown that then fails leaves the cachestore closed until the node restarts.
  • DUMP sits in the diagnostics tier but writes to the router's local disk, and the directory it creates is never reclaimed — called out separately so "safe to run" doesn't read as "free".

Two corrections while writing it

  • The keyword is SYS, not SYSTEM — there's no SYSTEM alias, so SYSTEM CACHESTORE WIPE isn't a real command.
  • The cache increment command parses as CACHE INCR. The parser's own error message advertises INC, which is wrong; documented the keyword that actually works.

The existing EXPLAIN section

Moved to the new page rather than duplicated. using-pre-aggregations.mdx keeps the connection recipe and information_schema.tables, and its ### EXPLAIN queries heading stays with an explicit {#explain-queries} anchor so any external inbound links keep working. The two same-page links now point at the new page directly rather than hopping through the stub.

Verified

mintlify broken-links --check-anchors reports nothing from these changes (the 16 it finds are pre-existing in embedding/iframe/, tracked separately). Both pages render locally and the moved content is intact.

Command surface and semantics checked against rust/cubestore/cubestore/src/sql/parser.rs, sql/mod.rs, metastore/rocks_store.rs, and cachestore/lazy.rs at master.

@igorlukanin
igorlukanin requested a review from keydunov as a code owner July 30, 2026 23:17
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 4m 8s —— View job


Docs-only PR; core claims check out against the Rust source. 2 medium, 6 low — nothing blocking.

Full review
  • Read the diff
  • Verify command surface against Cube Store parser/source
  • Check links, anchors, and nav registration
  • Post review

What I verified against master

Every substantive semantic claim on the page holds up:

Claim Source
Keyword is SYS, no SYSTEM alias parser.rs:257w.value.eq_ignore_ascii_case("sys")
CACHE INCR, not INC parser.rs:502 matches "incr"; the error string at :511 advertising INC is genuinely wrong
CLEAR iterates and deletes per row cache_clearRocksTable::truncate (rocks_table.rs:972-982) does table_scan + delete_row per item
TRUNCATE is a single range delete, no per-row reads rocks_store.rs:1043-1058 — one delete_range over [0x00, 0xff×32)
…which is why it survives a store too damaged to iterate follows directly from the above
WIPE destroys and reopens RocksDB lazy.rs:405-466 + wipe_teardown:470-505
SYS DROP CACHESYS DROP QUERY CACHE sql/mod.rs:870-881 — both arms are self.cache.clear()
SYS KILL ALL JOBS deletes queued jobs from the metastore sql/mod.rs:825delete_all_jobs()
Port 3306, CUBESTORE_PORT / CUBESTORE_BIND_ADDR config/mod.rs:1715-1719
SYS subcommand list is complete matches parse_system (:806-828) exactly — 5 for 5
SYS METASTORE / SYS CACHESTORE lists complete match parse_metastore / parse_cachestore exactly

The CLEAR / TRUNCATE / WIPE distinction — the part most likely to be hand-waved — is precise and correct. The two corrections in the PR body are both real bugs in the source's user-facing strings.

Nav placement in docs.json is sensible (sibling of "CLI", inside the Reference tab), and the {#explain-queries} anchor preserves the two same-page links at using-pre-aggregations.mdx:343 and :787.

Findings

Medium

  1. SYS CACHESTORE WIPE omits the failure mode (inline) — if teardown fails past the point of no return, the store is left Closed and rejects every operation until the node is restarted (lazy.rs:420-441). Changes the recovery plan, not just the data outcome.
  2. DUMP is filed under "only read state" (inline) — it writes a metastore backup plus every Parquet file the query reads to local disk, and TempDir::…into_path() deliberately leaks the directory, so nothing cleans it up.

Low

  1. QUEUE synopsis gaps (inline) — EXTERNAL_ID missing from QUEUE ADD and QUEUE RESULT; QUEUE ADD options are order-independent (there's a test for it) though the synopsis implies fixed order; QUEUE ACK accepts NULL; queue keys can be a numeric id or a path.
  2. Unverifiable safety claim (inline) — "Only read-only statements are accepted on that path" has no counterpart in this repo. Worth confirming, since it's the sentence that tells readers the SQL Runner can't truncate a production cachestore.
  3. PR-body inaccuracy — the description cites "the inbound link from cube-core/architecture.mdx", but no such file exists under docs-mintlify/, and grep -rl explain-queries docs-mintlify returns only using-pre-aggregations.mdx. The anchor work is still correct; only the description is off.
  4. The two stub links now hop twiceusing-pre-aggregations.mdx:343 and :787 land on a three-line section that immediately forwards to the new page. Pointing them at /reference/cubestore/sql-commands directly would save readers the bounce, and the {#explain-queries} anchor can stay for external inbound links.
  5. Scope wording — the page opens "commands beyond SELECT" and the PR says "the whole command surface", but DDL/DML (CREATE SCHEMA, CREATE TABLE, CREATE SOURCE, INSERT, SHOW) isn't covered. "Administrative commands" would avoid over-promising.
  6. Follow-up in the Rust source — since the PR already identified both, fixing them keeps code and docs in agreement: parser.rs:511 advertises INC in its error message, and lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE. A reader who hits either string will conclude the docs are wrong.

Not checked

Couldn't run docs.json JSON validation or the Mintlify build — node/python3 invocations were blocked by the sandbox. The docs.json addition is a well-formed group object and structurally matches its siblings, and the PR reports a clean mintlify broken-links --check-anchors.

No prior review threads on this PR to dedupe against or resolve.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

@mintlify

mintlify Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cubed3 🟢 Ready View Preview Jul 30, 2026, 11:20 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Comment thread docs-mintlify/reference/cubestore/sql-commands.mdx
Comment thread docs-mintlify/reference/cubestore/sql-commands.mdx
Comment thread docs-mintlify/reference/cubestore/sql-commands.mdx
Comment thread docs-mintlify/reference/cubestore/sql-commands.mdx
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 2m 22s —— View job


All 4 prior findings addressed and re-verified against the Rust source. 0 new issues — 2 nits, nothing blocking.

Full re-review of 506d916
  • Read the new commit (506d916) diff
  • Verify the new claims against the Rust source
  • Check prior review threads (all 4 already resolved by author)
  • Post review

Prior findings → status

# Finding Status
M1 WIPE omits the Closed failure mode ✅ Fixed — "If the teardown then fails to finish, the cachestore is left closed and rejects every operation until the node is restarted" (sql-commands.mdx:315-317)
M2 DUMP filed under "only read state" ✅ Fixed — tier blurb reworded to "don't change cluster state", DUMP gets its own <Warning> covering the disk write, the missing cleanup, and per-partition growth
L3 QUEUE synopsis gaps ✅ Fixed — all four sub-points landed
L4 Unverifiable read-only-path claim ◐ Reworded, still unverifiable in this repo (see below)
L6 Stub links hopping twice ✅ Fixed — :343 and :787 now point at [ref-cubestore-sql-commands] directly; {#explain-queries} anchor retained on the stub for inbound links
L7 Over-broad scope wording ✅ Fixed — "administrative commands" replaces "commands beyond SELECT"

Re-verified against master

The QUEUE corrections are precise, including the parts that were easy to get subtly wrong:

Claim Source
QUEUE ADD … [EXTERNAL_ID 'id'] parser.rs:666"external_id" => …parse_literal_string() inside parse_sql_options!
QUEUE RESULT [EXTERNAL_ID 'id'] key — option before key parser.rs:774-782 parses external_id then parse_queue_key(); the synopsis order matches
QUEUE ACK key { result | NULL } parser.rs:686-690parse_keyword(Keyword::NULL)None
QUEUE ADD options in any order parse_sql_options! loops until no option matches; test parse_queue_add_options_any_order
key is path or numeric id for exactly GET/ACK/CANCEL/HEARTBEAT/RESULT/RESULT_BLOCKING/MERGE_EXTRA those seven use parse_queue_key(); ADD and RETRIEVE use parse_identifier() and are correctly excluded from the list
DUMP writes a metastore backup + every Parquet file read, dir never reclaimed sql/mod.rs:523 TempDir::new_in(…).into_path(), :527 debug_dump, :535-540 loop over all_required_files() + tokio::fs::copy
DUMP "returns its path" :553-558 — single dump_path string column
WIPE leaves the store closed until restart cachestore/lazy.rs:420-441, drain-timeout bail at :498-505

Getting the RETRIEVE/ADD exclusion right is the detail I'd have expected to slip.

Links also check out: [ref-cubestore-sql-commands] is defined at using-pre-aggregations.mdx:1195/reference/cubestore/sql-commands, [ref-cube-store-architecture] at sql-commands.mdx:344 → a page that exists, and the {#explain-queries} stub still resolves.

Nits

  1. sql-commands.mdx:62-63 — the DUMP carve-out sits oddly next to the sentence above it. "That path only accepts read-only statements" followed by "Avoid DUMP there — it isn't blocked" reads as a contradiction on first pass; a reader wonders how a command that writes to disk clears a read-only gate. Presumably the gate is syntactic (DUMP wraps a SELECT). One clause — "DUMP parses as a read, so it isn't blocked, but…" — would close the gap. Separately, the underlying read-only gating still has no counterpart in this repo (grep for cache-data-source gating in packages/ finds nothing), so it remains a cloud-platform claim I can't confirm here; you resolved that thread, so I'll assume you have.
  2. using-pre-aggregations.mdx:343-344 — the soft wrap falls inside a code span: [use `EXPLAIN` and `EXPLAIN\nANALYZE`]. CommonMark collapses the newline to a space so it renders correctly, but it's a fragile place to wrap. Moving the break to before `EXPLAIN ANALYZE` costs nothing.

Still open outside this PR

parser.rs:511 still advertises INC in its error message and lazy.rs:427/:433 still log SYSTEM CACHESTORE WIPE — both confirmed present at HEAD. A reader who hits either string will conclude the docs are wrong. Out of scope for a docs PR, but worth a follow-up. Fix this →

Also: the PR description still cites "the inbound link from cube-core/architecture.mdx" — no such file exists under docs-mintlify/, and explain-queries appears only in using-pre-aggregations.mdx. Description-only inaccuracy; the anchor work itself is correct.

Not checked

docs.json JSON validation and the Mintlify build — node was blocked by the sandbox. The docs.json addition is a well-formed group object matching its siblings, and the Mintlify preview deployment reports 🟢 Ready.

All four of my prior threads were already resolved by the author before this run, so nothing to resolve and no duplicate inline comments to skip. No new inline comments — everything above is either a nit or out of scope.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

@igorlukanin

igorlukanin commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Took 1–4, 6 and 7 in 506d91638d; details on the inline threads.

On 5 — docs-mintlify/cube-core/architecture.mdx does exist, but you were right that the claim was wrong: it links to #inspecting-pre-aggregations, not #explain-queries, so it was never riding on the anchor at all. PR body corrected. 6 is done too, so the {#explain-queries} anchor now only serves external inbound links.

On 8 — the INC half is CORE-697. Leaving the SYSTEM CACHESTORE WIPE log strings in lazy.rs out of a docs PR; same class, worth folding into that one.

@igorlukanin

igorlukanin commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Both nits taken in 2478b7ff56: DUMP now says it parses as a read, and the wrap moved out of the code span.

The Rust strings are worth folding into CORE-697 — same class, and that PR is already in the parser. The description note is stale: I corrected it before this run.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 47s —— View job


Both nits from the last round are addressed; the new DUMP explanation is accurate. 0 new issues — LGTM.

Full re-review of 2478b7f
  • Read the new commit diff
  • Verify the reworded claims against the Rust source
  • Check prior review threads (all 4 resolved)
  • Post review

What changed

2478b7f is a 5-line follow-up touching only the two nits from the previous round.

Nit 1 — the DUMP carve-out no longer reads as a contradiction (sql-commands.mdx:62-63):

Avoid DUMP there. It wraps a SELECT, so it reads as a query and gets through, but it writes to the router's local disk and nothing cleans it up.

The mechanism claim checks out: parser.rs:295-306 requires a SELECT after dump ("Expected select query after 'dump'") and wraps it as Statement::Dump(Box<Query>). So a keyword allowlist scanning the statement text does see a SELECT and no gated keyword — exactly the gap you described on the thread. The sentence now explains why the gate misses it rather than leaving the reader to reconcile "read-only path" with "writes to disk".

Nit 2 — the soft wrap is out of the code span (using-pre-aggregations.mdx:343-345): `EXPLAIN ANALYZE` is now intact on one line, break moved to after always. Same rendering, less fragile.

Nothing else moved, and the surrounding text still reads coherently: the tier blurb at :69-71 keeps the DUMP exception, the <Note> keeps the rejection claim for the destructive tiers, and the two are consistent with each other.

Threads

All four of my prior threads are resolved — nothing to resolve, nothing to re-affirm, no duplicate inline comments to skip.

Still open outside this PR

Confirmed present at HEAD, both correctly deferred to CORE-697: parser.rs:511 advertises INC, and lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE. Folding the log strings into that PR makes sense — same class, and it's already in the parser.

The description note about cube-core/architecture.mdx was indeed stale on my side; your correction (it links #inspecting-pre-aggregations, so the anchor was never load-bearing for it) is right, and the {#explain-queries} anchor now serves external inbound links only.

Not checked

docs.json JSON validation and the Mintlify build — node is blocked by the sandbox. This commit doesn't touch docs.json, and the Mintlify preview reports 🟢 Ready.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands) •

@igorlukanin
igorlukanin force-pushed the igor/core-667-docs-cubestore-sql-commands branch from 2478b7f to 77bdedb Compare July 31, 2026 14:10
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 2m 14s —— View job


Rebase-only synchronize — file contents are byte-identical to the commit I approved. 0 new issues, 1 optional nit.

Full re-review of 77bdedb
  • Determine what changed since the last reviewed commit
  • Verify any new claims against the Rust source
  • Check prior review threads
  • Post review

What changed

Nothing in the content. The three commits were rewritten onto the new master tip (a433127, v1.7.16) — all three share a single committer timestamp (2026-07-31T16:09:16+02:00), and the merge base is now a433127 rather than the older one:

77bdedb docs(cubestore): explain why DUMP clears the read-only gate
ff6402a docs(cubestore): flag DUMP's disk writes, the WIPE closed state, and QUEUE syntax gaps
f40df6c docs(cubestore): add a reference page for Cube Store SQL commands

The resulting blobs are the same ones I reviewed at 2478b7f:

File Blob
reference/cubestore/sql-commands.mdx cbc00ae
docs/pre-aggregations/using-pre-aggregations.mdx cd540f3
docs.json d66b74b

Both previous-round fixes are intact — sql-commands.mdx:62-63 still carries "It wraps a SELECT, so it reads as a query and gets through", and using-pre-aggregations.mdx:343-345 still has `EXPLAIN ANALYZE` unbroken on one line. Nothing regressed in the rewrite.

Spot-checks I hadn't run before

Since the diff is unchanged, I used this pass to close two gaps from earlier rounds:

Claim Source
EXPLAIN ANALYZE DETAILED "actually executes the query under per-query tracing" sql/explain_detailed.rs:1-4 module doc says exactly that; :57-60 calls run_router_select_detailed on a real worker node
Link target /docs/pre-aggregations/cube-store-architecture resolves docs-mintlify/docs/pre-aggregations/cube-store-architecture.mdx exists
No competing Cube Store nav group cubestore appears exactly once in docs.json (:565) — no duplicate group to merge into

Nit (optional, non-blocking)

sql-commands.mdx:147-163EXPLAIN ANALYZE DETAILED is rejected for system-table selects: explain_detailed.rs:39-42 returns "EXPLAIN ANALYZE DETAILED is not supported for selects from system tables" on QueryPlan::Meta. Since information_schema.tables is documented two sections above on the same page, a reader may well try EXPLAIN ANALYZE DETAILED SELECT * FROM information_schema.tables first. One clause would cover it — plain EXPLAIN and EXPLAIN ANALYZE don't have this restriction, so the contrast is worth a half-sentence. Entirely up to you; not worth another round on its own.

Threads

All four of my prior threads are resolved (gh list-review-threads confirms isResolved: true on each). Nothing to resolve, nothing to re-affirm, no duplicate inline comments to skip. No new inline comments posted.

Still open outside this PR

Unchanged and correctly deferred to CORE-697: parser.rs:511 advertises INC, lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE.

Not checked

docs.json JSON validation and the Mintlify build — both node and python3 invocations were blocked by the sandbox this run. The docs.json hunk is a well-formed group object matching its siblings and is byte-identical to the version the Mintlify preview already built 🟢.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

@igorlukanin

Copy link
Copy Markdown
Member Author

Took the nit: explain_detailed.rs:39-42 does reject QueryPlan::Meta, and information_schema.tables is right there on the same page, so the contrast is worth having. Added in 69e0bd0878.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 1m 18s —— View job


One factual error in the new commit: EXPLAIN ANALYZE is rejected on system tables too. 1 medium — traceable to my own prior nit.

Full re-review of 69e0bd0
  • Read the new commit diff
  • Verify the new claim against the Rust source
  • Check prior review threads
  • Post review

What changed

69e0bd0 adds four lines after the EXPLAIN ANALYZE DETAILED example (sql-commands.mdx:165-168):

Because it runs the query on a worker, it only accepts selects from data tables. Selects from system tables such as information_schema.tables are rejected; use plain EXPLAIN or EXPLAIN ANALYZE for those.

Half of that is right. The rejection is real — explain_detailed.rs:38-43 returns "EXPLAIN ANALYZE DETAILED is not supported for selects from system tables" on QueryPlan::Meta. But the escape hatch it offers doesn't hold.

The finding (inline)

EXPLAIN ANALYZE rejects system tables as well. sql/mod.rs:654-670 — the QueryPlan::Meta arm of explain() branches on the analyze flag:

Command System-table select
EXPLAIN ✅ returns the logical plan (mod.rs:656-663)
EXPLAIN ANALYZE "EXPLAIN ANALYZE is not supported for selects from system tables" (mod.rs:664-668)
EXPLAIN ANALYZE DETAILED "…DETAILED is not supported…" (explain_detailed.rs:39-42)

Plain EXPLAIN is the only one of the three that works. A reader who follows the sentence to EXPLAIN ANALYZE SELECT * FROM information_schema.tables gets an error — the exact failure the sentence was added to prevent.

This traces directly to my nit last round, which asserted "plain EXPLAIN and EXPLAIN ANALYZE don't have this restriction." I checked the DETAILED path and stopped there without reading the Meta arm of explain(). The correction belongs on my side.

Secondary: the stated reason is off by a step. The rejection fires in the router before any worker is involved — logical_plan returns QueryPlan::Meta, so there's no serialized plan to route. "Because it runs the query on a worker" is a fair gloss for DETAILED alone, but it can't explain why EXPLAIN ANALYZE — which does dispatch to workers — is refused too. "System-table selects don't produce a distributable plan" covers both cases. Suggested replacement is on the inline thread.

Everything else

Unchanged from 77bdedb, which I'd already verified. The two files this commit doesn't touch are byte-identical, and all previously confirmed claims (the CLEAR/TRUNCATE/WIPE distinction, the QUEUE synopsis including the RETRIEVE/ADD exclusion, DUMP's disk writes and read-only-gate carve-out, SYS subcommand completeness) still hold.

Threads

All four of my prior threads are resolved (isResolved: true on each) — nothing to resolve, and the new comment is on a line no existing thread covers, so no dedupe skip.

Still open outside this PR

Unchanged and correctly deferred to CORE-697: parser.rs:511 advertises INC, lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE.

Not checked

docs.json JSON validation and the Mintlify build — node is blocked by the sandbox. This commit touches only sql-commands.mdx.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

Comment thread docs-mintlify/reference/cubestore/sql-commands.mdx Outdated
@igorlukanin
igorlukanin force-pushed the igor/core-667-docs-cubestore-sql-commands branch from 69e0bd0 to 1892050 Compare July 31, 2026 16:11
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 51s —— View job


The rewritten sentence is correct against the source; my prior finding is resolved. 0 new issues — LGTM, 1 optional nit.

Full re-review of 1892050
  • Read the new commit diff
  • Verify the rewritten claim against the Rust source
  • Check prior review threads
  • Post review

What changed

1892050 is a 4-line replacement of the sentence I flagged last round (sql-commands.mdx:165-167), and nothing else — git diff 77bdedb HEAD touches only this file, only this hunk.

System-table selects don't produce a distributable plan, so both EXPLAIN ANALYZE
and EXPLAIN ANALYZE DETAILED reject them. Plain EXPLAIN is the only one of the
three that works on information_schema.tables.

Both halves check out, and the incorrect escape hatch is gone:

Claim Source
EXPLAIN ANALYZE rejects system-table selects sql/mod.rs:664-668QueryPlan::Meta + analyze"EXPLAIN ANALYZE is not supported for selects from system tables"
EXPLAIN ANALYZE DETAILED rejects them explain_detailed.rs:38-43QueryPlan::Meta(_) => return Err(…)
Plain EXPLAIN is the only one that works mod.rs:655-663if !analyze returns the logical plan
Reason: no distributable plan logical_plan yields Meta, so there's no serialized.to_serialized_plan() to route; the refusal is in the router, before any worker

The reason now covers both commands rather than only DETAILED — that was the part of the previous wording that couldn't be made to work, and the new framing fixes it at the root instead of patching the command list.

Everything else on the page is byte-identical to 77bdedb, which I'd already verified end to end (the CLEAR/TRUNCATE/WIPE distinction, the QUEUE synopsis including the RETRIEVE/ADD exclusion from parse_queue_key, DUMP's disk writes and its read-only-gate carve-out, SYS subcommand completeness, SYS-not-SYSTEM, CACHE INCR).

Nit (optional, non-blocking)

The note sits at the end of the ### EXPLAIN ANALYZE DETAILED section but constrains EXPLAIN ANALYZE too. A reader who scrolls to ### EXPLAIN ANALYZE, gets their answer, and stops never sees it. Half a sentence in the EXPLAIN ANALYZE section, or hoisting the note up under ## Diagnostics where it governs all three, would close that. Not worth another round on its own — the content is right either way.

Threads

All five of my threads are resolved (isResolved: true on each), including the one from last round, which is now isOutdated as well since the line was rewritten. Nothing to resolve, no duplicate inline comments to skip, no new inline comments posted.

Still open outside this PR

Unchanged and correctly deferred to CORE-697: parser.rs:511 advertises INC, lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE.

Not checked

docs.json JSON validation and the Mintlify build — node is blocked by the sandbox. This commit touches only sql-commands.mdx; docs.json is unchanged from the version the preview already built 🟢.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

@igorlukanin
igorlukanin force-pushed the igor/core-667-docs-cubestore-sql-commands branch from 1892050 to 4f7613f Compare July 31, 2026 16:15
@igorlukanin

Copy link
Copy Markdown
Member Author

Fair point — someone who stops at EXPLAIN ANALYZE was the reader the sentence was for. Moved it up under that section in 4f7613fa8b; it still names both commands, so DETAILED is covered on the way past.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 50s —— View job


The note moved into the EXPLAIN ANALYZE section and reads correctly there; source claims still hold. 0 issues — LGTM.

Full re-review of 4f7613f
  • Determine what changed since the last reviewed state
  • Verify the claim and its new placement against the Rust source
  • Check prior review threads
  • Post review

What changed

The branch was rewritten so the note lands in one commit rather than as a fix-up on top: history is now four commits, and git diff 77bdedb HEAD is a single 4-line hunk in sql-commands.mdx:147-149 — nothing else on the page or in the other two files moved.

The note now sits at the end of ### EXPLAIN ANALYZE, right after that section's output example:

System-table selects don't produce a distributable plan, so neither EXPLAIN ANALYZE nor EXPLAIN ANALYZE DETAILED accepts one. Plain EXPLAIN is the only one of the three that works on information_schema.tables.

That's the placement my nit asked for, and it closes the gap it was about: a reader who scrolls to EXPLAIN ANALYZE, gets their answer, and stops now sees the restriction before leaving. Naming both commands means the DETAILED reader is covered on the way past, so nothing is lost by not repeating it two sections down. The forward reference to EXPLAIN ANALYZE DETAILED — a heading four lines below — is harmless.

Re-verified against master

Claim Source
EXPLAIN ANALYZE rejects system-table selects sql/mod.rs:654-670QueryPlan::Meta + analyze"EXPLAIN ANALYZE is not supported for selects from system tables"
EXPLAIN ANALYZE DETAILED rejects them explain_detailed.rs:38-43QueryPlan::Meta(_) => return Err(…)
Plain EXPLAIN is the only one that works mod.rs:655-663if !analyze returns the logical plan
Reason: no distributable plan logical_plan yields Meta, so there's no serialized.to_serialized_plan() to route; both refusals are in the router

Everything else on the page is byte-identical to 77bdedb, which I'd already verified end to end: the CLEAR / TRUNCATE / WIPE distinction, the QUEUE synopsis including the RETRIEVE/ADD exclusion from parse_queue_key, DUMP's disk writes and its read-only-gate carve-out, SYS subcommand completeness, SYS-not-SYSTEM, and CACHE INCR.

Threads

All five of my threads are resolved (isResolved: true on each; the last one is isOutdated too, since the line was rewritten). Nothing to resolve, no duplicate inline comments to skip, no new inline comments posted.

Still open outside this PR

Unchanged and correctly deferred to CORE-697: parser.rs:511 advertises INC, and lazy.rs:427/:433 log SYSTEM CACHESTORE WIPE.

Not checked

docs.json JSON validation and the Mintlify build — node is blocked by the sandbox. This commit touches only sql-commands.mdx; docs.json is unchanged from the version the preview already built 🟢.

• branch [`igor/core-667-docs-cubestore-sql-commands`](https://github.com/cube-js/cube/tree/igor/core-667-docs-cubestore-sql-commands)

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.

1 participant