From cc8a056cc738c026ce99f7490bf2f36ff8e5c952 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Fri, 17 Jul 2026 09:07:34 +0000 Subject: [PATCH] =?UTF-8?q?refactor(#276):=20extract=20QueryExecutionServi?= =?UTF-8?q?ce=20=E2=80=94=20phases=200-1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0: architecture guard build/check-boundaries.mjs (wired into pretest as check:arch) enforcing the day-1 rule — src/application/** must not import src/ui/** or src/editor/** (type-only imports count). Grows with later #276 phases. Characterization coverage was audited against the issue's Phase-0 list and already exists in tests/unit/app.test.ts and dashboard.test.ts (streaming, explicit FORMAT, cancel+partial, script retry matrix incl. the "may have executed" warning, Basic/OAuth refresh, export cancellation, schema-graph stale-write, save/share validation). Phase 1: src/application/query-execution-service.ts — the shared request/stream/normalize core (formerly app.runReadInto) plus runScript's per-statement transport loop (SESSION_IS_LOCKED / transient-read-only retry classification, stop-on-first-failure, fresh query_id per attempt published synchronously via onStatementStart so Cancel's KILL QUERY always targets the live statement). Constructible without App/AppState/ DOM; deps injected ({runQuery, killQuery, ctx, now, uid, retryMs, sleep, sqlString}) — deliberately no wallNow (the F6 wave-clock invariant stays caller-side) and deliberately kill(queryId) instead of the issue's cancel(operationId) registry (cancellation stays caller-owned; see the issue comment). Behavior, wire format, retry rules, and error strings are byte-identical. All four callers migrated in one pass and app.runReadInto deleted: run()/runScript() (app.ts), dashboard tiles + filter sources (dashboard.ts), detached Data view (results.ts) → app.exec.executeRead / executeScript; cancel() → app.exec.kill. fake-app + ~70 test-mock sites renamed to the exec shape. Type homes tightened along the way: ResultSort → core/sort.ts and ScriptEntry → core/script-result.ts (old paths re-export, zero importer churn); filterExecution/panelExecution params bags and tab.chSession now carry their real wire types, deleting every as-cast at the execution call sites. Metrics (issue Phase-0 requirement): app.ts 2964 → 2878 lines; the App contract loses runReadInto and gains one nested exec member; the new service is 264 lines at 100/97.87/100/100 per-file coverage. Gate: npm test (103 files / 2940 tests) green, npm run build green, e2e chromium+webkit fully green, firefox green serialized (--workers=1; the parallel-run timeouts are the documented environment flake). Part of #276 (phases 0-1 of 5; roadmap #68 updated). Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2 Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 21 + build/check-boundaries.mjs | 95 +++++ package.json | 3 +- src/application/query-execution-service.ts | 264 ++++++++++++ src/core/dashboard.ts | 4 +- src/core/filter-execution.ts | 4 +- src/core/panel-execution.ts | 6 +- src/core/script-result.ts | 20 + src/core/sort.ts | 7 + src/state.ts | 16 +- src/ui/app.ts | 172 ++------ src/ui/app.types.ts | 11 +- src/ui/dashboard.ts | 12 +- src/ui/results.ts | 39 +- tests/helpers/fake-app.ts | 34 +- tests/unit/ch-client.test.ts | 2 +- tests/unit/dashboard.test.ts | 96 ++--- tests/unit/query-execution-service.test.ts | 462 +++++++++++++++++++++ tests/unit/results.test.ts | 88 ++-- 19 files changed, 1074 insertions(+), 282 deletions(-) create mode 100644 build/check-boundaries.mjs create mode 100644 src/application/query-execution-service.ts create mode 100644 tests/unit/query-execution-service.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cc153a0..58e10b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,27 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Changed +- **Query execution extracted into an application service** (#276 Phases 0–1, + the first step of the app.ts → services refactor). The shared + request/stream/normalize core (`runReadInto`) and the multiquery-script + transport loop (per-statement retry/classification, stop-on-first-failure, + per-attempt `query_id` publication for Cancel's `KILL QUERY`) now live in + `src/application/query-execution-service.ts` — constructible without + `App`/`AppState`/DOM, with every side effect (ch-client, clock, uid, timer) + injected. `app.runReadInto` is deleted; the workbench `run()`/`runScript()`, + dashboard tiles, and the detached Data view all execute through + `app.exec.executeRead`/`executeScript`, and `cancel()` uses the stateless + `app.exec.kill(queryId)`. Behavior, wire format, retry rules, and error + messages are byte-identical. A new architecture guard + (`build/check-boundaries.mjs`, wired into `pretest` as `check:arch`) enforces + the day-1 boundary rule: `src/application/**` must not import `src/ui/**` or + `src/editor/**`. Type homes tightened along the way: `ResultSort` moved to + `core/sort.ts` and `ScriptEntry` to `core/script-result.ts` (old import paths + re-exported), and the `filterExecution`/`panelExecution` params bags are now + the strict wire shape (`Record`), deleting the + narrowing casts at every execution call site. + ### Added - **Bundle-size report on every PR** (#275). `npm run size-report` builds the production artifact once (through the same `buildArtifact` the release uses, so diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs new file mode 100644 index 0000000..4da81bb --- /dev/null +++ b/build/check-boundaries.mjs @@ -0,0 +1,95 @@ +// Architecture boundary guard (issue #276 Phase 0). Day-1 rule only: modules +// under src/application/ must not import from src/ui/ or src/editor/ — the +// route/session layer coordinates state and network, it does not reach into +// DOM rendering or the CodeMirror editor adapters. `import type` counts too: +// a type-only import still couples the layers at compile time. Later phases +// (#276 Phases 3-5) extend the rule set (e.g. route sessions must not import +// each other) — add checks here rather than growing a second script. +// +// Hand-rolled regex scan, no AST parser: the codebase has no exotic import +// syntax, so scanning for import/export specifiers is enough and keeps this +// a zero-dependency, sub-second pretest step. + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const appDir = path.join(repoRoot, 'src', 'application'); +const FORBIDDEN = ['src/ui', 'src/editor']; +const SOURCE_EXT = /\.(ts|tsx|js|mjs)$/; + +function collectFiles(dir) { + const out = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...collectFiles(full)); + else if (SOURCE_EXT.test(entry.name)) out.push(full); + } + return out; +} + +const files = fs.existsSync(appDir) ? collectFiles(appDir) : []; +if (files.length === 0) { + console.log('check-boundaries: no files under src/application/ yet'); + process.exit(0); +} + +// Matches, in order: static `import ... from '...'` (incl. `import type`), +// `export ... from '...'` (incl. `export type`), a bare side-effect +// `import '...'`, and dynamic `import('...')`. Each pattern requires only +// identifier/brace/comma/whitespace characters between the keyword and +// `from`, so it can't skip past a from-less import into a later statement's +// clause, and `\b` keeps it off the word "import" inside an identifier. +const SPECIFIER_PATTERNS = [ + /\bimport\s+[\w*{}\s,]+\s+from\s*['"]([^'"]+)['"]/g, + /\bexport\s+[\w*{}\s,]+\s+from\s*['"]([^'"]+)['"]/g, + /\bimport\s*['"]([^'"]+)['"]/g, + /\bimport\s*\(\s*['"]([^'"]+)['"]/g, +]; + +function extractSpecifiers(source) { + const specs = []; + for (const pattern of SPECIFIER_PATTERNS) { + pattern.lastIndex = 0; + let match; + while ((match = pattern.exec(source))) specs.push(match[1]); + } + return specs; +} + +// Relative specifiers resolve like esbuild/tsc do: a `.js` specifier written +// against a `.ts` source file still resolves to the `.ts` file on disk. +function resolveRelative(fromFile, spec) { + const resolved = path.resolve(path.dirname(fromFile), spec); + const noExt = resolved.replace(/\.(ts|tsx|js|mjs)$/, ''); + const candidates = [ + resolved, `${noExt}.ts`, `${noExt}.tsx`, `${noExt}.js`, `${noExt}.mjs`, + path.join(resolved, 'index.ts'), path.join(resolved, 'index.js'), + ]; + return candidates.find((candidate) => fs.existsSync(candidate)) ?? resolved; +} + +const violations = []; +for (const file of files) { + const source = fs.readFileSync(file, 'utf8'); + for (const spec of extractSpecifiers(source)) { + if (!spec.startsWith('.')) continue; // bare/package specifiers can't reach src/ui or src/editor + const resolved = resolveRelative(file, spec); + const relResolved = path.relative(repoRoot, resolved).split(path.sep).join('/'); + const inForbidden = FORBIDDEN.some((f) => relResolved === f || relResolved.startsWith(`${f}/`)); + if (inForbidden) { + const relFile = path.relative(repoRoot, file).split(path.sep).join('/'); + violations.push(`${relFile} → ${spec} (resolved: ${relResolved})`); + } + } +} + +if (violations.length) { + console.error('check-boundaries: src/application/ must not import src/ui/ or src/editor/ (issue #276, day-1 rule):'); + for (const line of violations) console.error(` ${line}`); + process.exit(1); +} + +console.log(`check-boundaries: OK (${files.length} file${files.length === 1 ? '' : 's'} under src/application/, no ui/editor imports)`); +process.exit(0); diff --git a/package.json b/package.json index a289ad3..99eb1b6 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ "prebuild": "npm run check:schemas", "build": "node build/build.mjs", "size-report": "node build/size-report.mjs", - "pretest": "npm run check:schemas && npm run check:types", + "check:arch": "node build/check-boundaries.mjs", + "pretest": "npm run check:schemas && npm run check:arch && npm run check:types", "test": "TZ=America/New_York vitest run --coverage --config tests/vitest.config.ts", "test:watch": "TZ=America/New_York vitest --config tests/vitest.config.ts", "test:e2e": "playwright test", diff --git a/src/application/query-execution-service.ts b/src/application/query-execution-service.ts new file mode 100644 index 0000000..feb4845 --- /dev/null +++ b/src/application/query-execution-service.ts @@ -0,0 +1,264 @@ +// #276 Phase 1's QueryExecutionService — the shared request/stream/normalize +// core (now `app.exec.executeRead`, formerly a private helper of the same name +// inline in app.ts) plus the multiquery script transport loop (formerly inline +// in app.ts's `runScript`'s per-statement retry/classify logic), extracted so +// it is constructible without App/AppState/DOM. Architectural rules (issue +// #276): no imports from `src/ui/**` or `src/editor/**` (a pretest check +// enforces this); every side effect is injected as a narrow dependency bag +// (`QueryExecutionDeps`), never imported directly, so the whole service is +// testable with plain stubs, exactly like `src/net/ch-client.ts`'s own `ChCtx` +// seam. Cancellation stays caller-owned: the caller holds its own +// `AbortController` and publishes the live `query_id` via `onStatementStart` +// (synchronously, before each attempt) so its own Cancel button can target +// it; `kill()` here is a stateless, one-shot best-effort `KILL QUERY` — +// deliberately NOT a `cancel(operationId)` registry (see the issue #276 +// discussion on why the service itself never tracks in-flight operations). + +import type { ChCtx, RunQueryOptions, RunQueryResult } from '../net/ch-client.js'; +import type { runQuery, killQuery } from '../net/ch-client.js'; +import { applyStreamLine } from '../core/stream.js'; +import type { StreamResult } from '../core/stream.js'; +import { isRowReturning } from '../core/sql-split.js'; +import { parseSelectResult, firstRowPreview, SELECT_ROW_CAP } from '../core/script-result.js'; +import type { ScriptEntry } from '../core/script-result.js'; + +// ── Injected dependency seam ───────────────────────────────────────────────── + +/** Every side effect this service needs, injected as a narrow bag — production + * wires the real `net/ch-client.js` functions + browser clock/crypto/timer; + * tests inject plain stubs. Mirrors `ch-client.ts`'s own `ChCtx` seam. */ +export interface QueryExecutionDeps { + /** Runs one statement and returns its parsed/streamed outcome. */ + runQuery: typeof runQuery; + /** Best-effort `KILL QUERY` for a query_id. */ + killQuery: typeof killQuery; + /** The live ClickHouse auth context — a *provider*, not a value: the caller + * may rebuild it (e.g. after a token refresh) between calls, so the + * service always reads the current one rather than closing over a stale + * snapshot. */ + ctx: () => ChCtx; + /** Perf clock for per-statement elapsed ms. Deliberately NOT the wall clock + * (`wallNow`) the #173 parameter pipeline uses for epoch-relative values — + * that F6 invariant (one wall-clock snapshot per run wave, resolved before + * any auth await) lives entirely caller-side; this service never resolves + * a wave clock of its own. */ + now: () => number; + /** Mints a query_id, prefixed `prefix` — matches app.ts's `uid('q')`. */ + uid: (prefix: string) => string; + /** Delay (ms) before the one same-session retry. */ + retryMs: number; + /** Injected timer — `sleep(retryMs)` before a retry attempt. */ + sleep: (ms: number) => Promise; + /** SQL-string-quoting function `killQuery` needs to build its + * `KILL QUERY WHERE query_id = …` literal (matches `core/format.js`'s + * `sqlString`). */ + sqlString: (s: unknown) => string; +} + +// ── executeRead ────────────────────────────────────────────────────────────── + +/** `executeRead`'s request — an already-prepared read: the wire SQL, output + * format (default 'Table'), client-side row cap (default 0 = uncapped), + * native ClickHouse query parameters, the caller's own `AbortSignal` / + * query_id, and a per-chunk repaint hook. */ +export interface ExecuteReadRequest { + sql: string; + format?: string; + rowLimit?: number; + params?: Record; + signal?: AbortSignal; + queryId?: string; + /** Per-read repaint hook — called with no arguments on each streamed chunk + * (the workbench repaints its pane; a tile/detached view repaints its own + * surface). Absent entirely when the caller passes none — no wrapper + * closure is created in that case. */ + onChunk?: () => void; +} + +// ── executeScript ──────────────────────────────────────────────────────────── + +/** One statement of a script run, as the caller hands it to `executeScript`. + * `sql` is the authored statement — the grid display text AND what + * `isRowReturning` classifies; `execSql` is the wire text actually sent + * (the #165 execution view: inactive optional blocks stripped, byte- + * identical to `sql` for SQL without blocks); `params` is the caller-merged + * session_id + bound native-parameter args for this one statement — the + * service adds the row-returning-only over-fetch cap on top, never a + * session_id or bound arg. */ +export interface ScriptStatement { + sql: string; + execSql: string; + params: Record; +} + +/** `executeScript`'s request: the statements to run in order, the caller's + * own `AbortSignal` (shared by every attempt), and two callbacks that let + * the caller own orchestration (tab/result mutation, the running signal, + * history, renders, boundParams recording, schema reload) while the service + * owns transport/retry/classify. `onStatementStart` fires synchronously + * BEFORE each attempt (fresh query_id per attempt, including the retry) so + * the caller can publish it for Cancel's `KILL QUERY`; `onStatementResult` + * fires once per pushed entry, after it's pushed. */ +export interface ScriptExecutionRequest { + statements: ScriptStatement[]; + signal?: AbortSignal; + onStatementStart: (index: number, info: { queryId: string; attempt: 1 | 2 }) => void; + onStatementResult: (index: number, entry: ScriptEntry) => void; +} + +/** `executeScript`'s result: the entries produced (one per statement that + * actually ran to completion or failure — an aborted statement gets none), + * and whether the script was cancelled mid-run. */ +export interface ScriptExecutionResult { + entries: ScriptEntry[]; + aborted: boolean; +} + +/** `attemptStatement`'s outcome — `ch.runQuery`'s own `RunQueryResult` + * (`streamed` unused here), plus the two classified failures the retry + * logic branches on. */ +export interface AttemptResult extends RunQueryResult { + aborted?: boolean; + transient?: boolean; +} + +// 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; + +/** The service surface `app.exec` will hold. */ +export interface QueryExecutionService { + executeRead(result: StreamResult, request: ExecuteReadRequest): Promise; + executeScript(request: ScriptExecutionRequest): Promise; + kill(queryId: string | null | undefined): Promise; +} + +/** Build a `QueryExecutionService` bound to `deps`. Trivial constructor — no + * validation, no defaulting; the caller supplies every field of `deps` + * exactly as it wants it used. */ +export function createQueryExecutionService(deps: QueryExecutionDeps): QueryExecutionService { + // 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: string, opts: RunQueryOptions): Promise { + try { + return await deps.runQuery(deps.ctx(), stmt, opts); + } catch (e) { + if (e instanceof Error && e.name === 'AbortError') return { aborted: true }; + return { error: e instanceof TypeError ? 'Network error' : String((e instanceof Error && e.message) || e), transient: e instanceof TypeError }; + } + } + + // Execute one already-prepared read request into a caller-owned `result`, + // with NO tab/global-state side effects. This is the request+stream+normalize + // core that the workbench run(), the dashboard tiles, and the detached Data + // view (#185) all perform identically: fold streamed lines into `result` via + // applyStreamLine, capture a raw (explicit-FORMAT/EXPLAIN) body, and classify + // an abort/network/other failure onto the result — never throwing. The caller + // owns token freshness (resolved before this call), the AbortController / + // query_id, parameter preparation, session_id, and any recent-value recording. + // `onChunk` is the per-read repaint hook (the workbench repaints its pane; a + // tile/detached view repaints its own surface). Returns the mutated `result`. + async function executeRead( + result: StreamResult, + { + sql, format = 'Table', rowLimit = 0, params, signal, queryId, onChunk, + }: ExecuteReadRequest, + ): Promise { + try { + const out = await deps.runQuery(deps.ctx(), sql, { + format, + resultRowLimit: rowLimit, + queryId, + signal, + params, + onLine: (json) => applyStreamLine(json, result), + onChunk, + }); + if (out.error != null) result.error = out.error; + else if (out.raw != null) { + result.rawText = out.raw; + result.progress.bytes = out.raw.length; + } + } catch (e) { + // Cancel = abort: keep whatever streamed in, flag it partial (no error). + if (e instanceof Error && e.name === 'AbortError') result.cancelled = true; + else if (e instanceof TypeError) result.error = 'Network error'; + else result.error = String((e instanceof Error && e.message) || e); + } + return result; + } + + // Run a `;`-separated script's transport loop 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 + // SELECT_ROW_CAP; everything else runs for effect and reports OK. + async function executeScript(req: ScriptExecutionRequest): Promise { + const { statements, signal, onStatementStart, onStatementResult } = req; + const entries: ScriptEntry[] = []; + let aborted = false; + for (let i = 0; i < statements.length; i++) { + const stmt = statements[i]; + const rowReturning = isRowReturning(stmt.sql); + // Over-fetch SELECTs by one past the display cap so a truncated result is + // detectable (at exactly the cap it isn't). + const opts: RunQueryOptions = { + format: rowReturning ? 'JSONCompact' : 'TSV', + signal, + params: { ...stmt.params, ...(rowReturning ? { max_result_rows: SELECT_ROW_CAP + 1, result_overflow_mode: 'break' } : {}) }, + }; + const s0 = deps.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. + let queryId = deps.uid('q'); + onStatementStart(i, { queryId, attempt: 1 }); + let out = await attemptStatement(stmt.execSql, { ...opts, queryId }); + // 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 deps.sleep(deps.retryMs); + queryId = deps.uid('q'); + onStatementStart(i, { queryId, attempt: 2 }); + out = await attemptStatement(stmt.execSql, { ...opts, queryId }); + } + 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 = deps.now() - s0; + let entry: ScriptEntry; + if (out.error != null) { + entry = { sql: stmt.sql, status: 'error', error: out.error, ms }; + entries.push(entry); + onStatementResult(i, entry); + break; // stop-on-first-failure: skip the remaining statements + } + if (rowReturning) { + const sel = parseSelectResult(out.raw, SELECT_ROW_CAP); + entry = { + sql: stmt.sql, status: 'rows', columns: sel.columns, rows: sel.rows, truncated: sel.truncated, preview: firstRowPreview(sel.rows), ms, + }; + } else { + entry = { sql: stmt.sql, status: 'ok', ms }; + } + entries.push(entry); + onStatementResult(i, entry); + } + return { entries, aborted }; + } + + // Stop an in-flight query: best-effort KILL QUERY for `queryId` (mirrors + // app.ts's cancel(), minus the AbortController.abort() the caller performs + // itself — cancellation stays caller-owned; see the module doc above). + function kill(queryId: string | null | undefined): Promise { + return deps.killQuery(deps.ctx(), queryId, deps.sqlString); + } + + return { executeRead, executeScript, kill }; +} diff --git a/src/core/dashboard.ts b/src/core/dashboard.ts index 2a7bfce..bc8fa39 100644 --- a/src/core/dashboard.ts +++ b/src/core/dashboard.ts @@ -4,7 +4,7 @@ // new schema. This module holds the route helpers and the tile result caps. // (Per-tile classification moved to core/panel-cfg.js's autoPanel/resolvePanel // in #166 — the panel union replaced classifyTile's chart-vs-skip ladder. The -// tiles stream through the shared `app.runReadInto` seam as of #193, so the +// tiles stream through the shared `app.exec.executeRead` seam as of #193/#276, so the // former `FORMAT JSON` → array-rows transform and its SQL prep were retired.) /** @@ -116,7 +116,7 @@ export const DASH_TABLE_DISPLAY_CAP = 1000; // (The tiles' SQL prep + `FORMAT JSON` → array-rows transform — the former // `dashboardTileSql` / `parseJsonResult` — were retired in #193 when the tiles -// moved onto the shared streaming `app.runReadInto` seam. The client row bound +// moved onto the shared streaming `app.exec.executeRead` seam. The client row bound // is now `newResult('Table', DASH_TILE_ROW_CAP)`'s trim + `capped` flag, and // the tile result shape is pinned by `dashboardTileResult` in src/ui/dashboard.js.) diff --git a/src/core/filter-execution.ts b/src/core/filter-execution.ts index 023a186..092484f 100644 --- a/src/core/filter-execution.ts +++ b/src/core/filter-execution.ts @@ -58,7 +58,7 @@ export function filterSqlDiagnostics(sql?: string | null): FilterSqlDiagnostic[] /** `filterExecution`'s caller-supplied defaults — today just extra/overriding * ClickHouse HTTP `params`. */ export interface FilterExecutionDefaults { - params?: Record; + params?: Record; } /** `filterExecution`'s return shape: the owned, lossless, bounded structured @@ -67,7 +67,7 @@ export interface FilterExecutionPlan { owned: true; format: 'Filter'; rowLimit: number; - params: Record; + params: Record; diagnostics: FilterSqlDiagnostic[]; error: string | null; } diff --git a/src/core/panel-execution.ts b/src/core/panel-execution.ts index 29db5f9..5b90826 100644 --- a/src/core/panel-execution.ts +++ b/src/core/panel-execution.ts @@ -26,7 +26,9 @@ export function explicitPanel(query: unknown): Panel | null { export interface PanelExecutionDefaults { format?: string; rowLimit?: number; - params?: Record; + /** ClickHouse HTTP params — the strict wire shape (`param_` bindings, + * settings), so an execution seam can forward them without narrowing. */ + params?: Record; [k: string]: unknown; } @@ -37,7 +39,7 @@ export interface PanelExecutionDefaults { export interface PanelExecutionResult { owned: boolean; error: string | null; - params: Record; + params: Record; format?: string; rowLimit?: number; [k: string]: unknown; diff --git a/src/core/script-result.ts b/src/core/script-result.ts index 248d197..45ae868 100644 --- a/src/core/script-result.ts +++ b/src/core/script-result.ts @@ -6,6 +6,7 @@ // of the first row in column 2; clicking it opens the full table in a side pane. import { truncate } from './format.js'; +import type { ResultSort } from './sort.js'; // 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 @@ -61,6 +62,25 @@ export function parseSelectResult(rawText: unknown, cap: number = SELECT_ROW_CAP return { columns, rows: data.slice(0, cap), truncated: data.length > cap }; } +/** One statement's outcome in a multiquery script run (#83) — one entry of + * `ScriptResult.script`. `sql`/`ms` ride on every status; the rest is + * per-status (a 'rows' entry is the only one carrying real data). */ +export type ScriptEntry = { sql?: string; ms?: number } & ( + | { status: 'ok' } + | { status: 'error'; error?: string } + | { + status: 'rows'; + columns: { name: string; type: string }[]; + rows: unknown[][]; + truncated?: boolean; + preview?: string; + /** Local sort/width state, lazily attached by openRowsViewer — persists + * for the life of that statement's open rows pane. */ + viewerSort?: ResultSort; + viewerWidths?: Record; + } +); + /** * 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 diff --git a/src/core/sort.ts b/src/core/sort.ts index 3c87e07..35ad05c 100644 --- a/src/core/sort.ts +++ b/src/core/sort.ts @@ -1,6 +1,13 @@ // Pure result-table sorting. Numeric when both cells parse as numbers, // lexicographic otherwise. Returns a new array; never mutates the input. +/** The global results-table sort: a zero-based column index (grid-render.js + * sorts positionally), or `col: null` for the natural row order. */ +export interface ResultSort { + col: number | null; + dir: 'asc' | 'desc'; +} + /** True when `v` is a string that is wholly a number literal. */ function looksNumeric(v: unknown): boolean { const s = String(v); diff --git a/src/state.ts b/src/state.ts index 7a5f757..652630f 100644 --- a/src/state.ts +++ b/src/state.ts @@ -16,6 +16,7 @@ import { loadStr as loadStrUntyped, saveStr as saveStrUntyped, } from './core/storage.js'; import { emptyRecentMap as emptyRecentMapUntyped } from './core/recent-values.js'; +import type { ResultSort } from './core/sort.js'; import { defaultSpecValidationService as defaultSpecValidationServiceUntyped, evaluateSpecText as evaluateSpecTextUntyped, @@ -154,8 +155,9 @@ export interface QueryTab { * adapter's dynamic sources. */ lastSuccessfulResultColumns: { name: string; type?: string }[]; savedId: string | null; - /** Set post-construction (app.js) once a query has run on this tab. */ - chSession?: unknown; + /** ClickHouse HTTP session id (lazily minted `sess-…` uid) — set + * post-construction (app.js) once a query has run on this tab. */ + chSession?: string; } /** One executed-query history entry (most-recent first, capped at 50). */ @@ -168,12 +170,10 @@ export interface HistoryEntry { ms: number; } -/** The global results-table sort: a zero-based column index (grid-render.js - * sorts positionally), or `col: null` for the natural row order. */ -export interface ResultSort { - col: number | null; - dir: 'asc' | 'desc'; -} +// The global results-table sort — moved to core/sort.ts (#276 Phase 1) so +// the pure sort module owns its own type; re-exported here so every existing +// importer of ResultSort-from-state keeps compiling unchanged. +export type { ResultSort } from './core/sort.js'; /** One recorded recent value for a variable (core/recent-values.js). */ export interface RecentValueEntry { diff --git a/src/ui/app.ts b/src/ui/app.ts index e1ff1fa..14079f1 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -22,7 +22,6 @@ import type { ParameterAnalysis, PreparedSource, PreparedBatch, PreparedFieldState, ValidationMode, BoundParamSnapshot, FieldControl, } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.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, supportsExplainPretty, userShortName, withStatementBreak, detectSqlFormat, isSchemaMutatingSql, prepareExportSql, formatBytes, formatRows } from '../core/format.js'; @@ -32,8 +31,7 @@ import { buildCardGraph } from '../core/schema-cards.js'; import type { SchemaCardColumnRow } from '../core/schema-cards.js'; import { resolveTarget } from '../core/target.js'; import { toTSV, formatFileMeta, exportFilename, scriptExportName } from '../core/export.js'; -import { newResult, applyStreamLine, parseErrorPos, findExceptionFrame } from '../core/stream.js'; -import type { StreamResult } from '../core/stream.js'; +import { newResult, parseErrorPos, findExceptionFrame } from '../core/stream.js'; import { buildResultSource } from '../core/query-source.js'; import { encodeShare } from '../core/share.js'; import { queryName, queryPanel, withQuerySpec } from '../core/saved-query.js'; @@ -99,6 +97,7 @@ import type { App, ActionsRegistry, SchemaFocus, ChCtx as AppChCtx } from './app import type { CreateAppEnv } from '../env.types.js'; import type { SchemaGraphFocus, SchemaGraphNode, SchemaGraphEdge } from '../core/schema-graph.js'; import type { LineageFocus } from '../net/ch-client.js'; +import { createQueryExecutionService } from '../application/query-execution-service.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a * `