From f64e70baefb42e50bd91319cef4f7a762226df26 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:01:32 +0000 Subject: [PATCH 01/12] feat(core): add sql-split + script-result pure helpers for multiquery (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pure foundation for running a ;-separated script (DDL/INSERT/SELECT): - sql-split.js: lexical statement splitter that skips ; inside '…'/"…"/`…` literals (\' and '' escapes) and -- / # / block comments, plus an isRowReturning() classifier. - script-result.js: parse a JSONCompact SELECT body → {columns,rows,truncated} (the renderTable shape) and a comma-joined first-row preview. Both 100% covered. No wiring yet. --- src/core/script-result.js | 37 +++++++++++ src/core/sql-split.js | 101 ++++++++++++++++++++++++++++ tests/unit/script-result.test.js | 64 ++++++++++++++++++ tests/unit/sql-split.test.js | 110 +++++++++++++++++++++++++++++++ 4 files changed, 312 insertions(+) create mode 100644 src/core/script-result.js create mode 100644 src/core/sql-split.js create mode 100644 tests/unit/script-result.test.js create mode 100644 tests/unit/sql-split.test.js diff --git a/src/core/script-result.js b/src/core/script-result.js new file mode 100644 index 0000000..52d00d6 --- /dev/null +++ b/src/core/script-result.js @@ -0,0 +1,37 @@ +// Pure helpers for script-mode SELECT outcomes. A row-returning statement is +// run with FORMAT JSONCompact (one JSON object: { meta:[{name,type}], data:[[…]] }) +// through the raw / wait_end_of_query path, so the whole body arrives as text and +// is parsed here once into a { columns, rows } shape — the same shape the result +// grid (renderTable) consumes. The script summary grid shows a one-line preview +// of the first row in column 2; clicking it opens the full table in a side pane. + +/** + * Parse a JSONCompact response body into `{ columns, rows, truncated }`, capping + * `rows` at `cap` (default 100; the server also caps via max_result_rows, so this + * is a display backstop). A blank body or one that isn't valid JSON yields an + * empty result rather than throwing. Pure. + */ +export function parseSelectResult(rawText, cap = 100) { + const text = String(rawText == null ? '' : rawText).trim(); + if (!text) return { columns: [], rows: [], truncated: false }; + let json; + try { + json = JSON.parse(text); + } catch { + return { columns: [], rows: [], truncated: false }; + } + const columns = (json.meta || []).map((m) => ({ name: m.name, type: m.type })); + const data = json.data || []; + return { columns, rows: data.slice(0, cap), truncated: data.length > cap }; +} + +/** + * A compact, comma-joined preview of the first row's values (the normal case is + * one row / one number, e.g. a count). NULLs render empty, matching the result + * grid. Truncated with an ellipsis past `max`. '' when there are no rows. Pure. + */ +export function firstRowPreview(rows, max = 160) { + if (!rows || !rows.length) return ''; + const s = rows[0].map((v) => (v == null ? '' : String(v))).join(', '); + return s.length > max ? s.slice(0, max - 1) + '…' : s; +} diff --git a/src/core/sql-split.js b/src/core/sql-split.js new file mode 100644 index 0000000..3ae779d --- /dev/null +++ b/src/core/sql-split.js @@ -0,0 +1,101 @@ +// Pure client-side SQL script splitter. ClickHouse's HTTP interface runs exactly +// one statement per request, so to run a `;`-separated script (DDL / INSERT / +// SELECT) we split it here and POST each statement in turn (the same model as +// `clickhouse-client --multiquery`). Splitting is purely lexical: it skips `;` +// inside '…' / "…" / `…` literals (honoring both `\'` backslash and `''` doubled +// escapes) and inside -- / # line comments and /* */ block comments. +// +// Known limitation: `INSERT … FORMAT CSV\n` whose inline data +// contains a `;` will mis-split — the splitter has no way to know where the +// format payload ends. Inline-data inserts should be run on their own. + +/** + * Split `sql` into individual statements on top-level `;`. Literals and comments + * are scanned so their `;` (and quote/comment characters) don't break a + * statement. Each returned statement is trimmed; comment-only / whitespace-only + * fragments are dropped. A single statement (± a trailing `;`) yields a + * one-element list, so the caller can preserve today's single-query path. Pure. + */ +export function splitStatements(sql) { + const text = String(sql || ''); + const n = text.length; + const out = []; + let buf = ''; + let hasCode = false; // the current fragment holds runnable (non-comment) text + let i = 0; + const push = () => { if (hasCode) out.push(buf.trim()); buf = ''; hasCode = false; }; + while (i < n) { + const c = text[i]; + const c2 = text[i + 1]; + // -- and # line comments: copy verbatim to end of line (not code). + if ((c === '-' && c2 === '-') || c === '#') { + let j = i; + while (j < n && text[j] !== '\n') j++; + buf += text.slice(i, j); + i = j; + continue; + } + // /* */ block comment (non-nesting, matching ClickHouse): copy verbatim. + if (c === '/' && c2 === '*') { + let j = i + 2; + while (j < n && !(text[j] === '*' && text[j + 1] === '/')) j++; + j = Math.min(n, j + 2); // include the closing */ (or run to EOF if unterminated) + buf += text.slice(i, j); + i = j; + continue; + } + // '…' string, "…" / `…` quoted identifier. Backslash escapes the next char; + // a doubled quote (`''`) is an escaped quote, not a terminator. + if (c === "'" || c === '"' || c === '`') { + const quote = c; + buf += c; + let j = i + 1; + while (j < n) { + const d = text[j]; + if (d === '\\') { buf += text.slice(j, j + 2); j += 2; continue; } + if (d === quote) { + if (text[j + 1] === quote) { buf += d + quote; j += 2; continue; } + buf += d; j += 1; break; + } + buf += d; j += 1; + } + i = j; + hasCode = true; + continue; + } + if (c === ';') { push(); i += 1; continue; } + buf += c; + if (!/\s/.test(c)) hasCode = true; + i += 1; + } + push(); + return out; +} + +// Statement keywords whose result is a row set (so script mode fetches them with +// a row-bearing format and shows a result preview). Everything else (CREATE / +// INSERT / ALTER / DROP / …) is run for effect and reported as OK. +const ROW_RETURNING = new Set([ + 'SELECT', 'WITH', 'SHOW', 'DESC', 'DESCRIBE', 'EXISTS', 'VALUES', 'EXPLAIN', +]); + +/** The first SQL keyword of `stmt`, uppercased, after skipping leading + * whitespace and -- / # / block comments. '' when none. Pure. */ +export function leadingKeyword(stmt) { + let s = String(stmt || ''); + for (;;) { + const before = s; + s = s.replace(/^\s+/, '') + .replace(/^--[^\n]*/, '') + .replace(/^#[^\n]*/, '') + .replace(/^\/\*[\s\S]*?\*\//, ''); + if (s === before) break; + } + const m = /^([A-Za-z]+)/.exec(s); + return m ? m[1].toUpperCase() : ''; +} + +/** True when `stmt` is a row-returning statement (SELECT/WITH/SHOW/…). Pure. */ +export function isRowReturning(stmt) { + return ROW_RETURNING.has(leadingKeyword(stmt)); +} diff --git a/tests/unit/script-result.test.js b/tests/unit/script-result.test.js new file mode 100644 index 0000000..516ec0c --- /dev/null +++ b/tests/unit/script-result.test.js @@ -0,0 +1,64 @@ +import { describe, it, expect } from 'vitest'; +import { parseSelectResult, firstRowPreview } from '../../src/core/script-result.js'; + +describe('parseSelectResult', () => { + it('parses a JSONCompact body into columns + rows', () => { + const body = JSON.stringify({ + meta: [{ name: 'count()', type: 'UInt64' }], + data: [['42']], + }); + expect(parseSelectResult(body)).toEqual({ + columns: [{ name: 'count()', type: 'UInt64' }], + rows: [['42']], + truncated: false, + }); + }); + + it('returns an empty result for a blank / nullish body', () => { + for (const v of ['', ' ', null, undefined]) { + expect(parseSelectResult(v)).toEqual({ columns: [], rows: [], truncated: false }); + } + }); + + it('returns an empty result for a non-JSON body', () => { + expect(parseSelectResult('not json')).toEqual({ columns: [], rows: [], truncated: false }); + }); + + it('tolerates a body missing meta / data', () => { + expect(parseSelectResult('{}')).toEqual({ columns: [], rows: [], truncated: false }); + }); + + it('caps rows and flags truncation', () => { + const data = Array.from({ length: 105 }, (_, i) => [String(i)]); + const out = parseSelectResult(JSON.stringify({ meta: [{ name: 'n', type: 'Int' }], data }), 100); + expect(out.rows).toHaveLength(100); + expect(out.truncated).toBe(true); + }); + + it('does not flag truncation at exactly the cap', () => { + const data = Array.from({ length: 100 }, (_, i) => [String(i)]); + const out = parseSelectResult(JSON.stringify({ meta: [{ name: 'n', type: 'Int' }], data }), 100); + expect(out.rows).toHaveLength(100); + expect(out.truncated).toBe(false); + }); +}); + +describe('firstRowPreview', () => { + it('joins the first row with commas', () => { + expect(firstRowPreview([['42']])).toBe('42'); + expect(firstRowPreview([['a', '1', 'x'], ['b']])).toBe('a, 1, x'); + }); + it('renders NULLs as empty', () => { + expect(firstRowPreview([['a', null, 'c']])).toBe('a, , c'); + }); + it('returns "" with no rows', () => { + expect(firstRowPreview([])).toBe(''); + expect(firstRowPreview(null)).toBe(''); + }); + it('truncates past max with an ellipsis', () => { + const long = 'x'.repeat(200); + const out = firstRowPreview([[long]], 10); + expect(out).toHaveLength(10); + expect(out.endsWith('…')).toBe(true); + }); +}); diff --git a/tests/unit/sql-split.test.js b/tests/unit/sql-split.test.js new file mode 100644 index 0000000..f8f12fa --- /dev/null +++ b/tests/unit/sql-split.test.js @@ -0,0 +1,110 @@ +import { describe, it, expect } from 'vitest'; +import { splitStatements, leadingKeyword, isRowReturning } from '../../src/core/sql-split.js'; + +describe('splitStatements', () => { + it('returns [] for empty / nullish input', () => { + expect(splitStatements('')).toEqual([]); + expect(splitStatements(null)).toEqual([]); + expect(splitStatements(undefined)).toEqual([]); + expect(splitStatements(' \n ')).toEqual([]); + }); + + it('returns a single trimmed statement (no semicolon)', () => { + expect(splitStatements(' SELECT 1 ')).toEqual(['SELECT 1']); + }); + + it('treats a single statement with a trailing ; as one element', () => { + expect(splitStatements('SELECT 1;')).toEqual(['SELECT 1']); + expect(splitStatements('SELECT 1 ; ')).toEqual(['SELECT 1']); + }); + + it('splits a multi-statement script in order', () => { + expect(splitStatements('CREATE TABLE t (a Int); INSERT INTO t VALUES (1); SELECT * FROM t')) + .toEqual(['CREATE TABLE t (a Int)', 'INSERT INTO t VALUES (1)', 'SELECT * FROM t']); + }); + + it('does not split on a ; inside a single-quoted string', () => { + expect(splitStatements("SELECT 'a;b'; SELECT 2")) + .toEqual(["SELECT 'a;b'", 'SELECT 2']); + }); + + it('handles backslash-escaped quotes inside a string', () => { + expect(splitStatements("SELECT 'it\\'s; fine'; SELECT 2")) + .toEqual(["SELECT 'it\\'s; fine'", 'SELECT 2']); + }); + + it('handles doubled-quote escapes inside a string', () => { + expect(splitStatements("SELECT 'it''s; fine'; SELECT 2")) + .toEqual(["SELECT 'it''s; fine'", 'SELECT 2']); + }); + + it('handles an escaped backslash at the end of a string', () => { + expect(splitStatements("SELECT 'a\\\\'; SELECT 2")) + .toEqual(["SELECT 'a\\\\'", 'SELECT 2']); + }); + + it('ignores ; inside double-quoted and backtick identifiers', () => { + expect(splitStatements('SELECT "a;b", `c;d`; SELECT 2')) + .toEqual(['SELECT "a;b", `c;d`', 'SELECT 2']); + }); + + it('leaves an unterminated string as one trailing statement', () => { + expect(splitStatements("SELECT 'oops")).toEqual(["SELECT 'oops"]); + }); + + it('ignores ; inside -- line comments', () => { + expect(splitStatements('SELECT 1 -- a;b\n; SELECT 2')) + .toEqual(['SELECT 1 -- a;b', 'SELECT 2']); + }); + + it('ignores ; inside # line comments', () => { + expect(splitStatements('SELECT 1 # a;b\n; SELECT 2')) + .toEqual(['SELECT 1 # a;b', 'SELECT 2']); + }); + + it('ignores ; inside /* */ block comments', () => { + expect(splitStatements('SELECT 1 /* a;b */; SELECT 2')) + .toEqual(['SELECT 1 /* a;b */', 'SELECT 2']); + }); + + it('tolerates an unterminated block comment', () => { + expect(splitStatements('SELECT 1; /* trailing')).toEqual(['SELECT 1']); + }); + + it('drops comment-only and whitespace-only fragments but keeps comments attached to code', () => { + // The leading comment belongs to the statement that follows it (harmless to + // send); the bare `;` and the trailing comment-only fragment are dropped. + expect(splitStatements('-- just a note\nSELECT 1; \n ; /* end */')) + .toEqual(['-- just a note\nSELECT 1']); + }); +}); + +describe('leadingKeyword', () => { + it('returns the uppercased first keyword', () => { + expect(leadingKeyword('select 1')).toBe('SELECT'); + expect(leadingKeyword(' Insert into t ...')).toBe('INSERT'); + }); + it('skips leading whitespace and comments of every kind', () => { + expect(leadingKeyword(' \n -- note\n # bang\n /* block */ SELECT 1')).toBe('SELECT'); + }); + it('returns "" when there is no leading keyword', () => { + expect(leadingKeyword('')).toBe(''); + expect(leadingKeyword('-- only a comment')).toBe(''); + expect(leadingKeyword('42 + 1')).toBe(''); + }); +}); + +describe('isRowReturning', () => { + it('is true for row-bearing statements', () => { + for (const s of ['SELECT 1', 'with x as (select 1) select * from x', + 'SHOW TABLES', 'DESC t', 'DESCRIBE t', 'EXISTS TABLE t', 'VALUES (1)', 'EXPLAIN SELECT 1']) { + expect(isRowReturning(s)).toBe(true); + } + }); + it('is false for effectful statements', () => { + for (const s of ['CREATE TABLE t (a Int)', 'INSERT INTO t VALUES (1)', + 'DROP TABLE t', 'ALTER TABLE t ADD COLUMN b Int', '-- comment only', '']) { + expect(isRowReturning(s)).toBe(false); + } + }); +}); From 06f9dac26e193801cd701c8ed3ffbc4f99e9ac28 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:02:22 +0000 Subject: [PATCH 02/12] feat(net): let runQuery forward caller params to chUrl (#83) Additive o.params passthrough merged alongside query_id, so multiquery SELECTs can cap results server-side (max_result_rows / result_overflow_mode) without a new function or SQL mangling. Isolated so it can be reverted independently. --- src/net/ch-client.js | 8 ++++++-- tests/unit/ch-client.test.js | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/net/ch-client.js b/src/net/ch-client.js index 3ec2ba7..f988f80 100644 --- a/src/net/ch-client.js +++ b/src/net/ch-client.js @@ -370,7 +370,9 @@ export async function loadEntityDoc(ctx, name, sqlString) { * * @param ctx * @param sql - * @param o { format, signal, onLine(json), onChunk(), onRaw(text) } + * @param o { format, signal, params, onLine(json), onChunk(), onRaw(text) } + * `params` are extra ClickHouse settings/query-string options (e.g. + * multiquery SELECTs pass max_result_rows / result_overflow_mode). */ export async function runQuery(ctx, sql, o = {}) { const fmt = o.format || 'Table'; @@ -392,7 +394,9 @@ export async function runQuery(ctx, sql, o = {}) { // and surfaces mid-stream errors via the in-band `exception` line instead. extra: { ...(isStreaming ? {} : { wait_end_of_query: 1 }), add_http_cors_header: 1 }, // Tagging the request with a query_id lets Cancel issue KILL QUERY for it. - params: o.queryId ? { query_id: o.queryId } : {}, + // Caller-supplied params (o.params) ride alongside — e.g. multiquery SELECTs + // add max_result_rows / result_overflow_mode to cap the result server-side. + params: { ...(o.queryId ? { query_id: o.queryId } : {}), ...(o.params || {}) }, }); const resp = await authedFetch(ctx, url, sql, o.signal); diff --git a/tests/unit/ch-client.test.js b/tests/unit/ch-client.test.js index 72af692..503e49a 100644 --- a/tests/unit/ch-client.test.js +++ b/tests/unit/ch-client.test.js @@ -298,6 +298,14 @@ describe('runQuery', () => { await runQuery(ctx, 'x', { queryId: 'abc-123' }); expect(ctx.fetch.mock.calls[0][0]).toContain('query_id=abc-123'); }); + it('passes caller params (e.g. result caps) alongside query_id', async () => { + const ctx = ctxWith(async () => textResp('{"meta":[],"data":[]}')); + await runQuery(ctx, 'x', { format: 'JSONCompact', queryId: 'q1', params: { max_result_rows: 100, result_overflow_mode: 'break' } }); + const url = ctx.fetch.mock.calls[0][0]; + expect(url).toContain('query_id=q1'); + expect(url).toContain('max_result_rows=100'); + expect(url).toContain('result_overflow_mode=break'); + }); it('streams without wait_end_of_query; raw modes keep it for clean error status', async () => { const s = ctxWith(async () => streamResp(['{"row":{}}\n'])); await runQuery(s, 'x', { format: 'Table' }); From 1140c1c44497e6693811b562204ddb2b2188abf7 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:15:39 +0000 Subject: [PATCH 03/12] =?UTF-8?q?feat(ui):=20multiquery=20+=20run-selectio?= =?UTF-8?q?n=20=E2=80=94=20run=20a=20;-separated=20script=20(#83)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run a ;-separated script (DDL / INSERT / SELECT) in one shot, or run just the highlighted text. ⌘+Enter auto-detects: a single statement keeps today's rich Table/Chart/EXPLAIN path; >1 statements run sequentially (one ClickHouse request each, stop-on-first-error) into a per-statement summary grid. - runScript in app.js: fresh query_id per statement (Cancel kills the live one), abort → stop + mark cancelled, one history entry for the whole script. - run() gains an opts.sql override so a single selected statement uses the rich path; runEntry routes the Run button + ⌘+Enter and picks script vs single. - Selection tracked via a hasSelection signal (selectionchange, gated on the editor being focused); the Run button flips to "Run selection". - Row-returning statements fetched as JSONCompact capped at 100 rows (max_result_rows / result_overflow_mode); Col 2 shows the first row comma-joined, click opens all rows in a side pane (reuses the cell-detail scaffold; shared Drawer deferred to #60). - state: hasSelection signal, recordScriptHistory, recordHistory sqlText override. Per-file coverage gate green (1078 tests); build OK. --- src/state.js | 41 ++++++++--- src/styles.css | 17 +++++ src/ui/app.js | 142 ++++++++++++++++++++++++++++++++----- src/ui/results.js | 124 ++++++++++++++++++++++++++++++++ tests/unit/app.test.js | 116 ++++++++++++++++++++++++++++++ tests/unit/results.test.js | 93 +++++++++++++++++++++++- tests/unit/state.test.js | 19 ++++- 7 files changed, 523 insertions(+), 29 deletions(-) diff --git a/src/state.js b/src/state.js index 0d9a99c..5e97650 100644 --- a/src/state.js +++ b/src/state.js @@ -72,6 +72,10 @@ export function createState(read = { loadJSON, loadStr }) { running: signal(false), abortController: null, resultView: signal('table'), + // True while the editor has a non-empty (non-whitespace) text selection, so + // ⌘+Enter / Run target just that text. Drives the Run button's + // "Run" ↔ "Run selection" label (an effect in createApp). Via `.value`. + hasSelection: signal(false), // `forceExplain` is set by the Explain button to put an ordinary query into // EXPLAIN-view mode; a normal Run clears it (session-only). The active view is // derived per-run from the typed statement / clicked tab, not stored here. @@ -294,21 +298,36 @@ export function markLibrarySaved(state) { state.libraryDirty.value = false; } -/** Record a successful run in history (most-recent first, capped at 50). */ -export function recordHistory(state, tab, save = saveJSON, now = Date.now()) { - const sql = String(tab.sql || '').trim(); - if (!sql) return; - state.history.unshift({ - id: makeId('h', now), - sql, - ts: now, - rows: tab.result.rawText != null ? null : tab.result.rows.length, - ms: Math.round(tab.result.progress.elapsed_ns / 1e6), - }); +// Push one history entry (most-recent first, capped at 50). Internal — the +// exported recorders below supply the sql/rows/ms. +function pushHistory(state, sql, rows, ms, save, now) { + const s = String(sql || '').trim(); + if (!s) return; + state.history.unshift({ id: makeId('h', now), sql: s, ts: now, rows, ms }); state.history = state.history.slice(0, 50); save(KEYS.history, state.history); } +/** + * Record a successful run in history. `sqlText` overrides the recorded SQL (used + * when a selection — not the whole tab — was run); it defaults to `tab.sql`. + */ +export function recordHistory(state, tab, save = saveJSON, now = Date.now(), sqlText) { + pushHistory( + state, + sqlText != null ? sqlText : tab.sql, + tab.result.rawText != null ? null : tab.result.rows.length, + Math.round(tab.result.progress.elapsed_ns / 1e6), + save, now, + ); +} + +/** Record a successful multiquery script run as one history entry (the whole + * script text); per-statement row counts aren't meaningful, so rows is null. */ +export function recordScriptHistory(state, sql, ms, save = saveJSON, now = Date.now()) { + pushHistory(state, sql, null, Math.round(ms), save, now); +} + /** Clear all history. */ export function clearHistory(state, save = saveJSON) { state.history = []; diff --git a/src/styles.css b/src/styles.css index 17f1cc4..d3d45be 100644 --- a/src/styles.css +++ b/src/styles.css @@ -1471,6 +1471,23 @@ table.res-table tbody tr:hover td.idx { background: var(--bg-hover); } background: color-mix(in oklab, #ef4444 12%, transparent); } +/* multiquery script summary grid */ +.script-grid table.res-table { table-layout: fixed; width: 100%; } +.script-grid td.script-sql { width: 55%; } +.script-grid td.script-sql .cell-val { font-family: var(--mono); color: var(--fg-mute); } +.script-cell { font-family: var(--mono); font-size: 12px; } +.script-cell.ok { color: #10b981; font-weight: 600; } +.script-cell.err { color: #ef4444; white-space: normal; word-break: break-word; } +.script-cell.rows { cursor: pointer; } +.script-cell.rows:hover { background: var(--hover); } +.script-cell .script-preview { color: var(--fg); } +.script-cell .script-meta { color: var(--fg-faint); margin-left: 8px; font-size: 11px; } +.script-running { + display: flex; align-items: center; gap: 8px; + padding: 10px 14px; font-size: 12px; color: var(--fg-mute); + border-top: 1px solid var(--border); +} + /* scrollbars */ *::-webkit-scrollbar { width: 10px; height: 10px; } *::-webkit-scrollbar-track { background: transparent; } diff --git a/src/ui/app.js b/src/ui/app.js index d9ceca5..feb9df1 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -7,8 +7,10 @@ import { h, zoomScale, fixedAnchor } from './dom.js'; import { Icon } from './icons.js'; import { - createState, activeTab, KEYS, recordHistory, saveQuery, savedForTab, tabChart, + createState, activeTab, KEYS, recordHistory, recordScriptHistory, saveQuery, savedForTab, tabChart, } from '../state.js'; +import { splitStatements, isRowReturning } from '../core/sql-split.js'; +import { parseSelectResult, firstRowPreview } from '../core/script-result.js'; import { saveJSON, saveStr } from '../core/storage.js'; import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, detectSqlFormat } from '../core/format.js'; @@ -434,7 +436,11 @@ export function createApp(env = {}) { async function run(opts) { if (app.state.running.value) return; // already running — cancel via cancel()/Esc const tab = app.activeTab(); - if (!tab.sql.trim()) return; + // `opts.sql` overrides the source SQL (a single selected statement); otherwise + // the whole tab runs, byte-for-byte as before (FORMAT / EXPLAIN detection, + // trailing `;`, history). + const srcSql = opts && opts.sql != null ? opts.sql : tab.sql; + if (!srcSql.trim()) return; await ensureConfig(); if (!(await getToken())) { chCtx.onSignedOut(); return; } @@ -447,10 +453,10 @@ export function createApp(env = {}) { // An explicit FORMAT clause runs raw and shows ClickHouse's response verbatim // (single raw tab). Otherwise an EXPLAIN (typed, or forced by the button) gets // the five EXPLAIN views; everything else streams structured (Table). - const explicitFmt = detectSqlFormat(tab.sql); - const parsed = explicitFmt ? null : parseExplain(tab.sql); + const explicitFmt = detectSqlFormat(srcSql); + const parsed = explicitFmt ? null : parseExplain(srcSql); const explainMode = !explicitFmt && (parsed != null || app.state.forceExplain); - let runSql = tab.sql; + let runSql = srcSql; let fmt; let explainView = null; if (explainMode) { @@ -463,9 +469,9 @@ export function createApp(env = {}) { || (parsed && detectExplainView(parsed)) || 'explain'; fmt = (EXPLAIN_VIEWS.find((v) => v.id === explainView) || EXPLAIN_VIEWS[0]).chFormat; - const inner = parsed ? parsed.inner : tab.sql; + const inner = parsed ? parsed.inner : srcSql; runSql = explainView === 'explain' - ? (parsed ? tab.sql : 'EXPLAIN ' + tab.sql) + ? (parsed ? srcSql : 'EXPLAIN ' + srcSql) : buildExplainQuery(inner, explainView); } else { fmt = explicitFmt || 'Table'; @@ -520,9 +526,99 @@ export function createApp(env = {}) { // render the final stats, so elapsed_ns must already be recorded. (Old // explicit setRunBtn(false)/renderResults are now those effects' job.) app.state.running.value = false; - if (!tab.result.error && !tab.result.cancelled) app.recordHistory(tab); + if (!tab.result.error && !tab.result.cancelled) app.recordHistory(tab, opts && opts.sql); } } + + // Run a `;`-separated script sequentially: one ClickHouse request per statement + // (CH's HTTP interface runs exactly one statement per request), stopping on the + // first failure. Row-returning statements (SELECT/WITH/SHOW/…) are fetched as + // JSONCompact capped at 100 rows; everything else runs for effect and reports + // OK. The result is a per-statement summary grid (tab.result.script). The whole + // script is recorded as one history entry on a clean run. `originalInput` is the + // exact text that was split (the selection or the whole editor). + async function runScript(statements, originalInput) { + if (app.state.running.value) return; + await ensureConfig(); + if (!(await getToken())) { chCtx.onSignedOut(); return; } + app.state.forceExplain = false; + const tab = app.activeTab(); + const t0 = now(); + const entries = []; + tab.result = { script: entries }; + app.state.resultSort = { col: null, dir: 'asc' }; + app.state.runT0 = t0; + app.state.abortController = new AbortController(); + app.state.runTick = setInterval(tickElapsed, 100); + let aborted = false; + app.state.running.value = true; // the results effect paints the (empty) grid + try { + for (let i = 0; i < statements.length; i++) { + const stmt = statements[i]; + const rowReturning = isRowReturning(stmt); + // Fresh query_id per statement, published before the request so Cancel + // issues KILL QUERY against the statement that's actually running. + app.state.runQueryId = cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'q' + t0 + '_' + i; + let out; + try { + out = await ch.runQuery(chCtx, stmt, { + format: rowReturning ? 'JSONCompact' : 'TSV', + queryId: app.state.runQueryId, + signal: app.state.abortController.signal, + params: rowReturning ? { max_result_rows: 100, result_overflow_mode: 'break' } : undefined, + }); + } catch (e) { + if (e.name === 'AbortError') { aborted = true; break; } + out = { error: e instanceof TypeError ? 'Network error' : String((e && e.message) || e) }; + } + if (out.error != null) { + entries.push({ sql: stmt, status: 'error', error: out.error }); + renderResults(app); + break; // stop-on-first-failure: skip the remaining statements + } + if (rowReturning) { + const sel = parseSelectResult(out.raw, 100); + entries.push({ sql: stmt, status: 'rows', columns: sel.columns, rows: sel.rows, truncated: sel.truncated, preview: firstRowPreview(sel.rows) }); + } else { + entries.push({ sql: stmt, status: 'ok' }); + } + renderResults(app); + } + } finally { + clearInterval(app.state.runTick); + app.state.runTick = null; + app.state.abortController = null; + app.state.runQueryId = null; + app.state.runT0 = null; + tab.result.elapsedMs = now() - t0; + if (aborted) tab.result.cancelled = true; + app.state.running.value = false; + // One history entry for the whole script — but only on a clean run (mirrors + // run(): no history for an aborted or failed script). + if (!aborted && !entries.some((e) => e.status === 'error')) { + recordScriptHistory(app.state, originalInput, tab.result.elapsedMs, saveJSON); + if (app.state.sidePanel.value === 'history') renderSavedHistory(app); + } + } + } + + // The Run button / ⌘+Enter entry point. A non-empty (non-whitespace) editor + // selection runs just that text; otherwise the whole tab. The chosen text is + // split: one statement keeps today's rich Table/Chart/EXPLAIN path (run()); + // more than one runs sequentially as a script (runScript). + function runEntry(opts) { + if (app.state.running.value) return; + const ta = app.dom.editorTextarea; + const sel = ta ? ta.value.slice(ta.selectionStart, ta.selectionEnd) : ''; + const hasSel = sel.trim() !== ''; + const input = hasSel ? sel : app.activeTab().sql; + const statements = splitStatements(input); + // >1 statement → script grid (a remembered single-result view doesn't apply). + if (statements.length > 1) return runScript(statements, input); + // 1 statement → today's rich path. Forward opts (e.g. a saved query's + // remembered view / Explain); a selection adds the sql override. + return run(hasSel ? { ...opts, sql: input } : opts); + } // Stop an in-flight query: abort the stream and KILL QUERY on the server. function cancel() { if (!app.state.running.value) return; @@ -532,10 +628,12 @@ export function createApp(env = {}) { function setRunBtn(running) { if (!app.dom.runBtn) return; app.dom.runBtn.disabled = running; - // Build the children and drop the null (replaceChildren would otherwise - // coerce a null arg into a "null" text node → "Running…null"). + // "Run selection" while the editor has a non-empty selection (so the mode is + // discoverable); plain "Run" otherwise. Build the children and drop the null + // (replaceChildren would coerce a null arg into a "null" text node). + const label = running ? 'Running…' : (app.state.hasSelection.value ? 'Run selection' : 'Run'); app.dom.runBtn.replaceChildren( - ...[Icon.play(), h('span', null, running ? 'Running…' : 'Run'), + ...[Icon.play(), h('span', null, label), running ? null : h('kbd', null, '⌘↵')].filter(Boolean)); } app.setRunBtn = setRunBtn; @@ -676,8 +774,8 @@ export function createApp(env = {}) { } // --- saved / history bridges ------------------------------------------ - app.recordHistory = (tab) => { - recordHistory(app.state, tab, saveJSON); + app.recordHistory = (tab, sqlText) => { + recordHistory(app.state, tab, saveJSON, undefined, sqlText); if (app.state.sidePanel.value === 'history') renderSavedHistory(app); }; @@ -847,7 +945,7 @@ export function createApp(env = {}) { // --- actions registry -------------------------------------------------- app.actions = { - run, + run: runEntry, cancel, newTab: () => newTab(app), selectTab: (id) => selectTab(app, id), @@ -996,8 +1094,20 @@ export function renderApp(app, helpers) { app.state.running.value; renderResults(app); }); - // The Run button reflects the run state (label + disabled). - effect(() => app.setRunBtn(app.state.running.value)); + // The Run button reflects the run state (label + disabled) and the selection + // (Run ↔ Run selection). + effect(() => { app.state.hasSelection.value; app.setRunBtn(app.state.running.value); }); + // Track the editor's text selection into a signal so the Run button label and + // ⌘+Enter target just the highlighted text. `selectionchange` is the one event + // that fires for keyboard, mouse, and programmatic selection; gate on the + // editor being focused so selecting elsewhere (results, address bar) is ignored. + app.syncSelection = () => { + const ta = app.dom.editorTextarea; + const focused = ta && (app.document || document).activeElement === ta; + const sel = focused ? ta.value.slice(ta.selectionStart, ta.selectionEnd) : ''; + app.state.hasSelection.value = sel.trim() !== ''; + }; + (app.document || document).addEventListener('selectionchange', app.syncSelection); // Reactive repaint of the schema tree — replaces the scattered renderSchema() // calls: re-runs on schema load, load error, filter text, or expand/collapse. // Registered here (post-mount) so app.dom.schemaList already exists; the effect diff --git a/src/ui/results.js b/src/ui/results.js index 3d41a28..73f26ca 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -93,6 +93,14 @@ export function renderResults(app) { // While running, pin a streaming strip to the top of the body: a determinate // fill at read/total when known, else an indeterminate sweep. if (app.state.running.value) inner.appendChild(streamStrip(r)); + // Multiquery script: a per-statement summary grid. Handled before the + // single-result chain below (a script result has no `rows`/`rawText`). + if (r && r.script) { + inner.appendChild(renderScriptGrid(app, r)); + body.appendChild(inner); + region.replaceChildren(body); + return; + } const streamingBlank = app.state.running.value && (!r || (r.rows.length === 0 && r.rawText == null)); if (streamingBlank) { inner.appendChild(h('div', { class: 'placeholder starting' }, @@ -149,8 +157,124 @@ function streamStrip(r) { : h('i', { class: 'sweep' })); } +// The multiquery summary grid: one row per executed statement. Col 1 is the +// collapsed statement text (full text on hover); Col 2 is the outcome — OK for an +// effectful statement (DDL/INSERT), the first-row preview for a SELECT (click to +// open all rows in a side pane), or the error for the failing statement (the last +// row, since the run stops on first failure). +function renderScriptGrid(app, r) { + const wrap = h('div', { class: 'res-table-wrap script-grid' }); + const table = document.createElement('table'); + table.className = 'res-table'; + const thead = document.createElement('thead'); + const trh = document.createElement('tr'); + trh.appendChild(h('th', null, 'Statement')); + trh.appendChild(h('th', null, 'Result')); + thead.appendChild(trh); + table.appendChild(thead); + const tbody = document.createElement('tbody'); + r.script.forEach((e) => { + const tr = document.createElement('tr'); + tr.appendChild(h('td', { class: 'script-sql', title: e.sql || '' }, + h('div', { class: 'cell-val' }, (e.sql || '').replace(/\s+/g, ' ').trim()))); + tr.appendChild(scriptOutcomeCell(app, e)); + tbody.appendChild(tr); + }); + table.appendChild(tbody); + wrap.appendChild(table); + if (app.state.running.value) { + wrap.appendChild(h('div', { class: 'script-running' }, + h('span', { class: 'spin' }, Icon.spinner()), h('span', null, 'Running…'))); + } + return wrap; +} + +// Column 2 of one script row, by outcome. +function scriptOutcomeCell(app, e) { + if (e.status === 'error') return h('td', { class: 'script-cell err' }, e.error || 'Error'); + if (e.status === 'ok') return h('td', { class: 'script-cell ok' }, 'OK'); + // status === 'rows' + if (!e.rows || !e.rows.length) return h('td', { class: 'script-cell' }, '(0 rows)'); + const n = e.rows.length; + const meta = '(' + n + ' row' + (n === 1 ? '' : 's') + (e.truncated ? ', first 100' : '') + ')'; + return h('td', { + class: 'script-cell rows', title: 'Click to view all rows', + onclick: () => openRowsViewer(app, e), + }, h('span', { class: 'script-preview' }, e.preview || ''), h('span', { class: 'script-meta' }, meta)); +} + +/** + * Open a right-side pane with the full rows of one script SELECT, as a static + * table. Reuses the cell-detail drawer scaffold (.cd-*); a shared Drawer + * primitive is deferred to #60. Escape / backdrop / ✕ closes. Exported for tests. + */ +export function openRowsViewer(app, entry) { + const doc = app.document || document; + let backdrop; + const onKey = (ev) => { if (ev.key === 'Escape') close(); }; + function close() { + if (backdrop) backdrop.remove(); + doc.removeEventListener('keydown', onKey, true); + } + const n = entry.rows.length; + const head = h('div', { class: 'cd-head' }, + h('div', { class: 'cd-title' }, + h('span', { class: 'cd-name' }, 'Result rows'), + h('span', { class: 'cd-type' }, n + (entry.truncated ? '+' : '') + ' row' + (n === 1 ? '' : 's'))), + h('button', { class: 'cd-close', title: 'Close (Esc)', onclick: close }, Icon.close())); + const body = h('div', { class: 'cd-body' }, scriptRowsTable(entry.columns || [], entry.rows)); + const panel = h('div', { class: 'cd-panel', onclick: (ev) => ev.stopPropagation() }, head, body); + backdrop = h('div', { class: 'cd-backdrop', onclick: close }, panel); + doc.body.appendChild(backdrop); + doc.addEventListener('keydown', onKey, true); + return backdrop; +} + +// A plain (no sort / resize) table of a script SELECT's rows for the side pane. +function scriptRowsTable(columns, rows) { + const table = document.createElement('table'); + table.className = 'res-table'; + const trh = document.createElement('tr'); + trh.appendChild(h('th', { class: 'idx' }, '#')); + columns.forEach((c) => trh.appendChild(h('th', { title: c.type || '' }, c.name))); + const thead = document.createElement('thead'); + thead.appendChild(trh); + table.appendChild(thead); + const tbody = document.createElement('tbody'); + rows.forEach((row, ri) => { + const tr = document.createElement('tr'); + tr.appendChild(h('td', { class: 'idx' }, String(ri + 1))); + row.forEach((v) => tr.appendChild(h('td', { class: 'cell' }, h('div', { class: 'cell-val' }, v == null ? '' : String(v))))); + tbody.appendChild(tr); + }); + table.appendChild(tbody); + return h('div', { class: 'res-table-wrap' }, table); +} + function buildToolbar(app, r) { const toolbar = h('div', { class: 'res-toolbar' }); + if (r && r.script) { + // Script view: a title (N statements) + live elapsed / Cancel while running, + // else the total elapsed. No view-switcher / copy / export (each statement + // owns its own preview + rows pane). + const n = r.script.length; + toolbar.appendChild(h('div', { class: 'result-view-tabs' }, + h('span', { class: 'res-graph-title' }, 'Script · ' + n + ' statement' + (n === 1 ? '' : 's')))); + toolbar.appendChild(h('div', { style: { flex: '1' } })); + if (app.state.running.value) { + app.dom.runElapsedEl = h('span', { class: 'v' }, app.elapsedMs().toFixed(0) + ' ms'); + toolbar.appendChild(h('div', { class: 'stat live' }, h('span', { class: 'ic spin' }, Icon.spinner()), app.dom.runElapsedEl)); + toolbar.appendChild(h('button', { + class: 'res-act cancel-act', title: 'Cancel script (Esc)', + onclick: () => app.actions.cancel(), + }, Icon.close(), h('span', null, 'Cancel'), h('kbd', null, 'Esc'))); + } else { + if (r.cancelled) toolbar.appendChild(h('span', { class: 'cancelled-badge' }, 'Cancelled · partial')); + toolbar.appendChild(h('div', { class: 'stat' }, h('span', { class: 'ic' }, Icon.clock()), + h('span', { class: 'v' }, (r.elapsedMs || 0).toFixed(0) + ' ms'))); + } + return toolbar; + } if (r && r.schemaGraph) { // Schema-lineage view: a title + Expand (fullscreen); no view-switcher / stats. const f = r.schemaGraph.focus || {}; diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index b8b2961..a55ca4d 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -475,6 +475,122 @@ describe('query run', () => { await app.actions.run(); expect(app.activeTab().result.rawFormat).toBe('JSON'); // FORMAT clause, not the EXPLAIN default }); + + // ── multiquery / run-selection (#83) ────────────────────────────────────── + const SCRIPT = 'CREATE TABLE t (a Int8); INSERT INTO t VALUES (1); SELECT count() AS c FROM t'; + const scriptRoutes = () => [ + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), resp({ text: '' })], + [(u, sql) => /SELECT count/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'c', type: 'UInt64' }], data: [['1']] }) })], + ]; + + it('runs a ;-separated script sequentially, one summary row per statement, and records one history entry', async () => { + const { app } = appForRun(scriptRoutes()); + app.state.sidePanel.value = 'history'; // exercise the history repaint in the finally + app.activeTab().sql = SCRIPT; + await app.actions.run(); + const script = app.activeTab().result.script; + expect(script.map((e) => e.status)).toEqual(['ok', 'ok', 'rows']); + expect(script[2]).toMatchObject({ preview: '1', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1']] }); + expect(app.state.history).toHaveLength(1); + expect(app.state.history[0].sql).toBe(SCRIPT); + // SELECT statements are sent with the JSONCompact + row-cap params. + const selUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /max_result_rows=100/.test(u)); + expect(selUrl).toMatch(/result_overflow_mode=break/); + }); + + it('stops on the first failing statement and skips the rest (no history)', async () => { + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), resp({ ok: false, status: 500, text: 'DB::Exception: boom' })], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + const script = app.activeTab().result.script; + expect(script).toHaveLength(2); // CREATE ok, INSERT error; SELECT never run + expect(script[1]).toMatchObject({ status: 'error' }); + expect(script[1].error).toMatch(/boom/); + expect(app.state.history).toHaveLength(0); + }); + + it('reports a network error (TypeError) per statement', async () => { + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), () => { throw new TypeError('fetch failed'); }], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result.script[0]).toMatchObject({ status: 'error', error: 'Network error' }); + }); + + it('surfaces a non-abort thrown error message per statement', async () => { + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), () => { throw new Error('kaput'); }], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result.script[0]).toMatchObject({ status: 'error', error: 'kaput' }); + }); + + it('aborts mid-script: marks the result cancelled and records no history', async () => { + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), () => { const e = new Error('aborted'); e.name = 'AbortError'; throw e; }], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result.cancelled).toBe(true); + expect(app.activeTab().result.script).toHaveLength(1); // CREATE ran; INSERT aborted before pushing + expect(app.state.history).toHaveLength(0); + }); + + it('run-selection: a non-empty selection runs only the selected statement (rich path) and records that text', async () => { + const { app } = appForRun([ + [(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })], + ]); + const ta = app.dom.editorTextarea; + ta.value = 'SELECT 1; SELECT 2'; + ta.selectionStart = 0; ta.selectionEnd = 8; // "SELECT 1" + app.activeTab().sql = ta.value; + await app.actions.run(); + expect(app.activeTab().result.rows).toEqual([['1']]); // single-statement rich path, not the script grid + expect(app.activeTab().result.script).toBeUndefined(); + expect(app.state.history[0].sql).toBe('SELECT 1'); // the selection, not the whole editor + }); + + it('runEntry while already running is a no-op', async () => { + const { app } = appForRun([]); + app.state.running.value = true; + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result).toBeNull(); + }); + + it('a signed-out script run hits onSignedOut and produces no result', async () => { + const app = createApp(env({ sessionStorage: memSession({}) })); + app.renderApp(); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result).toBeNull(); // returns before building the grid + }); + + it('syncSelection drives hasSelection; setRunBtn flips to "Run selection"', () => { + const { app } = appForRun([]); + const ta = app.dom.editorTextarea; + ta.value = 'SELECT 1; SELECT 2'; + ta.focus(); + ta.selectionStart = 0; ta.selectionEnd = 8; + app.syncSelection(); + expect(app.state.hasSelection.value).toBe(true); + app.setRunBtn(false); + expect(app.dom.runBtn.textContent).toContain('Run selection'); + // collapsed selection → false; missing textarea → false + ta.selectionEnd = 0; + app.syncSelection(); + expect(app.state.hasSelection.value).toBe(false); + app.dom.editorTextarea = null; + app.syncSelection(); + expect(app.state.hasSelection.value).toBe(false); + }); }); describe('formatQuery', () => { diff --git a/tests/unit/results.test.js b/tests/unit/results.test.js index 90ebb18..fe0b747 100644 --- a/tests/unit/results.test.js +++ b/tests/unit/results.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { renderResults, renderJson, renderTable, renderChart, colResizeWidth, openCellDetail, installChartZoomFix } from '../../src/ui/results.js'; +import { renderResults, renderJson, renderTable, renderChart, colResizeWidth, openCellDetail, openRowsViewer, installChartZoomFix } from '../../src/ui/results.js'; import { makeApp } from '../helpers/fake-app.js'; import { newResult } from '../../src/core/stream.js'; import { schemaKey } from '../../src/core/chart-data.js'; @@ -623,3 +623,94 @@ describe('schema lineage result', () => { expect([...region.querySelectorAll('.res-act')].find((b) => /Expand/.test(b.textContent))).toBeFalsy(); }); }); + +describe('multiquery script grid (#83)', () => { + const scriptResult = (over = {}) => ({ + elapsedMs: 12, + script: [ + { sql: 'CREATE TABLE t (a Int8)', status: 'ok' }, + { sql: 'SELECT count() AS c\nFROM t', status: 'rows', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1'], ['2']], truncated: false, preview: '1' }, + { sql: 'SELECT * FROM nope', status: 'rows', columns: [], rows: [] }, + { sql: 'BAD SQL', status: 'error', error: 'DB::Exception: boom' }, + ], + ...over, + }); + + it('renders one row per statement with OK / preview / 0-rows / error outcomes', () => { + const app = appWithResult(scriptResult()); + renderResults(app); + const region = app.dom.resultsRegion; + expect(region.querySelector('.script-grid')).not.toBeNull(); + expect(region.querySelector('.res-graph-title').textContent).toContain('4 statements'); + const cells = [...region.querySelectorAll('.script-cell')]; + expect(cells[0].textContent).toBe('OK'); + expect(cells[1].textContent).toContain('1'); // preview + expect(cells[1].textContent).toContain('2 rows'); + expect(cells[2].textContent).toBe('(0 rows)'); + expect(cells[3].textContent).toContain('boom'); + // SQL is collapsed to one line, full text on the title attribute + const sqlCell = region.querySelector('.script-sql'); + expect(sqlCell.querySelector('.cell-val').textContent).toBe('CREATE TABLE t (a Int8)'); + }); + + it('flags a truncated SELECT in its row meta', () => { + const app = appWithResult(scriptResult({ + script: [{ sql: 'SELECT * FROM big', status: 'rows', columns: [{ name: 'a', type: 'Int' }], rows: [['x']], truncated: true, preview: 'x' }], + })); + renderResults(app); + expect(app.dom.resultsRegion.querySelector('.script-cell.rows').textContent).toContain('first 100'); + }); + + it('clicking a SELECT row opens the rows pane; Escape and backdrop close it', () => { + const app = appWithResult(scriptResult()); + renderResults(app); + click(app.dom.resultsRegion.querySelector('.script-cell.rows')); + let backdrop = document.querySelector('.cd-backdrop'); + expect(backdrop).not.toBeNull(); + expect(backdrop.querySelectorAll('tbody tr')).toHaveLength(2); // both rows + expect(backdrop.querySelector('.cd-type').textContent).toContain('2 rows'); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + expect(document.querySelector('.cd-backdrop')).toBeNull(); + // reopen + close via backdrop click + click(app.dom.resultsRegion.querySelector('.script-cell.rows')); + backdrop = document.querySelector('.cd-backdrop'); + click(backdrop); + expect(document.querySelector('.cd-backdrop')).toBeNull(); + }); + + it('openRowsViewer tolerates missing columns and renders NULL cells empty', () => { + const app = makeApp(); + openRowsViewer(app, { rows: [['a', null]], truncated: true }); + const backdrop = document.querySelector('.cd-backdrop'); + expect(backdrop.querySelector('.cd-type').textContent).toContain('1+ row'); + const cells = [...backdrop.querySelectorAll('tbody td')]; + expect(cells[cells.length - 1].textContent).toBe(''); // null → empty + backdrop.remove(); + }); + + it('toolbar shows live elapsed + Cancel while running, with a running footer', () => { + const app = appWithResult(scriptResult(), { running: true }); + renderResults(app); + const region = app.dom.resultsRegion; + const cancel = region.querySelector('.cancel-act'); + expect(cancel).not.toBeNull(); + expect(region.querySelector('.script-running')).not.toBeNull(); + click(cancel); + expect(app.actions.cancel).toHaveBeenCalled(); + }); + + it('toolbar shows total elapsed + a cancelled badge when a script was aborted', () => { + const app = appWithResult(scriptResult({ cancelled: true })); + renderResults(app); + const region = app.dom.resultsRegion; + expect(region.querySelector('.cancelled-badge')).not.toBeNull(); + expect(region.textContent).toContain('12 ms'); + expect(region.querySelector('.script-running')).toBeNull(); + }); + + it('handles a single-statement script label without an "s"', () => { + const app = appWithResult(scriptResult({ script: [{ sql: 'SELECT 1', status: 'ok' }] })); + renderResults(app); + expect(app.dom.resultsRegion.querySelector('.res-graph-title').textContent).toContain('1 statement'); + }); +}); diff --git a/tests/unit/state.test.js b/tests/unit/state.test.js index 26c26f9..306a103 100644 --- a/tests/unit/state.test.js +++ b/tests/unit/state.test.js @@ -2,7 +2,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { KEYS, DEFAULT_LIBRARY_NAME, newTabObj, createState, activeTab, allocTabId, saveQuery, savedForTab, renameSaved, toggleFavorite, sortedSaved, filterSaved, filterHistory, importSaved, - deleteSaved, recordHistory, clearHistory, deleteHistory, tabChart, + deleteSaved, recordHistory, recordScriptHistory, clearHistory, deleteHistory, tabChart, renameLibrary, newLibrary, replaceLibrary, appendLibrary, markLibrarySaved, } from '../../src/state.js'; @@ -422,6 +422,23 @@ describe('history', () => { recordHistory(s, tab({ result: { rawText: 'x', rows: [], progress: { elapsed_ns: 0 } } }), vi.fn()); expect(s.history[0].rows).toBeNull(); }); + it('recordHistory records sqlText override (selection run) over tab.sql', () => { + const s = createState(reader()); + recordHistory(s, tab(), vi.fn(), 1000, 'SELECT just_this'); + expect(s.history[0]).toMatchObject({ sql: 'SELECT just_this', rows: 2 }); + }); + it('recordScriptHistory records the whole script with null rows', () => { + const s = createState(reader()); + const save = vi.fn(); + recordScriptHistory(s, 'CREATE x; INSERT y; SELECT z', 12.6, save, 2000); + expect(s.history[0]).toMatchObject({ sql: 'CREATE x; INSERT y; SELECT z', ts: 2000, rows: null, ms: 13 }); + expect(save).toHaveBeenCalledWith(KEYS.history, s.history); + }); + it('recordScriptHistory skips empty script text', () => { + const s = createState(reader()); + recordScriptHistory(s, ' ', 5, vi.fn()); + expect(s.history).toHaveLength(0); + }); it('recordHistory caps at 50 entries', () => { const s = createState(reader()); s.history = Array.from({ length: 50 }, (_, i) => ({ id: 'h' + i })); From c0acb5e1e250afa58ba7c5b1c55352859eacc593 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:23:14 +0000 Subject: [PATCH 04/12] fix(multiquery): address code-review findings (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Truncation was undetectable: result_overflow_mode=break capped at exactly max_result_rows, so `data.length > cap` was never true and the "first N" badge never showed. Over-fetch by one (cap+1) so truncation is detectable. - leadingKeyword now skips a leading `(`, so `(SELECT …) UNION …` is classified row-returning instead of silently running as an effectful "OK" with rows hidden. - exportableResult() guards the script result shape (no rows/rawText) so a future Copy/Export binding can't TypeError on it. - runEntry no-ops when the split yields nothing (comment-/whitespace-only selection no longer POSTs a comment). - Single SELECT_ROW_CAP constant replaces the 100 literal repeated across the runner, parser, and grid label. --- src/core/script-result.js | 14 +++++++++---- src/core/sql-split.js | 6 ++++-- src/ui/app.js | 12 ++++++++---- src/ui/results.js | 3 ++- tests/unit/app.test.js | 38 ++++++++++++++++++++++++++++++++++-- tests/unit/sql-split.test.js | 7 ++++++- 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/core/script-result.js b/src/core/script-result.js index 52d00d6..5b7e9d6 100644 --- a/src/core/script-result.js +++ b/src/core/script-result.js @@ -5,13 +5,19 @@ // grid (renderTable) consumes. The script summary grid shows a one-line preview // of the first row in column 2; clicking it opens the full table in a side pane. +// The display cap for a script-mode SELECT. The runner asks the server for +// SELECT_ROW_CAP + 1 rows (so it can tell a result was truncated — at exactly +// the cap it can't) and shows at most SELECT_ROW_CAP. +export const SELECT_ROW_CAP = 100; + /** * Parse a JSONCompact response body into `{ columns, rows, truncated }`, capping - * `rows` at `cap` (default 100; the server also caps via max_result_rows, so this - * is a display backstop). A blank body or one that isn't valid JSON yields an - * empty result rather than throwing. Pure. + * `rows` at `cap` (default SELECT_ROW_CAP). `truncated` is true when more than + * `cap` rows came back (the runner over-fetches by one to detect this). A blank + * body or one that isn't valid JSON yields an empty result rather than throwing. + * Pure. */ -export function parseSelectResult(rawText, cap = 100) { +export function parseSelectResult(rawText, cap = SELECT_ROW_CAP) { const text = String(rawText == null ? '' : rawText).trim(); if (!text) return { columns: [], rows: [], truncated: false }; let json; diff --git a/src/core/sql-split.js b/src/core/sql-split.js index 3ae779d..e569046 100644 --- a/src/core/sql-split.js +++ b/src/core/sql-split.js @@ -80,7 +80,8 @@ const ROW_RETURNING = new Set([ ]); /** The first SQL keyword of `stmt`, uppercased, after skipping leading - * whitespace and -- / # / block comments. '' when none. Pure. */ + * whitespace, -- / # / block comments, and `(` (so a parenthesized + * `(SELECT …) UNION …` is still recognized as row-returning). '' when none. Pure. */ export function leadingKeyword(stmt) { let s = String(stmt || ''); for (;;) { @@ -88,7 +89,8 @@ export function leadingKeyword(stmt) { s = s.replace(/^\s+/, '') .replace(/^--[^\n]*/, '') .replace(/^#[^\n]*/, '') - .replace(/^\/\*[\s\S]*?\*\//, ''); + .replace(/^\/\*[\s\S]*?\*\//, '') + .replace(/^\(+/, ''); if (s === before) break; } const m = /^([A-Za-z]+)/.exec(s); diff --git a/src/ui/app.js b/src/ui/app.js index feb9df1..e0e17d9 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -10,7 +10,7 @@ import { createState, activeTab, KEYS, recordHistory, recordScriptHistory, saveQuery, savedForTab, tabChart, } from '../state.js'; import { splitStatements, isRowReturning } from '../core/sql-split.js'; -import { parseSelectResult, firstRowPreview } from '../core/script-result.js'; +import { parseSelectResult, firstRowPreview, SELECT_ROW_CAP } from '../core/script-result.js'; import { saveJSON, saveStr } from '../core/storage.js'; import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, detectSqlFormat } from '../core/format.js'; @@ -565,7 +565,9 @@ export function createApp(env = {}) { format: rowReturning ? 'JSONCompact' : 'TSV', queryId: app.state.runQueryId, signal: app.state.abortController.signal, - params: rowReturning ? { max_result_rows: 100, result_overflow_mode: 'break' } : undefined, + // Over-fetch by one past the display cap so a truncated result is + // detectable (at exactly the cap the client can't tell it was cut). + params: rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : undefined, }); } catch (e) { if (e.name === 'AbortError') { aborted = true; break; } @@ -577,7 +579,7 @@ export function createApp(env = {}) { break; // stop-on-first-failure: skip the remaining statements } if (rowReturning) { - const sel = parseSelectResult(out.raw, 100); + const sel = parseSelectResult(out.raw, SELECT_ROW_CAP); entries.push({ sql: stmt, status: 'rows', columns: sel.columns, rows: sel.rows, truncated: sel.truncated, preview: firstRowPreview(sel.rows) }); } else { entries.push({ sql: stmt, status: 'ok' }); @@ -613,6 +615,7 @@ export function createApp(env = {}) { const hasSel = sel.trim() !== ''; const input = hasSel ? sel : app.activeTab().sql; const statements = splitStatements(input); + if (!statements.length) return; // nothing runnable (empty / comments-only) // >1 statement → script grid (a remembered single-result view doesn't apply). if (statements.length > 1) return runScript(statements, input); // 1 statement → today's rich path. Forward opts (e.g. a saved query's @@ -799,7 +802,8 @@ export function createApp(env = {}) { // A result is exportable once it has raw text or at least one row. function exportableResult() { const r = app.activeTab().result; - return r && !r.error && (r.rawText != null || r.rows.length > 0) ? r : null; + // A script result is a per-statement grid, not a single exportable table. + return r && !r.error && !r.script && (r.rawText != null || r.rows.length > 0) ? r : null; } function copyResult() { const r = exportableResult(); diff --git a/src/ui/results.js b/src/ui/results.js index 73f26ca..0300b9f 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -9,6 +9,7 @@ import { looksLikeHtml, prettyValue } from '../core/cell.js'; import { sortRows } from '../core/sort.js'; import { autoChart, schemaKey, chartFieldOptions, chartColors, chartJsConfig, chartCfgValid, normalizeChartCfg, unzoomChartEvent, CHART_ROW_CAP } from '../core/chart-data.js'; import { EXPLAIN_VIEWS } from '../core/explain.js'; +import { SELECT_ROW_CAP } from '../core/script-result.js'; import { renderExplainGraph, openPipelineFullscreen, renderSchemaGraph } from './explain-graph.js'; // View id → tab glyph for the EXPLAIN view strip (kept here so core/explain.js @@ -196,7 +197,7 @@ function scriptOutcomeCell(app, e) { // status === 'rows' if (!e.rows || !e.rows.length) return h('td', { class: 'script-cell' }, '(0 rows)'); const n = e.rows.length; - const meta = '(' + n + ' row' + (n === 1 ? '' : 's') + (e.truncated ? ', first 100' : '') + ')'; + const meta = '(' + n + ' row' + (n === 1 ? '' : 's') + (e.truncated ? ', first ' + SELECT_ROW_CAP : '') + ')'; return h('td', { class: 'script-cell rows', title: 'Click to view all rows', onclick: () => openRowsViewer(app, e), diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index a55ca4d..f4f6b63 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -494,11 +494,45 @@ describe('query run', () => { expect(script[2]).toMatchObject({ preview: '1', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1']] }); expect(app.state.history).toHaveLength(1); expect(app.state.history[0].sql).toBe(SCRIPT); - // SELECT statements are sent with the JSONCompact + row-cap params. - const selUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /max_result_rows=100/.test(u)); + // SELECT statements are sent with the JSONCompact + row-cap params + // (over-fetched by one past the display cap to detect truncation). + const selUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /max_result_rows=101/.test(u)); expect(selUrl).toMatch(/result_overflow_mode=break/); }); + it('flags a SELECT as truncated when more than the cap rows come back', async () => { + const data = Array.from({ length: 101 }, (_, i) => [String(i)]); + const { app } = appForRun([ + [(u, sql) => /SELECT/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'n', type: 'Int' }], data }) })], + ]); + app.activeTab().sql = 'SELECT 1; SELECT 2'; // two statements → script mode + await app.actions.run(); + const last = app.activeTab().result.script[1]; + expect(last.rows).toHaveLength(100); // displayed cap + expect(last.truncated).toBe(true); + }); + + it('a comment-only selection is a no-op (nothing is sent)', async () => { + const { app } = appForRun([]); + const ta = app.dom.editorTextarea; + ta.value = '-- just a note'; + ta.selectionStart = 0; ta.selectionEnd = ta.value.length; + app.activeTab().sql = 'SELECT 1'; + await app.actions.run(); + expect(app.activeTab().result).toBeNull(); // no run started + // the comment text was never POSTed to ClickHouse + expect(app.chCtx.fetch.mock.calls.some((c) => /just a note/.test(c[1] && c[1].body))).toBe(false); + }); + + it('copy/export treat a script result as non-exportable (no throw)', async () => { + const { app } = appForRun(scriptRoutes()); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(app.activeTab().result.script).toHaveLength(3); + expect(() => app.actions.copyResult()).not.toThrow(); + expect(() => app.actions.exportResult()).not.toThrow(); + }); + it('stops on the first failing statement and skips the rest (no history)', async () => { const { app } = appForRun([ [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], diff --git a/tests/unit/sql-split.test.js b/tests/unit/sql-split.test.js index f8f12fa..73816a1 100644 --- a/tests/unit/sql-split.test.js +++ b/tests/unit/sql-split.test.js @@ -87,6 +87,10 @@ describe('leadingKeyword', () => { it('skips leading whitespace and comments of every kind', () => { expect(leadingKeyword(' \n -- note\n # bang\n /* block */ SELECT 1')).toBe('SELECT'); }); + it('skips leading parentheses so a parenthesized SELECT is recognized', () => { + expect(leadingKeyword('((SELECT 1) UNION ALL (SELECT 2))')).toBe('SELECT'); + expect(leadingKeyword('/* c */ ( select 1 )')).toBe('SELECT'); + }); it('returns "" when there is no leading keyword', () => { expect(leadingKeyword('')).toBe(''); expect(leadingKeyword('-- only a comment')).toBe(''); @@ -97,7 +101,8 @@ describe('leadingKeyword', () => { describe('isRowReturning', () => { it('is true for row-bearing statements', () => { for (const s of ['SELECT 1', 'with x as (select 1) select * from x', - 'SHOW TABLES', 'DESC t', 'DESCRIBE t', 'EXISTS TABLE t', 'VALUES (1)', 'EXPLAIN SELECT 1']) { + 'SHOW TABLES', 'DESC t', 'DESCRIBE t', 'EXISTS TABLE t', 'VALUES (1)', 'EXPLAIN SELECT 1', + '(SELECT 1) UNION ALL (SELECT 2)']) { expect(isRowReturning(s)).toBe(true); } }); From b2cc694cd5424d8a36273a34aa3b9ed138a8a8aa Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:25:05 +0000 Subject: [PATCH 05/12] docs(changelog): note multiquery + run-selection under [Unreleased] (#83) --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8c5cb..c51b077 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Added +- **Multiquery + run-selection** (#83): run a `;`-separated script (DDL / INSERT / + SELECT) in one shot, or run just the highlighted text. ⌘+Enter auto-detects — a + single statement behaves exactly as before; more than one runs **sequentially** + (one ClickHouse request per statement, stopping on the first failure) into a + compact per-statement summary grid. A non-empty editor selection runs only that + text (the Run button flips to **Run selection**); a single selected statement + still gets the full Table/Chart/EXPLAIN view. Row-returning statements show the + first row inline (comma-separated) — click to open all rows (capped at 100) in a + side pane; effectful statements show **OK**. Cancel aborts mid-script. Splitting + is purely lexical (`src/core/sql-split.js`), skipping `;` inside string/identifier + literals and `--` / `#` / `/* */` comments. Known limitation: an `INSERT … FORMAT + …` with inline data containing `;` mis-splits — run those on their own. - Playwright e2e now runs on **WebKit** in addition to Chromium and Firefox, so many Safari regressions on the `html{zoom}`-based layout fail CI instead of shipping silently. README gained a **Supported browsers** stance: desktop From 62a60963197cebcfb1aadf738c8b6d4228cb522f Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 16:51:00 +0000 Subject: [PATCH 06/12] feat(multiquery): per-statement format, explain guard, gated saved auto-run (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up UX fixes for multi-statement scripts: - Format now formats each statement via formatQuery() and rejoins them with `;` + a blank line (best-effort: an unformattable statement keeps its text). The Format button shows a busy spinner ("Formatting…") since it's one request per statement. Single-statement behavior (incl. syntax-error caret reveal) unchanged. - Explain (and the EXPLAIN view-switcher) show a clear toast — "Explain isn't available for a multi-statement script — run one statement at a time." — instead of sending `EXPLAIN a; b; …` and surfacing a confusing ClickHouse parse error. - Opening a saved query / history entry auto-runs ONLY read-only queries (new pure `isAutoRunnable` — every statement row-returning). An effectful query (CREATE/ALTER/DROP/INSERT/…) loads into the editor without executing, so a destructive statement is never run just by clicking it in the sidebar. Per-file coverage gate green (1093 tests); build OK; e2e green. --- CHANGELOG.md | 6 +++ src/core/sql-split.js | 11 +++++ src/ui/app.js | 79 +++++++++++++++++++++++++------- src/ui/saved-history.js | 5 +- tests/unit/app.test.js | 57 +++++++++++++++++++++++ tests/unit/saved-history.test.js | 20 ++++++++ tests/unit/sql-split.test.js | 18 +++++++- 7 files changed, 176 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c51b077..9a98bf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ auto-generated per-PR notes; this file is the curated, human-readable history. is purely lexical (`src/core/sql-split.js`), skipping `;` inside string/identifier literals and `--` / `#` / `/* */` comments. Known limitation: an `INSERT … FORMAT …` with inline data containing `;` mis-splits — run those on their own. + **Format** pretty-prints each statement of a script and rejoins them (`;` + blank + line; best-effort — an unformattable statement keeps its text), with a busy + spinner on the button. **Explain** shows a clear message instead of a generic + ClickHouse error when the editor holds more than one statement. Opening a saved + query / history entry **auto-runs only read-only queries** — an effectful one + (CREATE/ALTER/DROP/INSERT/…) loads into the editor without executing. - Playwright e2e now runs on **WebKit** in addition to Chromium and Firefox, so many Safari regressions on the `html{zoom}`-based layout fail CI instead of shipping silently. README gained a **Supported browsers** stance: desktop diff --git a/src/core/sql-split.js b/src/core/sql-split.js index e569046..493ec8c 100644 --- a/src/core/sql-split.js +++ b/src/core/sql-split.js @@ -101,3 +101,14 @@ export function leadingKeyword(stmt) { export function isRowReturning(stmt) { return ROW_RETURNING.has(leadingKeyword(stmt)); } + +/** + * True when `sql` is safe to auto-run on open (e.g. clicking a saved query): it + * has at least one statement and **every** statement is row-returning. An + * effectful statement (CREATE/ALTER/DROP/INSERT/…) anywhere makes it false, so + * opening such a query loads it into the editor without executing it. Pure. + */ +export function isAutoRunnable(sql) { + const stmts = splitStatements(sql); + return stmts.length > 0 && stmts.every(isRowReturning); +} diff --git a/src/ui/app.js b/src/ui/app.js index e0e17d9..dafbe26 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -640,34 +640,71 @@ export function createApp(env = {}) { running ? null : h('kbd', null, '⌘↵')].filter(Boolean)); } app.setRunBtn = setRunBtn; + // Busy state for the Format button — formatting a multi-statement script is one + // request per statement, so it can take a moment; show a spinner + disable. + function setFmtBtn(busy) { + if (!app.dom.fmtBtn) return; + app.dom.fmtBtn.disabled = busy; + app.dom.fmtBtn.replaceChildren( + busy ? h('span', { class: 'spin' }, Icon.spinner()) : Icon.braces(), + busy ? 'Formatting…' : 'Format'); + } + app.setFmtBtn = setFmtBtn; // Pretty-print the editor's SQL via ClickHouse's formatQuery(), in place. The // raw (untrimmed) SQL is sent so a syntax error's reported position maps 1:1 // onto the editor text. On error we show it persistently in the results panel // and jump the caret to the offending token; a later successful format clears // that error. Success never touches real run results. + // Clear a prior format-error result (a later successful format clears just this). + function clearFormatError() { + const tab = app.activeTab(); + if (tab.result && tab.result.formatError) { tab.result = null; renderResults(app); } + } + // Format one statement via ClickHouse's formatQuery(); returns the formatted text. + const formatOne = async (s) => { + const json = await ch.queryJson(chCtx, 'SELECT formatQuery(' + sqlString(s) + ') AS q FORMAT JSON'); + return (json.data && json.data[0] && json.data[0].q) || ''; + }; + async function formatQuery() { const raw = app.activeTab().sql || ''; if (!raw.trim()) return; await ensureConfig(); if (!(await getToken())) { chCtx.onSignedOut(); return; } const tab = app.activeTab(); + const stmts = splitStatements(raw); + setFmtBtn(true); // formatting a script is one request per statement — show busy try { - const json = await ch.queryJson(chCtx, 'SELECT formatQuery(' + sqlString(raw) + ') AS q FORMAT JSON'); - const q = (json.data && json.data[0] && json.data[0].q) || ''; - // Terminate so the caret lands past the last token — otherwise the input - // event from the replace re-opens autocomplete on the trailing word. - if (q) replaceEditor(app, withStatementBreak(q)); - if (tab.result && tab.result.formatError) { tab.result = null; renderResults(app); } // clear a prior format error - } catch (e) { - const msg = String((e && e.message) || e); - tab.result = newResult('Table'); - tab.result.error = msg; - tab.result.formatError = true; // a format error, not a run result (so success can clear just this) - app.state.resultView.value = 'table'; - renderResults(app); // explicit: the format-error tab.result is an in-place write, and resultView may already be 'table' (no effect) - const pos = parseErrorPos(msg); - if (pos != null) app.dom.editorRevealCaret(pos); + if (stmts.length > 1) { + // Multi-statement: format each (best-effort — keep the original text for any + // statement that won't format, like insertCreate), then reassemble with a + // `;` and a blank line between statements. + const formatted = await Promise.all(stmts.map((s) => formatOne(s).catch(() => s))); + replaceEditor(app, withStatementBreak(formatted.map((q, i) => q || stmts[i]).join(';\n\n'))); + clearFormatError(); + return; + } + // Single statement: send the raw (untrimmed) SQL so a syntax error's reported + // position maps 1:1 onto the editor text; show it persistently + jump the caret. + try { + const q = await formatOne(raw); + // Terminate so the caret lands past the last token — otherwise the input + // event from the replace re-opens autocomplete on the trailing word. + if (q) replaceEditor(app, withStatementBreak(q)); + clearFormatError(); + } catch (e) { + const msg = String((e && e.message) || e); + tab.result = newResult('Table'); + tab.result.error = msg; + tab.result.formatError = true; // a format error, not a run result (so success can clear just this) + app.state.resultView.value = 'table'; + renderResults(app); // explicit: the format-error tab.result is an in-place write, and resultView may already be 'table' (no effect) + const pos = parseErrorPos(msg); + if (pos != null) app.dom.editorRevealCaret(pos); + } + } finally { + setFmtBtn(false); } } @@ -748,11 +785,19 @@ export function createApp(env = {}) { openDetailPane(app, node, detail, targetDoc); } + // EXPLAIN wraps the whole editor as a single statement, so it can't run against a + // `;`-separated script (ClickHouse would reject `EXPLAIN a; b; …` with a confusing + // parse error). Say so with our own message instead. + function explainMultiBlocked() { + if (splitStatements(app.activeTab().sql).length <= 1) return false; + flashToast('Explain isn’t available for a multi-statement script — run one statement at a time.', { document: doc }); + return true; + } // Explain the current query without editing it: run it through the EXPLAIN // views (the editor SQL is left untouched; run() wraps it as needed). - function explainQuery() { return run({ explain: true }); } + function explainQuery() { return explainMultiBlocked() ? undefined : run({ explain: true }); } // Switch the active EXPLAIN view (re-runs the derived query, keeps the mode). - function setExplainView(id) { return run({ explainView: id }); } + function setExplainView(id) { return explainMultiBlocked() ? undefined : run({ explainView: id }); } // Fetch the DDL for `target` (e.g. 'db.table' or 'DATABASE db') with // SHOW CREATE, pretty-print it through formatQuery(), and drop it into the diff --git a/src/ui/saved-history.js b/src/ui/saved-history.js index eb3512a..2e70817 100644 --- a/src/ui/saved-history.js +++ b/src/ui/saved-history.js @@ -10,6 +10,7 @@ import { SUBQUERY_MIME } from './editor.js'; import { sortedSaved, filterSaved, filterHistory, renameSaved, toggleFavorite, deleteSaved, deleteHistory, } from '../state.js'; +import { isAutoRunnable } from '../core/sql-split.js'; // Make a Library/History row draggable; dropping it on the editor inserts the // query wrapped as a `( … )` subquery (see the editor's drop handler). @@ -109,7 +110,7 @@ function renderSaved(app, list) { onclick: (e) => { e.stopPropagation(); toggleFavorite(state, q.id, app.saveJSON); renderSavedHistory(app); }, }, Icon.star(q.favorite)); - const row = h('div', { class: 'saved-row', ...dragProps(q.sql), onclick: () => { app.actions.loadIntoNewTab(q.name, q.sql, q.id, q.chart); app.actions.run({ view: q.view }); } }, + const row = h('div', { class: 'saved-row', ...dragProps(q.sql), onclick: () => { app.actions.loadIntoNewTab(q.name, q.sql, q.id, q.chart); if (isAutoRunnable(q.sql)) app.actions.run({ view: q.view }); } }, h('div', { class: 'top' }, star, h('span', { class: 'name' }, q.name), @@ -181,7 +182,7 @@ function renderHistory(app, list) { return; } for (const ent of items) { - list.appendChild(h('div', { class: 'history-row', ...dragProps(ent.sql), onclick: () => { app.actions.loadIntoNewTab('From history', ent.sql); app.actions.run(); } }, + list.appendChild(h('div', { class: 'history-row', ...dragProps(ent.sql), onclick: () => { app.actions.loadIntoNewTab('From history', ent.sql); if (isAutoRunnable(ent.sql)) app.actions.run(); } }, h('button', { class: 'sv-act del', title: 'Delete', onclick: (e) => { e.stopPropagation(); deleteHistory(state, ent.id, app.saveJSON); renderSavedHistory(app); }, diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index f4f6b63..821e91c 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -457,6 +457,21 @@ describe('query run', () => { expect(app.activeTab().result.explainView).toBe('explain'); expect(sentExplains(e)).toContain('EXPLAIN SELECT 1'); }); + it('Explain on a multi-statement script shows a message and sends no EXPLAIN', async () => { + const { app, e } = appForRun([[(u, sql) => /EXPLAIN/.test(sql), resp({ text: 'plan' })]]); + app.activeTab().sql = 'SELECT 1; SELECT 2'; + await app.actions.explainQuery(); + expect(document.querySelector('.share-toast').textContent).toMatch(/multi-statement/); + expect(sentExplains(e)).toHaveLength(0); // nothing sent to ClickHouse + expect(app.activeTab().result).toBeNull(); + }); + it('setExplainView on a multi-statement script is also blocked', async () => { + const { app, e } = appForRun([[(u, sql) => /EXPLAIN/.test(sql), resp({ text: 'plan' })]]); + app.activeTab().sql = 'SELECT 1; SELECT 2'; + await app.actions.setExplainView('pipeline'); + expect(document.querySelector('.share-toast').textContent).toMatch(/multi-statement/); + expect(sentExplains(e)).toHaveLength(0); + }); it('runs ESTIMATE as a structured table (streaming), not raw', async () => { const { app } = appForRun([ [(u, sql) => /ESTIMATE/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"rows","type":"UInt64"}]}\n', '{"row":{"rows":"42"}}\n']) })], @@ -685,6 +700,48 @@ describe('formatQuery', () => { expect(app.root.querySelector('.results-error')).toBeNull(); // error cleared expect(app.activeTab().result).toBeNull(); }); + it('formats a multi-statement script one statement at a time, joined by ;', async () => { + const { app } = appFor([ + [(u, sql) => /create table/.test(sql), resp({ json: { data: [{ q: 'CREATE TABLE t\n(\n a Int8\n)' }] } })], + [(u, sql) => /count/.test(sql), resp({ json: { data: [{ q: 'SELECT count()\nFROM t' }] } })], + ]); + app.activeTab().sql = 'create table t (a Int8); select count() from t'; + await app.actions.formatQuery(); + expect(app.dom.editorTextarea.value).toBe('CREATE TABLE t\n(\n a Int8\n);\n\nSELECT count()\nFROM t\n'); + }); + it('multi-statement format is best-effort: an unformattable statement keeps its original text', async () => { + const { app } = appFor([ + [(u, sql) => /create table/.test(sql), resp({ json: { data: [{ q: 'CREATE TABLE t (a Int8)' }] } })], + [(u, sql) => /bad syntax/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"Syntax error"}' })], + ]); + app.activeTab().sql = 'create table t (a Int8); bad syntax here'; + await app.actions.formatQuery(); + expect(app.dom.editorTextarea.value).toContain('bad syntax here'); // original kept + expect(app.root.querySelector('.results-error')).toBeNull(); // no scary error for the script + }); + it('a multi-statement format clears a prior single-statement format error', async () => { + const { app } = appFor([ + [(u, sql) => /BEWEEN/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"Syntax error: failed at position 8 (BEWEEN): x"}' })], + [(u, sql) => /formatQuery/.test(sql), resp({ json: { data: [{ q: 'SELECT 1' }] } })], + ]); + app.activeTab().sql = 'select x BEWEEN 2'; + await app.actions.formatQuery(); + expect(app.root.querySelector('.results-error')).not.toBeNull(); + app.activeTab().sql = 'select 1; select 2'; // now a script + await app.actions.formatQuery(); + expect(app.root.querySelector('.results-error')).toBeNull(); + }); + it('setFmtBtn toggles a busy/spinner state and no-ops without the button', () => { + const { app } = appFor([]); + app.setFmtBtn(true); + expect(app.dom.fmtBtn.disabled).toBe(true); + expect(app.dom.fmtBtn.textContent).toContain('Formatting…'); + app.setFmtBtn(false); + expect(app.dom.fmtBtn.disabled).toBe(false); + expect(app.dom.fmtBtn.textContent).toBe('Format'); + const noRender = createApp(env()); // no renderApp → no fmtBtn + expect(() => noRender.setFmtBtn(true)).not.toThrow(); + }); }); describe('insertCreate', () => { diff --git a/tests/unit/saved-history.test.js b/tests/unit/saved-history.test.js index e289ad6..629dce7 100644 --- a/tests/unit/saved-history.test.js +++ b/tests/unit/saved-history.test.js @@ -44,6 +44,16 @@ describe('renderSavedHistory', () => { expect(app.updateSaveBtn).toHaveBeenCalled(); }); + it('saved: an effectful query loads into the editor but does NOT auto-run', () => { + const app = makeApp(); + app.state.sidePanel.value = 'saved'; + app.state.savedQueries = [{ id: 's1', name: 'Setup', sql: 'CREATE TABLE t (a Int8)', favorite: false }]; + renderSavedHistory(app); + click(app.dom.savedList.querySelector('.saved-row')); + expect(app.actions.loadIntoNewTab).toHaveBeenCalledWith('Setup', 'CREATE TABLE t (a Int8)', 's1', undefined); + expect(app.actions.run).not.toHaveBeenCalled(); + }); + it('saved: live count + star toggles favorite and re-sorts favorites first', () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; @@ -178,6 +188,16 @@ describe('renderSavedHistory', () => { expect(app.actions.run).toHaveBeenCalled(); // re-runs on restore }); + it('history: an effectful entry loads into the editor but does NOT auto-run', () => { + const app = makeApp(); + app.state.sidePanel.value = 'history'; + app.state.history = [{ id: 'h1', sql: 'DROP TABLE t', ts: Date.now(), rows: null, ms: 1 }]; + renderSavedHistory(app); + click(app.dom.savedList.querySelector('.history-row')); + expect(app.actions.loadIntoNewTab).toHaveBeenCalledWith('From history', 'DROP TABLE t'); + expect(app.actions.run).not.toHaveBeenCalled(); + }); + it('history: per-row delete removes just that entry without loading it', () => { const app = makeApp(); app.state.sidePanel.value = 'history'; diff --git a/tests/unit/sql-split.test.js b/tests/unit/sql-split.test.js index 73816a1..5995bf3 100644 --- a/tests/unit/sql-split.test.js +++ b/tests/unit/sql-split.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { splitStatements, leadingKeyword, isRowReturning } from '../../src/core/sql-split.js'; +import { splitStatements, leadingKeyword, isRowReturning, isAutoRunnable } from '../../src/core/sql-split.js'; describe('splitStatements', () => { it('returns [] for empty / nullish input', () => { @@ -113,3 +113,19 @@ describe('isRowReturning', () => { } }); }); + +describe('isAutoRunnable', () => { + it('is true when every statement is row-returning (one or many)', () => { + expect(isAutoRunnable('SELECT 1')).toBe(true); + expect(isAutoRunnable('SELECT 1; SHOW TABLES; WITH x AS (SELECT 1) SELECT * FROM x')).toBe(true); + }); + it('is false when any statement is effectful', () => { + expect(isAutoRunnable('CREATE TABLE t (a Int)')).toBe(false); + expect(isAutoRunnable('DROP TABLE t')).toBe(false); + expect(isAutoRunnable('SELECT 1; INSERT INTO t VALUES (1)')).toBe(false); + }); + it('is false for empty / comment-only input', () => { + expect(isAutoRunnable('')).toBe(false); + expect(isAutoRunnable(' -- just a note ')).toBe(false); + }); +}); From ed968ecafacbcf376c6d0780cf81f20dd2d38ee9 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 17:03:36 +0000 Subject: [PATCH 07/12] feat(multiquery): per-statement time column + resizable script-grid columns (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The script summary grid gains a 3rd column showing each statement's own execution time (right-aligned). The toolbar still shows the script total. - Grid columns are drag-resizable like the data table — generalized the existing resize helpers (applyFixedWidths/startColumnResize) with a keyOf mapper so the data grid keeps its row-number ('idx') column while the script grid keys by plain column index; no duplicated drag logic. - Initial column proportions 25% / 65% / 10% (Statement / Result / Time) via CSS; a drag then pins px widths, same as the data grid. Per-file coverage gate green (1097 tests); build OK; e2e green (39). --- CHANGELOG.md | 4 ++- src/styles.css | 7 ++++- src/ui/app.js | 8 +++--- src/ui/results.js | 52 ++++++++++++++++++++++++++------------ tests/unit/app.test.js | 1 + tests/unit/results.test.js | 49 +++++++++++++++++++++++++++++++---- 6 files changed, 95 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a98bf0..8a5178b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ auto-generated per-PR notes; this file is the curated, human-readable history. text (the Run button flips to **Run selection**); a single selected statement still gets the full Table/Chart/EXPLAIN view. Row-returning statements show the first row inline (comma-separated) — click to open all rows (capped at 100) in a - side pane; effectful statements show **OK**. Cancel aborts mid-script. Splitting + side pane; effectful statements show **OK**. Each grid row also shows that + statement's own execution time (the toolbar still shows the script total), and + the grid's columns are drag-resizable like the data table. Cancel aborts mid-script. Splitting is purely lexical (`src/core/sql-split.js`), skipping `;` inside string/identifier literals and `--` / `#` / `/* */` comments. Known limitation: an `INSERT … FORMAT …` with inline data containing `;` mis-splits — run those on their own. diff --git a/src/styles.css b/src/styles.css index d3d45be..306b79b 100644 --- a/src/styles.css +++ b/src/styles.css @@ -1473,8 +1473,13 @@ table.res-table tbody tr:hover td.idx { background: var(--bg-hover); } /* multiquery script summary grid */ .script-grid table.res-table { table-layout: fixed; width: 100%; } -.script-grid td.script-sql { width: 55%; } +/* initial proportions (drag-resize then pins px widths, like the data grid) */ +.script-grid th.script-sql, .script-grid td.script-sql { width: 25%; } +.script-grid th.script-res { width: 65%; } +.script-grid th.script-time, .script-grid td.script-time { width: 10%; text-align: right; } +.script-grid th { position: relative; } /* anchor the col-resize-h handle */ .script-grid td.script-sql .cell-val { font-family: var(--mono); color: var(--fg-mute); } +.script-grid td.script-time { font-family: var(--mono); font-size: 11px; color: var(--fg-faint); white-space: nowrap; } .script-cell { font-family: var(--mono); font-size: 12px; } .script-cell.ok { color: #10b981; font-weight: 600; } .script-cell.err { color: #ef4444; white-space: normal; word-break: break-word; } diff --git a/src/ui/app.js b/src/ui/app.js index dafbe26..77e66c9 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -560,6 +560,7 @@ export function createApp(env = {}) { // issues KILL QUERY against the statement that's actually running. app.state.runQueryId = cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'q' + t0 + '_' + i; let out; + const s0 = now(); // this statement's own wall-clock (grid Time column) try { out = await ch.runQuery(chCtx, stmt, { format: rowReturning ? 'JSONCompact' : 'TSV', @@ -573,16 +574,17 @@ export function createApp(env = {}) { if (e.name === 'AbortError') { aborted = true; break; } out = { error: e instanceof TypeError ? 'Network error' : String((e && e.message) || e) }; } + const ms = now() - s0; if (out.error != null) { - entries.push({ sql: stmt, status: 'error', error: out.error }); + entries.push({ sql: stmt, status: 'error', error: out.error, ms }); renderResults(app); break; // stop-on-first-failure: skip the remaining statements } if (rowReturning) { const sel = parseSelectResult(out.raw, SELECT_ROW_CAP); - entries.push({ sql: stmt, status: 'rows', columns: sel.columns, rows: sel.rows, truncated: sel.truncated, preview: firstRowPreview(sel.rows) }); + entries.push({ sql: stmt, status: 'rows', columns: sel.columns, rows: sel.rows, truncated: sel.truncated, preview: firstRowPreview(sel.rows), ms }); } else { - entries.push({ sql: stmt, status: 'ok' }); + entries.push({ sql: stmt, status: 'ok', ms }); } renderResults(app); } diff --git a/src/ui/results.js b/src/ui/results.js index 0300b9f..164dc29 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -31,17 +31,23 @@ export function colResizeWidth(startW, dx, scale) { return Math.max(MIN_COL, Math.round(startW + dx / (scale || 1))); } +// Map a header cell index to its `r.colWidths` key. The data grid's first cell is +// the row-number column ('idx'); its data columns are then 0-based. The script +// grid has no row-number column, so every cell keys by its own index. +const IDX_KEY = (k) => (k === 0 ? 'idx' : k - 1); +const PLAIN_KEY = (k) => k; + /** - * Pin every column of `table` to the px widths in `r.colWidths` (key 'idx' for - * the row-number column, then 0-based data-column indices) and switch it to - * fixed layout so columns honor those widths exactly (and the wrap scrolls). + * Pin every column of `table` to the px widths in `r.colWidths` (keyed via + * `keyOf(cellIndex)`) and switch it to fixed layout so columns honor those widths + * exactly (and the wrap scrolls). Shared by the data grid and the script grid. */ -function applyFixedWidths(table, r) { +function applyFixedWidths(table, r, keyOf) { table.classList.add('fixed'); const cells = table.querySelectorAll('thead th'); let total = 0; for (let k = 0; k < cells.length; k++) { - const w = r.colWidths[k === 0 ? 'idx' : k - 1]; + const w = r.colWidths[keyOf(k)]; cells[k].style.width = w + 'px'; total += w; } @@ -49,27 +55,28 @@ function applyFixedWidths(table, r) { table.style.minWidth = '0'; } -/** Begin dragging the right edge of header `th` (a data column) to resize it. */ -function startColumnResize(r, th, ev) { +/** Begin dragging the right edge of header `th` to resize its column. `keyOf` + * maps a cell index to its `r.colWidths` key (see IDX_KEY / PLAIN_KEY). */ +function startColumnResize(r, th, ev, keyOf) { ev.preventDefault(); ev.stopPropagation(); // don't let the handle's mousedown reach the sort header const table = th.closest('table'); const cells = table.querySelectorAll('thead th'); - const colIndex = [].indexOf.call(cells, th) - 1; // 'idx' is cell 0 + const colIndex = keyOf([].indexOf.call(cells, th)); // First resize: freeze every column at its current rendered width, then fix. if (!Object.keys(r.colWidths).length) { for (let k = 0; k < cells.length; k++) { - r.colWidths[k === 0 ? 'idx' : k - 1] = cells[k].offsetWidth; + r.colWidths[keyOf(k)] = cells[k].offsetWidth; } } - applyFixedWidths(table, r); + applyFixedWidths(table, r, keyOf); const win = th.ownerDocument.defaultView; const scale = zoomScale(th); const startX = ev.clientX; const startW = r.colWidths[colIndex]; const onMove = (m) => { r.colWidths[colIndex] = colResizeWidth(startW, m.clientX - startX, scale); - applyFixedWidths(table, r); + applyFixedWidths(table, r, keyOf); }; const onUp = () => { win.removeEventListener('mousemove', onMove); @@ -162,23 +169,36 @@ function streamStrip(r) { // collapsed statement text (full text on hover); Col 2 is the outcome — OK for an // effectful statement (DDL/INSERT), the first-row preview for a SELECT (click to // open all rows in a side pane), or the error for the failing statement (the last -// row, since the run stops on first failure). +// row, since the run stops on first failure); Col 3 is that statement's own +// execution time (the toolbar still shows the script total). Columns are +// drag-resizable like the data grid (initial 25 / 65 / 10 from CSS). function renderScriptGrid(app, r) { + r.colWidths = r.colWidths || {}; // persists drag-resized widths across re-renders const wrap = h('div', { class: 'res-table-wrap script-grid' }); const table = document.createElement('table'); table.className = 'res-table'; const thead = document.createElement('thead'); const trh = document.createElement('tr'); - trh.appendChild(h('th', null, 'Statement')); - trh.appendChild(h('th', null, 'Result')); + for (const [cls, label] of [['script-sql', 'Statement'], ['script-res', 'Result'], ['script-time', 'Time']]) { + const th = h('th', { class: cls }, h('span', { class: 'h-name' }, label), + h('span', { + class: 'col-resize-h', + title: 'Drag to resize column', + onmousedown: (e) => startColumnResize(r, th, e, PLAIN_KEY), + onclick: (e) => e.stopPropagation(), + })); + trh.appendChild(th); + } thead.appendChild(trh); table.appendChild(thead); + if (Object.keys(r.colWidths).length) applyFixedWidths(table, r, PLAIN_KEY); const tbody = document.createElement('tbody'); r.script.forEach((e) => { const tr = document.createElement('tr'); tr.appendChild(h('td', { class: 'script-sql', title: e.sql || '' }, h('div', { class: 'cell-val' }, (e.sql || '').replace(/\s+/g, ' ').trim()))); tr.appendChild(scriptOutcomeCell(app, e)); + tr.appendChild(h('td', { class: 'script-time' }, e.ms != null ? e.ms.toFixed(0) + ' ms' : '')); tbody.appendChild(tr); }); table.appendChild(tbody); @@ -408,7 +428,7 @@ export function renderTable(app, r) { h('span', { class: 'col-resize-h', title: 'Drag to resize column', - onmousedown: (e) => startColumnResize(r, th, e), + onmousedown: (e) => startColumnResize(r, th, e, IDX_KEY), onclick: (e) => e.stopPropagation(), })); trh.appendChild(th); @@ -416,7 +436,7 @@ export function renderTable(app, r) { const thead = document.createElement('thead'); thead.appendChild(trh); table.appendChild(thead); - if (Object.keys(r.colWidths).length) applyFixedWidths(table, r); + if (Object.keys(r.colWidths).length) applyFixedWidths(table, r, IDX_KEY); const tbody = document.createElement('tbody'); rows.slice(0, VIS_CAP).forEach((row, ri) => { diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index 821e91c..dbff902 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -507,6 +507,7 @@ describe('query run', () => { const script = app.activeTab().result.script; expect(script.map((e) => e.status)).toEqual(['ok', 'ok', 'rows']); expect(script[2]).toMatchObject({ preview: '1', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1']] }); + expect(script.every((e) => typeof e.ms === 'number')).toBe(true); // per-statement time recorded expect(app.state.history).toHaveLength(1); expect(app.state.history[0].sql).toBe(SCRIPT); // SELECT statements are sent with the JSONCompact + row-cap params diff --git a/tests/unit/results.test.js b/tests/unit/results.test.js index fe0b747..3515671 100644 --- a/tests/unit/results.test.js +++ b/tests/unit/results.test.js @@ -628,10 +628,10 @@ describe('multiquery script grid (#83)', () => { const scriptResult = (over = {}) => ({ elapsedMs: 12, script: [ - { sql: 'CREATE TABLE t (a Int8)', status: 'ok' }, - { sql: 'SELECT count() AS c\nFROM t', status: 'rows', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1'], ['2']], truncated: false, preview: '1' }, - { sql: 'SELECT * FROM nope', status: 'rows', columns: [], rows: [] }, - { sql: 'BAD SQL', status: 'error', error: 'DB::Exception: boom' }, + { sql: 'CREATE TABLE t (a Int8)', status: 'ok', ms: 3 }, + { sql: 'SELECT count() AS c\nFROM t', status: 'rows', columns: [{ name: 'c', type: 'UInt64' }], rows: [['1'], ['2']], truncated: false, preview: '1', ms: 7 }, + { sql: 'SELECT * FROM nope', status: 'rows', columns: [], rows: [], ms: 1 }, + { sql: 'BAD SQL', status: 'error', error: 'DB::Exception: boom', ms: 2 }, ], ...over, }); @@ -649,7 +649,7 @@ describe('multiquery script grid (#83)', () => { expect(cells[2].textContent).toBe('(0 rows)'); expect(cells[3].textContent).toContain('boom'); // SQL is collapsed to one line, full text on the title attribute - const sqlCell = region.querySelector('.script-sql'); + const sqlCell = region.querySelector('tbody td.script-sql'); expect(sqlCell.querySelector('.cell-val').textContent).toBe('CREATE TABLE t (a Int8)'); }); @@ -713,4 +713,43 @@ describe('multiquery script grid (#83)', () => { renderResults(app); expect(app.dom.resultsRegion.querySelector('.res-graph-title').textContent).toContain('1 statement'); }); + + it('shows each statement’s own execution time in a third column', () => { + const app = appWithResult(scriptResult()); + renderResults(app); + const region = app.dom.resultsRegion; + expect([...region.querySelectorAll('thead th')].map((th) => th.textContent.trim())).toEqual(['Statement', 'Result', 'Time']); + expect([...region.querySelectorAll('tbody td.script-time')].map((td) => td.textContent)).toEqual(['3 ms', '7 ms', '1 ms', '2 ms']); + }); + + it('leaves the Time cell blank when a statement has no recorded ms', () => { + const app = appWithResult(scriptResult({ script: [{ sql: 'SELECT 1', status: 'ok' }] })); + renderResults(app); + expect(app.dom.resultsRegion.querySelector('tbody td.script-time').textContent).toBe(''); + }); + + it('columns are drag-resizable (3 handles; keyed by plain column index)', () => { + const app = appWithResult(scriptResult()); + renderResults(app); + const region = app.dom.resultsRegion; + const r = app.activeTab().result; + const handles = region.querySelectorAll('.script-grid th .col-resize-h'); + expect(handles).toHaveLength(3); // Statement, Result, Time + const win = handles[0].ownerDocument.defaultView; + handles[1].dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); + const table = region.querySelector('.res-table'); + expect(table.classList.contains('fixed')).toBe(true); + expect(Object.keys(r.colWidths).sort()).toEqual(['0', '1', '2']); // no 'idx' column + win.dispatchEvent(new MouseEvent('mousemove', { clientX: 180 })); + expect(r.colWidths[1]).toBe(80); // startW 0 + 80/1 + win.dispatchEvent(new MouseEvent('mouseup', {})); + }); + + it('reapplies stored script-grid widths on re-render', () => { + const app = appWithResult(scriptResult({ colWidths: { 0: 120, 1: 300, 2: 60 } })); + renderResults(app); + const cells = app.dom.resultsRegion.querySelectorAll('.script-grid thead th'); + expect(cells[0].style.width).toBe('120px'); + expect(cells[2].style.width).toBe('60px'); + }); }); From b2b2de4e5adaa64c1263d2e6d0a1dd65788dd87b Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 17:29:04 +0000 Subject: [PATCH 08/12] feat(multiquery): run statements inside a per-tab ClickHouse HTTP session (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ClickHouse's HTTP interface is stateless per request, so a multiquery script like CREATE TEMPORARY TABLE …; INSERT …; SELECT … couldn't see its own temp table across the separate requests. Each tab now gets a session_id (lazily generated) passed on its run + script requests with session_timeout=600, so session state — temporary tables, SET settings — survives across a script's statements and across successive runs in the tab. Schema/reference loads stay session-less (they fan out in parallel and would deadlock on the session lock); only one user query runs at a time, so the session is never contended. --- src/ui/app.js | 26 +++++++++++++++++++++++--- tests/unit/app.test.js | 11 ++++++++++- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/ui/app.js b/src/ui/app.js index 77e66c9..99cdf55 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -433,6 +433,21 @@ export function createApp(env = {}) { } app.tickElapsed = tickElapsed; + // A ClickHouse HTTP session ties a tab's requests together so session state — + // temporary tables, SET settings — survives across the separate HTTP requests + // of a multiquery script (and across successive runs in the tab). ClickHouse's + // HTTP interface runs one statement per request and is otherwise stateless, so + // without this a `CREATE TEMPORARY TABLE …; INSERT …; SELECT …` script can't + // see its own temp table. Each tab gets its own id (lazily) so tabs don't share + // state and never collide on the per-session lock (only one query runs at a + // time, guarded by `running`). 600s idle timeout (default is 60s). Schema / + // reference loads deliberately run session-less — they fire in parallel and + // would otherwise deadlock on the lock. + function sessionParams(tab) { + tab.chSession = tab.chSession || (cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'sess-' + now()); + return { session_id: tab.chSession, session_timeout: 600 }; + } + async function run(opts) { if (app.state.running.value) return; // already running — cancel via cancel()/Esc const tab = app.activeTab(); @@ -502,6 +517,7 @@ export function createApp(env = {}) { format: fmt, queryId: app.state.runQueryId, signal: app.state.abortController.signal, + params: sessionParams(tab), onLine: (json) => applyStreamLine(json, tab.result), onChunk: () => renderResults(app), }); @@ -566,9 +582,13 @@ export function createApp(env = {}) { format: rowReturning ? 'JSONCompact' : 'TSV', queryId: app.state.runQueryId, signal: app.state.abortController.signal, - // Over-fetch by one past the display cap so a truncated result is - // detectable (at exactly the cap the client can't tell it was cut). - params: rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : undefined, + // Shared session across the script's statements (temp tables / SET + // persist); over-fetch SELECTs by one past the display cap so a + // truncated result is detectable (at exactly the cap it isn't). + params: { + ...sessionParams(tab), + ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}), + }, }); } catch (e) { if (e.name === 'AbortError') { aborted = true; break; } diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index dbff902..552a0d4 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -311,6 +311,9 @@ describe('query run', () => { await app.actions.run(); expect(app.activeTab().result.rows).toEqual([['1']]); expect(app.state.history.length).toBe(1); + // the query runs inside a per-tab ClickHouse session (600s timeout) + const runUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); + expect(runUrl).toMatch(/session_timeout=600/); }); it('keeps the current result view on a plain re-run, and restores a remembered view when opened (#34)', async () => { const routes = [[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })]]; @@ -512,8 +515,14 @@ describe('query run', () => { expect(app.state.history[0].sql).toBe(SCRIPT); // SELECT statements are sent with the JSONCompact + row-cap params // (over-fetched by one past the display cap to detect truncation). - const selUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /max_result_rows=101/.test(u)); + const urls = app.chCtx.fetch.mock.calls.map((c) => c[0]); + const selUrl = urls.find((u) => /max_result_rows=101/.test(u)); expect(selUrl).toMatch(/result_overflow_mode=break/); + // every statement shares one ClickHouse session (so temp tables / SET persist) + const sids = urls.filter((u) => /session_id=/.test(u)).map((u) => /session_id=([^&]+)/.exec(u)[1]); + expect(sids).toHaveLength(3); + expect(new Set(sids).size).toBe(1); + expect(urls.some((u) => /session_timeout=600/.test(u))).toBe(true); }); it('flags a SELECT as truncated when more than the cap rows come back', async () => { From 11002acdb5f7cdc0308f3cd57b3f280efff61826 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 17:49:17 +0000 Subject: [PATCH 09/12] feat(results): splitter column-resize + one shared grid for the rows pane (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Column resize now uses a splitter model: dragging a border trades width between the column and its right neighbor (total width and other columns stay put), fixing the shift/scroll artifact. The last column (no neighbor) still grows the table. Applies to the data grid, the script grid, and the rows pane. - Extracted renderGrid({columns,rows,sort,onSort,widths,onCell}) — one sortable + resizable table component. renderTable wires it to the global result/sort; the script-row side pane (openRowsViewer) now reuses it with local sort/width state, so the rows pane is sortable + resizable like the main grid (was a static table). - Resize helpers (applyFixedWidths/startColumnResize) now take a plain widths object, decoupled from the result. - Stacked drawers: only the topmost responds to Escape, so dismissing a cell drawer opened from the rows pane returns to the pane instead of closing both. - Confirmed -- / # / /* */ comment support end-to-end (added a script test). Per-file coverage gate green (1103 tests); build OK; e2e green (39). --- CHANGELOG.md | 13 +++- src/ui/results.js | 146 +++++++++++++++++++++-------------- tests/unit/results.test.js | 110 ++++++++++++++++++++------ tests/unit/sql-split.test.js | 10 +++ 4 files changed, 196 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a5178b..632bb00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,12 @@ auto-generated per-PR notes; this file is the curated, human-readable history. still gets the full Table/Chart/EXPLAIN view. Row-returning statements show the first row inline (comma-separated) — click to open all rows (capped at 100) in a side pane; effectful statements show **OK**. Each grid row also shows that - statement's own execution time (the toolbar still shows the script total), and - the grid's columns are drag-resizable like the data table. Cancel aborts mid-script. Splitting + statement's own execution time (the toolbar still shows the script total). The + click-to-open row pane is the **same sortable + resizable grid** as the main + results table (one shared component). Statements run inside a **per-tab + ClickHouse HTTP session** (`session_id` + `session_timeout=600`), so a script's + temporary tables / `SET`s persist across its separate per-statement requests. + Cancel aborts mid-script. Splitting is purely lexical (`src/core/sql-split.js`), skipping `;` inside string/identifier literals and `--` / `#` / `/* */` comments. Known limitation: an `INSERT … FORMAT …` with inline data containing `;` mis-splits — run those on their own. @@ -53,6 +57,11 @@ auto-generated per-PR notes; this file is the curated, human-readable history. loads rather than in-place mutation. This **completes the migration**. (#88, #91) ### Fixed +- Result-table **column resize** now uses a splitter model: dragging a column's + right edge trades width with its right neighbor (the table's total width and the + other columns stay put), instead of growing the whole table and shifting later + columns sideways. Dragging the last column still widens the table. Applies to the + data grid, the multiquery script grid, and the script-row pane (one shared grid). - The fullscreen schema / EXPLAIN graph panels were mis-sized on **Safari** (#70). They size off viewport units, and engines disagree on how `vw`/`vh` interact with `html{zoom}`: Chromium's ignore `zoom` (so `100vh` overshoots one screen by diff --git a/src/ui/results.js b/src/ui/results.js index 164dc29..4ceb832 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -38,16 +38,16 @@ const IDX_KEY = (k) => (k === 0 ? 'idx' : k - 1); const PLAIN_KEY = (k) => k; /** - * Pin every column of `table` to the px widths in `r.colWidths` (keyed via + * Pin every column of `table` to the px widths in `widths` (keyed via * `keyOf(cellIndex)`) and switch it to fixed layout so columns honor those widths * exactly (and the wrap scrolls). Shared by the data grid and the script grid. */ -function applyFixedWidths(table, r, keyOf) { +function applyFixedWidths(table, widths, keyOf) { table.classList.add('fixed'); const cells = table.querySelectorAll('thead th'); let total = 0; for (let k = 0; k < cells.length; k++) { - const w = r.colWidths[keyOf(k)]; + const w = widths[keyOf(k)]; cells[k].style.width = w + 'px'; total += w; } @@ -55,28 +55,45 @@ function applyFixedWidths(table, r, keyOf) { table.style.minWidth = '0'; } -/** Begin dragging the right edge of header `th` to resize its column. `keyOf` - * maps a cell index to its `r.colWidths` key (see IDX_KEY / PLAIN_KEY). */ -function startColumnResize(r, th, ev, keyOf) { +/** + * Begin dragging the right edge of header `th` to resize its column. `keyOf` maps + * a cell index to its `r.colWidths` key (see IDX_KEY / PLAIN_KEY). + * + * Splitter model: the drag moves the *border* between this column and its right + * neighbor — the column grows and the neighbor shrinks by the same amount, so the + * table's total width (and every other column) stays put. Dragging the last + * column's edge has no neighbor to take from, so it grows the table (scroll). + */ +function startColumnResize(widths, th, ev, keyOf) { ev.preventDefault(); ev.stopPropagation(); // don't let the handle's mousedown reach the sort header const table = th.closest('table'); const cells = table.querySelectorAll('thead th'); - const colIndex = keyOf([].indexOf.call(cells, th)); + const cellIdx = [].indexOf.call(cells, th); + const colIndex = keyOf(cellIdx); + const nextKey = cellIdx + 1 < cells.length ? keyOf(cellIdx + 1) : null; // First resize: freeze every column at its current rendered width, then fix. - if (!Object.keys(r.colWidths).length) { + if (!Object.keys(widths).length) { for (let k = 0; k < cells.length; k++) { - r.colWidths[keyOf(k)] = cells[k].offsetWidth; + widths[keyOf(k)] = cells[k].offsetWidth; } } - applyFixedWidths(table, r, keyOf); + applyFixedWidths(table, widths, keyOf); const win = th.ownerDocument.defaultView; const scale = zoomScale(th); const startX = ev.clientX; - const startW = r.colWidths[colIndex]; + const startW = widths[colIndex]; + const pairW = nextKey != null ? startW + widths[nextKey] : 0; // combined width of the pair const onMove = (m) => { - r.colWidths[colIndex] = colResizeWidth(startW, m.clientX - startX, scale); - applyFixedWidths(table, r, keyOf); + let w = colResizeWidth(startW, m.clientX - startX, scale); + if (nextKey != null) { + // Keep the pair's combined width constant; both stay ≥ MIN_COL (a pair + // narrower than 2·MIN_COL can't satisfy both — the floor wins over total). + w = Math.max(MIN_COL, Math.min(w, pairW - MIN_COL)); + widths[nextKey] = Math.max(MIN_COL, pairW - w); + } + widths[colIndex] = w; + applyFixedWidths(table, widths, keyOf); }; const onUp = () => { win.removeEventListener('mousemove', onMove); @@ -184,14 +201,14 @@ function renderScriptGrid(app, r) { h('span', { class: 'col-resize-h', title: 'Drag to resize column', - onmousedown: (e) => startColumnResize(r, th, e, PLAIN_KEY), + onmousedown: (e) => startColumnResize(r.colWidths, th, e, PLAIN_KEY), onclick: (e) => e.stopPropagation(), })); trh.appendChild(th); } thead.appendChild(trh); table.appendChild(thead); - if (Object.keys(r.colWidths).length) applyFixedWidths(table, r, PLAIN_KEY); + if (Object.keys(r.colWidths).length) applyFixedWidths(table, r.colWidths, PLAIN_KEY); const tbody = document.createElement('tbody'); r.script.forEach((e) => { const tr = document.createElement('tr'); @@ -225,14 +242,16 @@ function scriptOutcomeCell(app, e) { } /** - * Open a right-side pane with the full rows of one script SELECT, as a static - * table. Reuses the cell-detail drawer scaffold (.cd-*); a shared Drawer - * primitive is deferred to #60. Escape / backdrop / ✕ closes. Exported for tests. + * Open a right-side pane with the full rows of one script SELECT, using the same + * sortable + resizable grid as the main results table (renderGrid). Sort state and + * column widths are local to this pane; clicking a cell opens its value (the same + * cell-detail drawer, stacked). Reuses the .cd-* drawer scaffold (a shared Drawer + * primitive is deferred to #60). Escape / backdrop / ✕ closes. Exported for tests. */ export function openRowsViewer(app, entry) { const doc = app.document || document; let backdrop; - const onKey = (ev) => { if (ev.key === 'Escape') close(); }; + const onKey = (ev) => { if (ev.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; function close() { if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); @@ -243,7 +262,19 @@ export function openRowsViewer(app, entry) { h('span', { class: 'cd-name' }, 'Result rows'), h('span', { class: 'cd-type' }, n + (entry.truncated ? '+' : '') + ' row' + (n === 1 ? '' : 's'))), h('button', { class: 'cd-close', title: 'Close (Esc)', onclick: close }, Icon.close())); - const body = h('div', { class: 'cd-body' }, scriptRowsTable(entry.columns || [], entry.rows)); + // Local sort + width state (persist for the lifetime of this open via the entry). + const sort = entry.viewerSort || (entry.viewerSort = { col: null, dir: 'asc' }); + const widths = entry.viewerWidths || (entry.viewerWidths = {}); + const body = h('div', { class: 'cd-body' }); + const paint = () => body.replaceChildren(renderGrid({ + columns: entry.columns || [], + rows: entry.rows, + sort, + onSort: (col, dir) => { sort.col = col; sort.dir = dir; paint(); }, + widths, + onCell: (name, type, value) => openCellDetail(app, name, type, value), + })); + paint(); const panel = h('div', { class: 'cd-panel', onclick: (ev) => ev.stopPropagation() }, head, body); backdrop = h('div', { class: 'cd-backdrop', onclick: close }, panel); doc.body.appendChild(backdrop); @@ -251,27 +282,6 @@ export function openRowsViewer(app, entry) { return backdrop; } -// A plain (no sort / resize) table of a script SELECT's rows for the side pane. -function scriptRowsTable(columns, rows) { - const table = document.createElement('table'); - table.className = 'res-table'; - const trh = document.createElement('tr'); - trh.appendChild(h('th', { class: 'idx' }, '#')); - columns.forEach((c) => trh.appendChild(h('th', { title: c.type || '' }, c.name))); - const thead = document.createElement('thead'); - thead.appendChild(trh); - table.appendChild(thead); - const tbody = document.createElement('tbody'); - rows.forEach((row, ri) => { - const tr = document.createElement('tr'); - tr.appendChild(h('td', { class: 'idx' }, String(ri + 1))); - row.forEach((v) => tr.appendChild(h('td', { class: 'cell' }, h('div', { class: 'cell-val' }, v == null ? '' : String(v))))); - tbody.appendChild(tr); - }); - table.appendChild(tbody); - return h('div', { class: 'res-table-wrap' }, table); -} - function buildToolbar(app, r) { const toolbar = h('div', { class: 'res-toolbar' }); if (r && r.script) { @@ -401,25 +411,28 @@ export function renderJson(r) { return h('div', { class: 'json-view', tabindex: '0' }, JSON.stringify(arr, null, 2)); } -export function renderTable(app, r) { - const { col, dir } = app.state.resultSort; - const rows = sortRows(r.rows, col, dir); - r.colWidths = r.colWidths || {}; // persists across re-renders (sort/streaming) +/** + * Shared sortable + resizable data grid (the one table view, reused by the main + * results table and the script-row side pane). Caller supplies the data plus the + * state seams so the same DOM/interaction code drives both: + * { columns, rows, sort:{col,dir}, onSort(col,dir), widths, onCell(name,type,value) } + * `widths` is a colWidths object mutated in place (drag-resize); `onSort` re-renders + * for the caller (global results effect, or the drawer's local repaint). + */ +export function renderGrid({ columns, rows: rawRows, sort, onSort, widths, onCell }) { + const { col, dir } = sort; + const rows = sortRows(rawRows, col, dir); const wrap = h('div', { class: 'res-table-wrap' }); const table = document.createElement('table'); table.className = 'res-table'; const trh = document.createElement('tr'); trh.appendChild(h('th', { style: { textAlign: 'center', color: 'var(--fg-faint)', minWidth: '36px' } }, '#')); - r.columns.forEach((c, i) => { + columns.forEach((c, i) => { const isSort = col === i; const th = h('th', { title: c.type || '', // type exposed on hover, not shown inline - onclick: () => { - if (isSort) app.state.resultSort.dir = dir === 'asc' ? 'desc' : 'asc'; - else { app.state.resultSort.col = i; app.state.resultSort.dir = 'asc'; } - renderResults(app); - }, + onclick: () => onSort(i, isSort && dir === 'asc' ? 'desc' : 'asc'), }, h('div', { class: 'h-inner' }, h('span', { class: 'h-name' }, c.name), h('span', { style: { flex: '1' } }), @@ -428,7 +441,7 @@ export function renderTable(app, r) { h('span', { class: 'col-resize-h', title: 'Drag to resize column', - onmousedown: (e) => startColumnResize(r, th, e, IDX_KEY), + onmousedown: (e) => startColumnResize(widths, th, e, IDX_KEY), onclick: (e) => e.stopPropagation(), })); trh.appendChild(th); @@ -436,21 +449,21 @@ export function renderTable(app, r) { const thead = document.createElement('thead'); thead.appendChild(trh); table.appendChild(thead); - if (Object.keys(r.colWidths).length) applyFixedWidths(table, r, IDX_KEY); + if (Object.keys(widths).length) applyFixedWidths(table, widths, IDX_KEY); const tbody = document.createElement('tbody'); rows.slice(0, VIS_CAP).forEach((row, ri) => { const tr = document.createElement('tr'); tr.appendChild(h('td', { class: 'idx' }, String(ri + 1))); row.forEach((v, ci) => { - const isNum = isNumericType(r.columns[ci].type); + const isNum = isNumericType(columns[ci].type); const text = v == null ? '' : String(v); // Truncate in-cell (CSS max-width + ellipsis); click opens the full value // in a side drawer so one fat column (e.g. HTML blobs) can't dominate. tr.appendChild(h('td', { class: 'cell' + (isNum ? ' num' : ''), title: text.length > 100 ? text.slice(0, 100) + '…' : text, - onclick: () => openCellDetail(app, r.columns[ci].name, r.columns[ci].type, v), + onclick: () => onCell(columns[ci].name, columns[ci].type, v), }, h('div', { class: 'cell-val' }, text))); }); tbody.appendChild(tr); @@ -465,16 +478,37 @@ export function renderTable(app, r) { return wrap; } +// The main results table: renderGrid wired to the global sort state + result. +export function renderTable(app, r) { + r.colWidths = r.colWidths || {}; // persists across re-renders (sort/streaming) + return renderGrid({ + columns: r.columns, + rows: r.rows, + sort: app.state.resultSort, + onSort: (col, dir) => { app.state.resultSort = { col, dir }; renderResults(app); }, + widths: r.colWidths, + onCell: (name, type, value) => openCellDetail(app, name, type, value), + }); +} + /** * Open a right-side drawer with one cell's full value: pretty-printed (JSON is * reindented), and for HTML a Rendered (sandboxed iframe) ↔ Source toggle. * Escape or a backdrop/✕ click closes it. Exported for tests. */ +// Only the topmost drawer responds to Escape, so dismissing a stacked cell drawer +// returns to the rows pane underneath instead of closing both at once. (The +// current backdrop is always in the DOM when its handler fires.) +function isTopDrawer(doc, el) { + const all = doc.querySelectorAll('.cd-backdrop'); + return all[all.length - 1] === el; +} + export function openCellDetail(app, name, type, value) { const doc = app.document || document; const text = value == null ? '' : String(value); let backdrop; - const onKey = (e) => { if (e.key === 'Escape') close(); }; + const onKey = (e) => { if (e.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; function close() { if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); diff --git a/tests/unit/results.test.js b/tests/unit/results.test.js index 3515671..412fcb5 100644 --- a/tests/unit/results.test.js +++ b/tests/unit/results.test.js @@ -209,29 +209,52 @@ describe('column resize', () => { expect(app.state.resultSort).toEqual(before); // stopPropagation → no sort }); - it('dragging a handle fixes the layout and updates widths; re-drag reuses them', () => { + it('first drag freezes the layout (measures every column) and switches to fixed', () => { const app = appWithResult(tableResult()); renderResults(app); - const r = app.activeTab().result; + const r = app.activeTab().result; // colWidths empty → freeze path const region = app.dom.resultsRegion; - let handle = region.querySelectorAll('.res-table th .col-resize-h')[0]; - const win = handle.ownerDocument.defaultView; - // first drag: colWidths empty → measure all columns, switch to fixed layout + const handle = region.querySelectorAll('.res-table th .col-resize-h')[0]; handle.dispatchEvent(new MouseEvent('mousedown', { clientX: 200, bubbles: true })); const table = region.querySelector('.res-table'); expect(table.classList.contains('fixed')).toBe(true); - expect(Object.keys(r.colWidths)).toContain('idx'); - win.dispatchEvent(new MouseEvent('mousemove', { clientX: 320 })); - expect(r.colWidths[0]).toBe(120); // startW 0 + 120/1 (scale NaN→1 in jsdom) - win.dispatchEvent(new MouseEvent('mouseup', {})); + expect(Object.keys(r.colWidths).sort()).toEqual(['0', '1', 'idx']); // every column measured + handle.ownerDocument.defaultView.dispatchEvent(new MouseEvent('mouseup', {})); + }); + + it('splitter model: dragging a border grows the column and shrinks its neighbor (total constant)', () => { + const r = tableResult(); + r.colWidths = { idx: 36, 0: 100, 1: 100 }; // pre-seeded so the pair math is meaningful + const app = appWithResult(r); + renderResults(app); + const region = app.dom.resultsRegion; + const win = region.ownerDocument.defaultView; + const handle = region.querySelectorAll('.res-table th .col-resize-h')[0]; // col 0, neighbor col 1 + handle.dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); + win.dispatchEvent(new MouseEvent('mousemove', { clientX: 130 })); // +30 + expect(r.colWidths[0]).toBe(130); + expect(r.colWidths[1]).toBe(70); // neighbor gave up 30 — pair sum stays 200 + // drag past the neighbor's floor: neighbor clamps at MIN_COL (48), column caps win.dispatchEvent(new MouseEvent('mousemove', { clientX: 999 })); - expect(r.colWidths[0]).toBe(120); // listeners removed on mouseup + expect(r.colWidths[1]).toBe(48); + expect(r.colWidths[0]).toBe(152); // 200 - 48 + win.dispatchEvent(new MouseEvent('mouseup', {})); + win.dispatchEvent(new MouseEvent('mousemove', { clientX: 0 })); + expect(r.colWidths[0]).toBe(152); // listeners removed on mouseup + }); - // second drag on column 1: colWidths already populated → measure skipped - handle = region.querySelectorAll('.res-table th .col-resize-h')[1]; + it('dragging the last column has no neighbor, so it grows the table', () => { + const r = tableResult(); + r.colWidths = { idx: 36, 0: 100, 1: 100 }; + const app = appWithResult(r); + renderResults(app); + const region = app.dom.resultsRegion; + const win = region.ownerDocument.defaultView; + const handle = region.querySelectorAll('.res-table th .col-resize-h')[1]; // last data column handle.dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); - win.dispatchEvent(new MouseEvent('mousemove', { clientX: 180 })); - expect(r.colWidths[1]).toBe(80); + win.dispatchEvent(new MouseEvent('mousemove', { clientX: 150 })); // +50 + expect(r.colWidths[1]).toBe(150); + expect(r.colWidths[0]).toBe(100); // unchanged — no redistribution win.dispatchEvent(new MouseEvent('mouseup', {})); }); @@ -678,9 +701,9 @@ describe('multiquery script grid (#83)', () => { expect(document.querySelector('.cd-backdrop')).toBeNull(); }); - it('openRowsViewer tolerates missing columns and renders NULL cells empty', () => { + it('openRowsViewer renders NULL cells empty and flags a truncated count', () => { const app = makeApp(); - openRowsViewer(app, { rows: [['a', null]], truncated: true }); + openRowsViewer(app, { columns: [{ name: 'x', type: 'String' }, { name: 'y', type: 'String' }], rows: [['a', null]], truncated: true }); const backdrop = document.querySelector('.cd-backdrop'); expect(backdrop.querySelector('.cd-type').textContent).toContain('1+ row'); const cells = [...backdrop.querySelectorAll('tbody td')]; @@ -688,6 +711,33 @@ describe('multiquery script grid (#83)', () => { backdrop.remove(); }); + it('the rows pane is the shared grid: sortable headers + clickable cells', () => { + const app = makeApp(); + openRowsViewer(app, { columns: [{ name: 'n', type: 'UInt64' }], rows: [['2'], ['1'], ['3']] }); + let backdrop = document.querySelector('.cd-backdrop'); + // a data column header sorts the pane in place (local sort state) + const colHeader = [...backdrop.querySelectorAll('thead th')].find((th) => th.textContent.includes('n')); + click(colHeader); + backdrop = document.querySelector('.cd-backdrop'); + const firstCell = backdrop.querySelector('tbody tr td.cell .cell-val'); + expect(firstCell.textContent).toBe('1'); // ascending now + // clicking a cell opens the (stacked) cell-detail drawer + click(backdrop.querySelector('tbody td.cell')); + expect(document.querySelectorAll('.cd-backdrop').length).toBe(2); + document.querySelectorAll('.cd-backdrop').forEach((b) => b.remove()); + }); + + it('Escape closes only the topmost stacked drawer (cell first, then the rows pane)', () => { + const app = makeApp(); + openRowsViewer(app, { columns: [{ name: 'n', type: 'String' }], rows: [['x']] }); + click(document.querySelector('.cd-backdrop tbody td.cell')); // opens a stacked cell drawer + expect(document.querySelectorAll('.cd-backdrop')).toHaveLength(2); + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + expect(document.querySelectorAll('.cd-backdrop')).toHaveLength(1); // only the cell drawer closed + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + expect(document.querySelectorAll('.cd-backdrop')).toHaveLength(0); // now the rows pane + }); + it('toolbar shows live elapsed + Cancel while running, with a running footer', () => { const app = appWithResult(scriptResult(), { running: true }); renderResults(app); @@ -728,23 +778,33 @@ describe('multiquery script grid (#83)', () => { expect(app.dom.resultsRegion.querySelector('tbody td.script-time').textContent).toBe(''); }); - it('columns are drag-resizable (3 handles; keyed by plain column index)', () => { - const app = appWithResult(scriptResult()); + it('columns are drag-resizable: 3 handles, keyed by plain index (no idx col), splitter model', () => { + const r = scriptResult({ colWidths: { 0: 200, 1: 400, 2: 100 } }); // pre-seeded pair math + const app = appWithResult(r); renderResults(app); const region = app.dom.resultsRegion; - const r = app.activeTab().result; const handles = region.querySelectorAll('.script-grid th .col-resize-h'); expect(handles).toHaveLength(3); // Statement, Result, Time const win = handles[0].ownerDocument.defaultView; - handles[1].dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); - const table = region.querySelector('.res-table'); - expect(table.classList.contains('fixed')).toBe(true); - expect(Object.keys(r.colWidths).sort()).toEqual(['0', '1', '2']); // no 'idx' column - win.dispatchEvent(new MouseEvent('mousemove', { clientX: 180 })); - expect(r.colWidths[1]).toBe(80); // startW 0 + 80/1 + handles[0].dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); // col 0, neighbor col 1 + win.dispatchEvent(new MouseEvent('mousemove', { clientX: 150 })); // +50 + expect(r.colWidths[0]).toBe(250); + expect(r.colWidths[1]).toBe(350); // neighbor shrank by 50; pair sum stays 600 + expect(r.colWidths[2]).toBe(100); // untouched win.dispatchEvent(new MouseEvent('mouseup', {})); }); + it('first drag on the script grid freezes every column (keys 0/1/2, no idx)', () => { + const app = appWithResult(scriptResult()); // no colWidths → freeze path + renderResults(app); + const r = app.activeTab().result; + const region = app.dom.resultsRegion; + region.querySelector('.script-grid th .col-resize-h').dispatchEvent(new MouseEvent('mousedown', { clientX: 100, bubbles: true })); + expect(region.querySelector('.res-table').classList.contains('fixed')).toBe(true); + expect(Object.keys(r.colWidths).sort()).toEqual(['0', '1', '2']); + region.ownerDocument.defaultView.dispatchEvent(new MouseEvent('mouseup', {})); + }); + it('reapplies stored script-grid widths on re-render', () => { const app = appWithResult(scriptResult({ colWidths: { 0: 120, 1: 300, 2: 60 } })); renderResults(app); diff --git a/tests/unit/sql-split.test.js b/tests/unit/sql-split.test.js index 5995bf3..1adfa29 100644 --- a/tests/unit/sql-split.test.js +++ b/tests/unit/sql-split.test.js @@ -71,6 +71,16 @@ describe('splitStatements', () => { expect(splitStatements('SELECT 1; /* trailing')).toEqual(['SELECT 1']); }); + it('handles a realistically-commented script (-- , #, /* */ all supported)', () => { + const sql = '-- setup\nCREATE TABLE t (a Int8); /* seed */ INSERT INTO t VALUES (1); # check\nSELECT * FROM t -- trailing'; + expect(splitStatements(sql)).toEqual([ + '-- setup\nCREATE TABLE t (a Int8)', + '/* seed */ INSERT INTO t VALUES (1)', + '# check\nSELECT * FROM t -- trailing', + ]); + expect(isAutoRunnable(sql)).toBe(false); // CREATE/INSERT present → don't auto-run + }); + it('drops comment-only and whitespace-only fragments but keeps comments attached to code', () => { // The leading comment belongs to the statement that follows it (harmless to // send); the bare `;` and the trailing comment-only fragment are dropped. From 92da7f589f370c2c1188e9e0f5add146e0f7c5fb Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 18:15:15 +0000 Subject: [PATCH 10/12] fix(multiquery): retry transient session/connection failures; harden id fallback (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multiquery scripts intermittently failed with "Network error". Cause: the script's statements share one ClickHouse HTTP session and run back-to-back, so a follow-up request can reach the server before it has released the per-session lock (released asynchronously, just after the prior response is flushed). Direct to ClickHouse that's a 500 SESSION_IS_LOCKED, but behind a proxy/LB it shows up as a reset connection → fetch TypeError → "Network error". Single queries issue one session request and never race, so they were stable. Fix: - runScript retries a statement ONCE on a transient failure — a connection reset (fetch TypeError) or a SESSION_IS_LOCKED response — after a short (env-injectable) delay, with a fresh query_id. Genuine query errors are not retried (stop-on-first). - Hardened the session_id / query_id fallback for non-secure (http://) contexts where crypto.randomUUID is undefined: mix in Math.random rather than only a coarse performance.now(), so two tabs can't generate the same id and collide on the session lock. Extracted a single uid(prefix) helper for all three id sites. Per-file coverage gate green (1107 tests); build OK; e2e green (39; 2 Firefox page.goto timeouts were load flakes, pass on re-run). --- CHANGELOG.md | 10 ++++++ src/ui/app.js | 76 ++++++++++++++++++++++++++++++------------ tests/unit/app.test.js | 50 +++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 632bb00..35afcb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,16 @@ auto-generated per-PR notes; this file is the curated, human-readable history. loads rather than in-place mutation. This **completes the migration**. (#88, #91) ### Fixed +- Multiquery scripts no longer fail intermittently with **"Network error"**: the + rapid, back-to-back statements of a script share one ClickHouse HTTP session, and + a follow-up request could arrive before the server released the session lock — + surfacing (behind a proxy/LB) as a reset connection. Each statement now **retries + once** on a transient failure (connection reset or `SESSION_IS_LOCKED`) with a + fresh `query_id`. Single queries were unaffected and are unchanged. +- The `session_id` / `query_id` fallback used when `crypto.randomUUID` is + unavailable (non-secure `http://` contexts) now mixes in `Math.random` instead of + only a coarse `performance.now()`, so two tabs can't mint the same id and collide + on the session lock. - Result-table **column resize** now uses a splitter model: dragging a column's right edge trades width with its right neighbor (the table's total width and the other columns stay put), instead of growing the whole table and shifting later diff --git a/src/ui/app.js b/src/ui/app.js index 99cdf55..3585ec6 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -423,6 +423,20 @@ export function createApp(env = {}) { // --- query run --------------------------------------------------------- const now = () => (env.now || (() => win.performance.now()))(); + // A unique id for a query_id / session_id. Prefer crypto.randomUUID; its + // fallback (non-secure context, where randomUUID is undefined) must still be + // unique across tabs sharing one time origin — so mix in Math.random, not just + // `now()` (performance.now is coarsened and can repeat for back-to-back calls). + const uid = (prefix) => (cryptoObj.randomUUID + ? cryptoObj.randomUUID() + : prefix + now().toString(36) + '-' + Math.random().toString(36).slice(2, 10)); + // One retry after this delay (ms) smooths a transient failure on the rapid, + // same-session requests of a script (env-injectable; tests set 0). + const retryMs = env.retryMs != null ? env.retryMs : 250; + const sleep = (ms) => new Promise((r) => win.setTimeout(r, ms)); + // ClickHouse's transient "session is busy / locked by a concurrent client" + // (SESSION_IS_LOCKED, code 373) — retryable once the prior request releases it. + const SESSION_BUSY = /SESSION_IS_LOCKED|session .* is locked|locked by a concurrent/i; // Milliseconds since the running query started (0 when idle). Used for the // live counter, computed fresh so each render/tick shows the current value. app.elapsedMs = () => (app.state.runT0 != null ? now() - app.state.runT0 : 0); @@ -444,7 +458,7 @@ export function createApp(env = {}) { // reference loads deliberately run session-less — they fire in parallel and // would otherwise deadlock on the lock. function sessionParams(tab) { - tab.chSession = tab.chSession || (cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'sess-' + now()); + tab.chSession = tab.chSession || uid('sess-'); return { session_id: tab.chSession, session_timeout: 600 }; } @@ -497,7 +511,7 @@ export function createApp(env = {}) { if (explainView) tab.result.explainView = explainView; app.state.resultSort = { col: null, dir: 'asc' }; app.state.runT0 = t0; - app.state.runQueryId = cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'q' + t0; + app.state.runQueryId = uid('q'); app.state.abortController = new AbortController(); app.state.runTick = setInterval(tickElapsed, 100); // Keep the current Table/JSON/Chart tab across re-runs (#34); a saved-query @@ -546,6 +560,19 @@ export function createApp(env = {}) { } } + // Run one script statement, classifying the outcome for the retry logic: a + // Cancel → { aborted }; a connection-level fetch failure → { error:'Network + // error', transient } (retryable); any other throw → { error }. Otherwise the + // runQuery result itself ({ raw } | { error }). + async function attemptStatement(stmt, opts) { + try { + return await ch.runQuery(chCtx, stmt, opts); + } catch (e) { + if (e.name === 'AbortError') return { aborted: true }; + return { error: e instanceof TypeError ? 'Network error' : String((e && e.message) || e), transient: e instanceof TypeError }; + } + } + // Run a `;`-separated script sequentially: one ClickHouse request per statement // (CH's HTTP interface runs exactly one statement per request), stopping on the // first failure. Row-returning statements (SELECT/WITH/SHOW/…) are fetched as @@ -572,28 +599,33 @@ export function createApp(env = {}) { for (let i = 0; i < statements.length; i++) { const stmt = statements[i]; const rowReturning = isRowReturning(stmt); - // Fresh query_id per statement, published before the request so Cancel - // issues KILL QUERY against the statement that's actually running. - app.state.runQueryId = cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'q' + t0 + '_' + i; - let out; + // Shared session across the script's statements (temp tables / SET + // persist); over-fetch SELECTs by one past the display cap so a truncated + // result is detectable (at exactly the cap it isn't). + const opts = { + format: rowReturning ? 'JSONCompact' : 'TSV', + signal: app.state.abortController.signal, + params: { + ...sessionParams(tab), + ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}), + }, + }; const s0 = now(); // this statement's own wall-clock (grid Time column) - try { - out = await ch.runQuery(chCtx, stmt, { - format: rowReturning ? 'JSONCompact' : 'TSV', - queryId: app.state.runQueryId, - signal: app.state.abortController.signal, - // Shared session across the script's statements (temp tables / SET - // persist); over-fetch SELECTs by one past the display cap so a - // truncated result is detectable (at exactly the cap it isn't). - params: { - ...sessionParams(tab), - ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}), - }, - }); - } catch (e) { - if (e.name === 'AbortError') { aborted = true; break; } - out = { error: e instanceof TypeError ? 'Network error' : String((e && e.message) || e) }; + // Fresh query_id per attempt, published before the request so Cancel + // issues KILL QUERY against the statement that's actually running. + app.state.runQueryId = uid('q'); + let out = await attemptStatement(stmt, { ...opts, queryId: app.state.runQueryId }); + // One retry on a transient failure: rapid same-session script requests can + // race ClickHouse's async session-lock release — a connection reset + // surfaces as a fetch TypeError ("Network error"), a still-held lock as + // SESSION_IS_LOCKED. A fresh attempt (new connection, fresh query_id) + // almost always succeeds. (A mid-retry Cancel aborts the retry itself.) + if (!out.aborted && (out.transient || (out.error != null && SESSION_BUSY.test(out.error)))) { + await sleep(retryMs); + app.state.runQueryId = uid('q'); + out = await attemptStatement(stmt, { ...opts, queryId: app.state.runQueryId }); } + if (out.aborted) { aborted = true; break; } const ms = now() - s0; if (out.error != null) { entries.push({ sql: stmt, status: 'error', error: out.error, ms }); diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index 552a0d4..01adcb7 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -56,6 +56,7 @@ function env(over = {}) { Dagre: dagre, fetch: makeFetch([]), now: () => 0, + retryMs: 0, // instant script-statement retry in tests (no real 250ms wait) navigator: { clipboard: { writeText: vi.fn(async () => {}) } }, ...over, }; @@ -602,6 +603,55 @@ describe('query run', () => { expect(app.state.history).toHaveLength(0); }); + it('retries a statement once on a transient connection reset (Network error → success)', async () => { + let creates = 0; + const { app } = appForRun([ + // first attempt resets the connection (fetch TypeError); the retry succeeds + [(u, sql) => /CREATE TABLE t/.test(sql), () => { if (creates++ === 0) throw new TypeError('Failed to fetch'); return resp({ text: '' }); }], + [(u, sql) => /INSERT INTO t/.test(sql), resp({ text: '' })], + [(u, sql) => /SELECT count/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'c', type: 'UInt64' }], data: [['1']] }) })], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(creates).toBe(2); // one retry + expect(app.activeTab().result.script.map((e) => e.status)).toEqual(['ok', 'ok', 'rows']); // recovered + }); + + it('retries a statement once when the ClickHouse session is briefly locked', async () => { + let n = 0; + const locked = '{"exception":"Code: 373. DB::Exception: Session abc is locked by a concurrent client. (SESSION_IS_LOCKED)"}'; + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), () => (n++ === 0 ? resp({ ok: false, status: 500, text: locked }) : resp({ text: '' }))], + [(u, sql) => /INSERT INTO t/.test(sql), resp({ text: '' })], + [(u, sql) => /SELECT count/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'c', type: 'UInt64' }], data: [['1']] }) })], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(n).toBe(2); // retried past the transient lock + expect(app.activeTab().result.script[0].status).toBe('ok'); + }); + + it('does not retry a genuine query error (stops on the first failure)', async () => { + let inserts = 0; + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), () => { inserts++; return resp({ ok: false, status: 400, text: '{"exception":"DB::Exception: bad value"}' }); }], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(inserts).toBe(1); // no retry for a non-transient error + expect(app.activeTab().result.script[1]).toMatchObject({ status: 'error' }); + }); + + it('session_id falls back to a unique non-UUID id without crypto.randomUUID', async () => { + const noUuid = { getRandomValues: (a) => webcrypto.getRandomValues(a) }; // non-secure context: no randomUUID + const { app } = appForRun([[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"row":{}}\n']) })]], { crypto: noUuid }); + app.activeTab().sql = 'SELECT 1'; + await app.actions.run(); + const url = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); + expect(decodeURIComponent(/session_id=([^&]+)/.exec(url)[1])).toMatch(/^sess-/); // collision-resistant fallback + }); + it('run-selection: a non-empty selection runs only the selected statement (rich path) and records that text', async () => { const { app } = appForRun([ [(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })], From 5172dd451918de282f2d8a4fd1a4dd091547a12f Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 19:03:46 +0000 Subject: [PATCH 11/12] fix(multiquery): scope sessions to when needed; idempotency-aware retry (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the multiquery session model after research into the official @clickhouse/client-web (rejected: it can't control browser sockets so it wouldn't fix the keep-alive reset, its auth is static at client creation which breaks our per-request OAuth refresh seam, and it's a 4th runtime dep behind our injected fetch). Browser JS can't pin a TCP connection; ClickHouse sessions are logical (keyed by session_id), so the fix is to use them sparingly and retry safely. - Attach session_id ONLY when the SQL needs it (CREATE TEMPORARY / SET) or the tab already opened one (sticky, so temp tables / SET persist across runs). Ordinary scripts run session-less → no session-lock / affinity reset → no more intermittent "Network error" for the common case. - Idempotency-aware retry: SESSION_IS_LOCKED (rejected pre-execution) and a connection reset on a READ-ONLY statement retry once; a connection reset on an INSERT/DDL is NOT retried (it may have executed) and is surfaced as such. Per-file coverage gate green (1110 tests); build OK; e2e green (39). --- CHANGELOG.md | 17 ++++++---- src/ui/app.js | 50 ++++++++++++++++++----------- tests/unit/app.test.js | 72 +++++++++++++++++++++++++++++++----------- 3 files changed, 96 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35afcb2..0e2540b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,12 +57,17 @@ auto-generated per-PR notes; this file is the curated, human-readable history. loads rather than in-place mutation. This **completes the migration**. (#88, #91) ### Fixed -- Multiquery scripts no longer fail intermittently with **"Network error"**: the - rapid, back-to-back statements of a script share one ClickHouse HTTP session, and - a follow-up request could arrive before the server released the session lock — - surfacing (behind a proxy/LB) as a reset connection. Each statement now **retries - once** on a transient failure (connection reset or `SESSION_IS_LOCKED`) with a - fresh `query_id`. Single queries were unaffected and are unchanged. +- Multiquery scripts no longer fail intermittently with **"Network error"**. A + ClickHouse HTTP session is now attached **only when the SQL actually needs one** + (a `CREATE TEMPORARY` table or a session `SET`), or when the tab already opened + one (sticky, so that state persists across runs in the tab) — ordinary scripts + run session-less, removing the session-lock / replica-affinity reset that + surfaced (behind a proxy/LB) as a reset connection. When a session *is* in use, + a transient failure is retried **only when safe**: a `SESSION_IS_LOCKED` + (rejected before execution) or a connection reset on a **read-only** statement. + A connection reset on an `INSERT`/DDL is **not** retried — it may have executed + server-side, so it's surfaced as "the statement may have executed; re-run + manually" rather than silently double-applied. - The `session_id` / `query_id` fallback used when `crypto.randomUUID` is unavailable (non-secure `http://` contexts) now mixes in `Math.random` instead of only a coarse `performance.now()`, so two tabs can't mint the same id and collide diff --git a/src/ui/app.js b/src/ui/app.js index 3585ec6..260cf18 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -9,7 +9,7 @@ import { Icon } from './icons.js'; import { createState, activeTab, KEYS, recordHistory, recordScriptHistory, saveQuery, savedForTab, tabChart, } from '../state.js'; -import { splitStatements, isRowReturning } from '../core/sql-split.js'; +import { splitStatements, isRowReturning, leadingKeyword } from '../core/sql-split.js'; import { parseSelectResult, firstRowPreview, SELECT_ROW_CAP } from '../core/script-result.js'; import { saveJSON, saveStr } from '../core/storage.js'; import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; @@ -452,15 +452,24 @@ export function createApp(env = {}) { // of a multiquery script (and across successive runs in the tab). ClickHouse's // HTTP interface runs one statement per request and is otherwise stateless, so // without this a `CREATE TEMPORARY TABLE …; INSERT …; SELECT …` script can't - // see its own temp table. Each tab gets its own id (lazily) so tabs don't share + // see its own temp table. The id is per-tab (lazily minted) so tabs don't share // state and never collide on the per-session lock (only one query runs at a - // time, guarded by `running`). 600s idle timeout (default is 60s). Schema / - // reference loads deliberately run session-less — they fire in parallel and - // would otherwise deadlock on the lock. + // time, guarded by `running`). 600s idle timeout (default is 60s). function sessionParams(tab) { tab.chSession = tab.chSession || uid('sess-'); return { session_id: tab.chSession, session_timeout: 600 }; } + // Only TEMPORARY tables and session `SET`s need a session; permanent DDL/DML and + // SELECTs are global. So we attach a session_id ONLY when the SQL needs one — or + // when the tab already opened one (sticky, so a temp table / SET from an earlier + // run stays visible to later runs in that tab). Ordinary scripts run session-LESS, + // which avoids the session lock / replica-affinity reset that intermittently + // surfaces as a "Network error". (Schema / reference loads are always + // session-less — they fan out in parallel and would deadlock on the lock.) + const needsSession = (sqls) => sqls.some((s) => /\bTEMPORARY\b/i.test(s) || leadingKeyword(s) === 'SET'); + function sessionParamsFor(tab, sqls) { + return tab.chSession != null || needsSession(sqls) ? sessionParams(tab) : {}; + } async function run(opts) { if (app.state.running.value) return; // already running — cancel via cancel()/Esc @@ -531,7 +540,7 @@ export function createApp(env = {}) { format: fmt, queryId: app.state.runQueryId, signal: app.state.abortController.signal, - params: sessionParams(tab), + params: sessionParamsFor(tab, [srcSql]), onLine: (json) => applyStreamLine(json, tab.result), onChunk: () => renderResults(app), }); @@ -594,38 +603,41 @@ export function createApp(env = {}) { app.state.abortController = new AbortController(); app.state.runTick = setInterval(tickElapsed, 100); let aborted = false; + // Attach a session only if the script needs one (TEMPORARY / SET) or the tab + // already has one — same params for every statement, computed once. + const sp = sessionParamsFor(tab, statements); app.state.running.value = true; // the results effect paints the (empty) grid try { for (let i = 0; i < statements.length; i++) { const stmt = statements[i]; const rowReturning = isRowReturning(stmt); - // Shared session across the script's statements (temp tables / SET - // persist); over-fetch SELECTs by one past the display cap so a truncated - // result is detectable (at exactly the cap it isn't). + // Over-fetch SELECTs by one past the display cap so a truncated result is + // detectable (at exactly the cap it isn't). const opts = { format: rowReturning ? 'JSONCompact' : 'TSV', signal: app.state.abortController.signal, - params: { - ...sessionParams(tab), - ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}), - }, + params: { ...sp, ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}) }, }; const s0 = now(); // this statement's own wall-clock (grid Time column) // Fresh query_id per attempt, published before the request so Cancel // issues KILL QUERY against the statement that's actually running. app.state.runQueryId = uid('q'); let out = await attemptStatement(stmt, { ...opts, queryId: app.state.runQueryId }); - // One retry on a transient failure: rapid same-session script requests can - // race ClickHouse's async session-lock release — a connection reset - // surfaces as a fetch TypeError ("Network error"), a still-held lock as - // SESSION_IS_LOCKED. A fresh attempt (new connection, fresh query_id) - // almost always succeeds. (A mid-retry Cancel aborts the retry itself.) - if (!out.aborted && (out.transient || (out.error != null && SESSION_BUSY.test(out.error)))) { + // Retry ONLY when it's safe. SESSION_IS_LOCKED means the statement was + // rejected before running → safe to retry (any statement). A connection + // reset (fetch TypeError → "Network error") leaves it UNKNOWN whether the + // statement ran, so only retry read-only statements — re-running an + // INSERT/DDL could double-apply it. (A mid-retry Cancel aborts the retry.) + const locked = out.error != null && SESSION_BUSY.test(out.error); + if (!out.aborted && (locked || (out.transient && rowReturning))) { await sleep(retryMs); app.state.runQueryId = uid('q'); out = await attemptStatement(stmt, { ...opts, queryId: app.state.runQueryId }); } if (out.aborted) { aborted = true; break; } + // A connection reset on a non-idempotent statement: don't silently retry — + // tell the user it may have run so they can decide whether to re-run. + if (out.transient && !rowReturning) out.error = 'Network error — the statement may have executed; re-run it manually if needed.'; const ms = now() - s0; if (out.error != null) { entries.push({ sql: stmt, status: 'error', error: out.error, ms }); diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index 01adcb7..f921b6f 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -312,9 +312,21 @@ describe('query run', () => { await app.actions.run(); expect(app.activeTab().result.rows).toEqual([['1']]); expect(app.state.history.length).toBe(1); - // the query runs inside a per-tab ClickHouse session (600s timeout) - const runUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); - expect(runUrl).toMatch(/session_timeout=600/); + // a plain SELECT needs no session, so none is opened (avoids the session race) + expect(app.chCtx.fetch.mock.calls.map((c) => c[0]).some((u) => /session_id=/.test(u))).toBe(false); + }); + it('opens a ClickHouse session (600s) only for SQL that needs one (SET / TEMPORARY), and it sticks to the tab', async () => { + const { app } = appForRun([[() => true, resp({ body: streamBody(['{"row":{}}\n']) })]]); + app.activeTab().sql = 'SET max_threads = 1'; + await app.actions.run(); // SET → opens a session + const setUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); + expect(setUrl).toMatch(/session_timeout=600/); + const sid = /session_id=([^&]+)/.exec(setUrl)[1]; + app.chCtx.fetch.mockClear(); + app.activeTab().sql = 'SELECT 1'; // plain SELECT now, but the tab already has a session + await app.actions.run(); + const selUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); + expect(/session_id=([^&]+)/.exec(selUrl)[1]).toBe(sid); // sticky: same session id }); it('keeps the current result view on a plain re-run, and restores a remembered view when opened (#34)', async () => { const routes = [[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })]]; @@ -519,11 +531,21 @@ describe('query run', () => { const urls = app.chCtx.fetch.mock.calls.map((c) => c[0]); const selUrl = urls.find((u) => /max_result_rows=101/.test(u)); expect(selUrl).toMatch(/result_overflow_mode=break/); - // every statement shares one ClickHouse session (so temp tables / SET persist) - const sids = urls.filter((u) => /session_id=/.test(u)).map((u) => /session_id=([^&]+)/.exec(u)[1]); - expect(sids).toHaveLength(3); - expect(new Set(sids).size).toBe(1); - expect(urls.some((u) => /session_timeout=600/.test(u))).toBe(true); + // this script needs no session (permanent table) → session-less (no race) + expect(urls.some((u) => /session_id=/.test(u))).toBe(false); + }); + + it('a script with CREATE TEMPORARY / SET shares one session across all its statements', async () => { + const { app } = appForRun([ + [(u, sql) => /TEMPORARY/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), resp({ text: '' })], + [(u, sql) => /SELECT \* FROM t/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'a', type: 'Int8' }], data: [['1']] }) })], + ]); + app.activeTab().sql = 'CREATE TEMPORARY TABLE t (a Int8); INSERT INTO t VALUES (1); SELECT * FROM t'; + await app.actions.run(); + const sids = app.chCtx.fetch.mock.calls.map((c) => c[0]).filter((u) => /session_id=/.test(u)).map((u) => /session_id=([^&]+)/.exec(u)[1]); + expect(sids).toHaveLength(3); // all three statements carry the session + expect(new Set(sids).size).toBe(1); // and it's the same one (temp table persists) }); it('flags a SELECT as truncated when more than the cap rows come back', async () => { @@ -573,13 +595,14 @@ describe('query run', () => { expect(app.state.history).toHaveLength(0); }); - it('reports a network error (TypeError) per statement', async () => { + it('reports a connection reset (TypeError) on a non-idempotent statement without retrying it', async () => { const { app } = appForRun([ [(u, sql) => /CREATE TABLE t/.test(sql), () => { throw new TypeError('fetch failed'); }], ]); app.activeTab().sql = SCRIPT; await app.actions.run(); - expect(app.activeTab().result.script[0]).toMatchObject({ status: 'error', error: 'Network error' }); + expect(app.activeTab().result.script[0]).toMatchObject({ status: 'error' }); + expect(app.activeTab().result.script[0].error).toMatch(/may have executed/); }); it('surfaces a non-abort thrown error message per statement', async () => { @@ -603,20 +626,33 @@ describe('query run', () => { expect(app.state.history).toHaveLength(0); }); - it('retries a statement once on a transient connection reset (Network error → success)', async () => { - let creates = 0; + it('retries a READ-ONLY statement once on a transient connection reset (Network error → success)', async () => { + let sel = 0; const { app } = appForRun([ - // first attempt resets the connection (fetch TypeError); the retry succeeds - [(u, sql) => /CREATE TABLE t/.test(sql), () => { if (creates++ === 0) throw new TypeError('Failed to fetch'); return resp({ text: '' }); }], + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], [(u, sql) => /INSERT INTO t/.test(sql), resp({ text: '' })], - [(u, sql) => /SELECT count/.test(sql), resp({ text: JSON.stringify({ meta: [{ name: 'c', type: 'UInt64' }], data: [['1']] }) })], + // the SELECT (idempotent) resets once, then the retry succeeds + [(u, sql) => /SELECT count/.test(sql), () => { if (sel++ === 0) throw new TypeError('Failed to fetch'); return resp({ text: JSON.stringify({ meta: [{ name: 'c', type: 'UInt64' }], data: [['1']] }) }); }], ]); app.activeTab().sql = SCRIPT; await app.actions.run(); - expect(creates).toBe(2); // one retry + expect(sel).toBe(2); // retried the SELECT expect(app.activeTab().result.script.map((e) => e.status)).toEqual(['ok', 'ok', 'rows']); // recovered }); + it('does NOT retry a non-idempotent statement on a connection reset (surfaces "may have executed")', async () => { + let inserts = 0; + const { app } = appForRun([ + [(u, sql) => /CREATE TABLE t/.test(sql), resp({ text: '' })], + [(u, sql) => /INSERT INTO t/.test(sql), () => { inserts++; throw new TypeError('Failed to fetch'); }], + ]); + app.activeTab().sql = SCRIPT; + await app.actions.run(); + expect(inserts).toBe(1); // the INSERT is NOT re-sent — it may have run server-side + expect(app.activeTab().result.script[1]).toMatchObject({ status: 'error' }); + expect(app.activeTab().result.script[1].error).toMatch(/may have executed/); + }); + it('retries a statement once when the ClickHouse session is briefly locked', async () => { let n = 0; const locked = '{"exception":"Code: 373. DB::Exception: Session abc is locked by a concurrent client. (SESSION_IS_LOCKED)"}'; @@ -645,8 +681,8 @@ describe('query run', () => { it('session_id falls back to a unique non-UUID id without crypto.randomUUID', async () => { const noUuid = { getRandomValues: (a) => webcrypto.getRandomValues(a) }; // non-secure context: no randomUUID - const { app } = appForRun([[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"row":{}}\n']) })]], { crypto: noUuid }); - app.activeTab().sql = 'SELECT 1'; + const { app } = appForRun([[() => true, resp({ body: streamBody(['{"row":{}}\n']) })]], { crypto: noUuid }); + app.activeTab().sql = 'SET max_threads = 1'; // SET opens a session, so a session_id is sent await app.actions.run(); const url = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); expect(decodeURIComponent(/session_id=([^&]+)/.exec(url)[1])).toMatch(/^sess-/); // collision-resistant fallback From 86d51d5e9a69ebb9317aac289b1dffca06332e07 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 19:32:37 +0000 Subject: [PATCH 12/12] fix(multiquery): drop session_timeout override; default 60s suffices (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ClickHouse resets a session's idle timer when each query is *released* (end of the request) and cancels it while a query runs (Session.cpp / NamedSessionsStorage), so the countdown only ticks during true idle between queries. A script's statements are sent back-to-back (gap well under the 60s default), so the session never lapses mid-batch — no session_timeout override needed. Send only session_id and rely on the server default. Tests + build green (1110). --- CHANGELOG.md | 8 ++++---- src/ui/app.js | 7 +++++-- tests/unit/app.test.js | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e2540b..167ccf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,10 +21,10 @@ auto-generated per-PR notes; this file is the curated, human-readable history. side pane; effectful statements show **OK**. Each grid row also shows that statement's own execution time (the toolbar still shows the script total). The click-to-open row pane is the **same sortable + resizable grid** as the main - results table (one shared component). Statements run inside a **per-tab - ClickHouse HTTP session** (`session_id` + `session_timeout=600`), so a script's - temporary tables / `SET`s persist across its separate per-statement requests. - Cancel aborts mid-script. Splitting + results table (one shared component). A script that needs cross-statement state + (a `CREATE TEMPORARY` table or a session `SET`) runs inside a **per-tab + ClickHouse HTTP session** so that state persists across its separate + per-statement requests; ordinary scripts run session-less. Cancel aborts mid-script. Splitting is purely lexical (`src/core/sql-split.js`), skipping `;` inside string/identifier literals and `--` / `#` / `/* */` comments. Known limitation: an `INSERT … FORMAT …` with inline data containing `;` mis-splits — run those on their own. diff --git a/src/ui/app.js b/src/ui/app.js index 260cf18..91ad406 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -454,10 +454,13 @@ export function createApp(env = {}) { // without this a `CREATE TEMPORARY TABLE …; INSERT …; SELECT …` script can't // see its own temp table. The id is per-tab (lazily minted) so tabs don't share // state and never collide on the per-session lock (only one query runs at a - // time, guarded by `running`). 600s idle timeout (default is 60s). + // time, guarded by `running`). No `session_timeout` override is needed: + // ClickHouse resets the idle timer when each query is *released* (end of the + // request, not the start) and cancels it while a query runs, so the default + // (60s) never lapses between a script's back-to-back statements. function sessionParams(tab) { tab.chSession = tab.chSession || uid('sess-'); - return { session_id: tab.chSession, session_timeout: 600 }; + return { session_id: tab.chSession }; } // Only TEMPORARY tables and session `SET`s need a session; permanent DDL/DML and // SELECTs are global. So we attach a session_id ONLY when the SQL needs one — or diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index f921b6f..683de10 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -315,12 +315,12 @@ describe('query run', () => { // a plain SELECT needs no session, so none is opened (avoids the session race) expect(app.chCtx.fetch.mock.calls.map((c) => c[0]).some((u) => /session_id=/.test(u))).toBe(false); }); - it('opens a ClickHouse session (600s) only for SQL that needs one (SET / TEMPORARY), and it sticks to the tab', async () => { + it('opens a ClickHouse session only for SQL that needs one (SET / TEMPORARY), and it sticks to the tab', async () => { const { app } = appForRun([[() => true, resp({ body: streamBody(['{"row":{}}\n']) })]]); app.activeTab().sql = 'SET max_threads = 1'; await app.actions.run(); // SET → opens a session const setUrl = app.chCtx.fetch.mock.calls.map((c) => c[0]).find((u) => /session_id=/.test(u)); - expect(setUrl).toMatch(/session_timeout=600/); + expect(setUrl).not.toMatch(/session_timeout/); // rely on the server default (60s) — see sessionParams const sid = /session_id=([^&]+)/.exec(setUrl)[1]; app.chCtx.fetch.mockClear(); app.activeTab().sql = 'SELECT 1'; // plain SELECT now, but the tab already has a session