Skip to content

[absorbed into #630 Phase 7] migrate query execution and export, then delete generic client mechanics #637

Description

@BorisTyshkevich

Part of #630.

Depends on: #636

Goal

Move the two application consumers that still depend on SQL Browser's legacy generic client facade—QueryExecutionService and ExportService—onto the authenticated/package boundary, then delete the superseded runQuery / exportQuery / ordinary killQuery transport mechanics from src/net/ch-client.ts.

This unit must leave application policy in the application layer while removing generic HTTP/stream/error ownership from SQL Browser.

Application-facing authenticated interface

Give application services a narrow SQL Browser-owned interface over #636 rather than handing them ChCtx plus free functions from ch-client.ts.

Use an interface equivalent to:

export interface AuthenticatedClickHouse {
  request(request: Omit<ClickHouseHttpRequest, 'authorization'>): Promise<Response>;
  queryJson<T>(request: Omit<ClickHouseHttpRequest, 'authorization'>): Promise<T>;
  queryText(request: Omit<ClickHouseHttpRequest, 'authorization'>): Promise<string>;
  queryProgress(
    request: Omit<ClickHouseHttpRequest, 'authorization'>,
    callbacks?: ProgressStreamCallbacks,
  ): Promise<Response>;
  killQuery(queryId: string, options?: { signal?: AbortSignal }): Promise<void>;
}

The production binding must read the live current ChCtx per operation (for example through a ctx: () => ChCtx provider) so token refresh/origin/auth-mode replacement is never pinned at service construction time.

This interface is SQL Browser composition, not a package API: it owns credential acquisition through #636 while the package stays credential-opaque.

QueryExecutionService migration

Remove runQuery, killQuery, and direct ChCtx dependencies from QueryExecutionDeps. Depend on the narrow authenticated interface/provider instead.

Move SQL Browser execution-mode policy into the service

The policy currently embedded in ch-client.ts::runQuery belongs with query execution, not the generic package. Preserve it exactly:

  • logical Table => JSONStringsEachRowWithProgress;
  • logical KPI => JSONEachRowWithProgress;
  • logical TSV => TabSeparatedWithNamesAndTypes;
  • other explicit/raw format names pass through unchanged;
  • streaming is the current Table/KPI set only;
  • raw modes keep wait_end_of_query = 1 because they are consumed whole;
  • streaming modes do not add wait_end_of_query;
  • add_http_cors_header = 1 remains where currently emitted;
  • resultRowLimit > 0 maps to max_result_rows plus result_overflow_mode = 'break' exactly as today;
  • caller params remain merged with query_id/session/native param_* vocabulary in the same precedence/order semantics as current behavior;
  • no authored FORMAT is appended or rewritten by this policy.

A small pure request-resolver helper is encouraged if it makes the mode mapping/cap/settings policy independently testable, but do not move these SQL Browser logical modes into @altinity/clickhouse-http.

Preserve runQuery observable behavior at the service boundary

Current behavior distinguishes:

  • non-2xx ClickHouse response => application { error: <parsed message> } outcome;
  • successful raw response => { raw };
  • successful streaming response => callbacks + streamed completion;
  • AbortError => cancellation path;
  • network TypeError => network/transient classification in the service;
  • other thrown failure => application error string.

When package consuming APIs throw ClickHouseError, translate it at the application boundary to the same result/error shape current callers expect. Do not let a new typed package error accidentally enter the existing retry branch as a network failure.

Preserve retry policy

QueryExecutionService continues to own:

  • fresh query_id per attempt;
  • SESSION_IS_LOCKED retry;
  • one retry for transient read-only network failure;
  • no automatic retry for uncertain non-idempotent statements;
  • stop-on-first-script-failure;
  • the existing warning for non-idempotent uncertain network outcome;
  • caller-owned AbortController and isCurrent fences.

The package and authenticated net layer must not acquire these policies.

Server cancellation

QueryExecutionService.kill() remains stateless/best-effort at this layer:

  • call authenticated killQuery(queryId);
  • swallow failure exactly as current service behavior requires;
  • do not introduce a query registry;
  • the owning workbench/dashboard/session still decides when to abort locally and when to call kill().

ExportService migration

Remove exportQuery, runQuery, killQuery, and direct ChCtx dependencies from ExportServiceDeps. Use the same authenticated interface/provider plus package protocol helpers.

Raw streaming export

For a row-returning export:

  1. prepare SQL/format using existing SQL Browser prepareExportSql policy;
  2. obtain a native Response through authenticated request() with the caller's signal/query ID/session/native params;
  3. classify non-2xx with the package's success/error helper from [absorbed into #630 Phase 4] add consuming query APIs, ClickHouse errors, and stateless KILL QUERY #634 while leaving a successful 2xx body untouched;
  4. stream response.body directly to the existing file sink;
  5. inspect the retained byte tail with package findExceptionFrame(tailBytes, response.headers.get('X-ClickHouse-Exception-Tag'));
  6. preserve existing .partial / rename/delete / user-facing export semantics.

Successful raw export must never use response.text().

Script export

Preserve current distinction:

  • row-returning statements stream raw output to files;
  • non-row-returning statements execute for effect with the existing result/error policy;
  • one query ID per in-flight statement;
  • stop/error/cancellation behavior remains unchanged;
  • schema-mutating follow-up behavior remains SQL Browser policy.

Cancellation ownership

Keep ExportService's independent controllers/query IDs/waves separate from workbench execution. Package and authenticated client are stateless with respect to operation ownership.

Use package killQuery through the authenticated interface for ordinary best-effort cancel. Authenticated execution-scope teardown continues to use the frozen-lease path from #636, not this mutable live-auth interface.

Delete superseded generic mechanics

After all production callers are migrated in this unit:

  • delete runQuery implementation/export from src/net/ch-client.ts;
  • delete exportQuery implementation/export;
  • delete ordinary mutable-context killQuery implementation/export if no product caller remains; use the authenticated interface instead;
  • delete temporary ClickHouseTransport.streamLines compatibility plumbing if no caller needs the old transport facade;
  • delete the old ClickHouseTransport seam/files entirely when all callers now use @altinity/clickhouse-http and [absorbed into #630 Phase 6] simplify SQL Browser authentication integration around the package #636 directly; do not preserve an adapter with zero architectural purpose;
  • keep killQueryWithLease or its [absorbed into #630 Phase 6] simplify SQL Browser authentication integration around the package #636 successor because frozen-lease cancellation is SQL Browser auth/lifecycle policy, but its wire request must be package-owned;
  • keep queryJson only if product-specific ch-client.ts operations still use it as a thin authenticated helper; it must contain no generic request/error implementation of its own.

Before deletion, search the full repository and migrate every real caller. Do not leave dead compatibility exports solely for old tests; rewrite fixtures/tests to the final architecture.

Tests

QueryExecutionService

Preserve/add coverage for:

  • Table/KPI/TSV/arbitrary-format mapping;
  • authored FORMAT passed unchanged;
  • row-limit settings and no-cap cases;
  • query/session/native params preserved;
  • package ClickHouseError converted to current { error } outcome;
  • network TypeError retains transient classification;
  • AbortError retains cancellation classification;
  • SESSION_IS_LOCKED retry;
  • read-only uncertain-network retry once;
  • non-idempotent uncertain-network no-retry warning;
  • fresh query ID for retry;
  • stop on first script failure;
  • stale isCurrent fences;
  • best-effort remote kill does not throw.

ExportService

Preserve/add coverage for:

  • raw response body streams as bytes;
  • invalid UTF-8 export bytes preserved;
  • non-2xx parsed before file streaming;
  • tagged late exception trims only the exception frame and keeps clean bytes;
  • legacy late exception behavior;
  • marker-like clean data is not falsely trimmed;
  • .partial behavior and final file naming remain unchanged;
  • cancel after headers aborts body streaming through the original caller signal;
  • export cancel does not cancel workbench query and vice versa;
  • script export row/non-row paths;
  • query IDs/remote kill;
  • stale execution-scope/wave completion cannot mutate replacement state.

Integration/browser proof

Because this unit changes browser streaming/export and application cancellation wiring, run full e2e plus the targeted Fetch cancellation suite. Include a real-browser export stream that receives headers/data, stalls, then is cancelled.

npm run check:types
npm run check:arch
npm run check:schemas
npm run check:examples
npm test
npm run build
npm run test:e2e
npm run test:client-spike:browser

Use current script names if earlier units rename the client regression harness; Chromium and WebKit cancellation proof remains required.

Acceptance criteria

  • QueryExecutionService no longer imports runQuery, ordinary killQuery, or ChCtx from ch-client.ts.
  • SQL Browser logical format/cap/retry policy lives in QueryExecutionService/application code, not the package.
  • ClickHouseError is translated to the existing application error outcome rather than network/transient retry.
  • Existing idempotency and SESSION_IS_LOCKED retry semantics are unchanged.
  • ExportService no longer imports exportQuery, runQuery, ordinary killQuery, or ChCtx.
  • Successful exports stream native Response bytes directly without text decoding.
  • ExportService uses package late-exception framing on raw tail bytes.
  • Workbench and export cancellation remain independent and owner-scoped.
  • Post-header abort still cancels the real body through the caller signal.
  • Superseded generic runQuery / exportQuery / old transport mechanics are deleted after caller migration.
  • ch-client.ts is left as SQL Browser product operations plus thin authenticated helpers, not a second generic ClickHouse client.
  • Full gate, e2e, and targeted browser cancellation tests pass.

Non-goals

Agent execution notes

Before planning, read #630#636, src/application/query-execution-service.ts, src/application/export-service.ts, src/net/ch-client.ts, src/core/stream.ts, all related unit tests, Workbench/Export cancellation wiring, and the browser fault harness. Treat this as high-risk and likely Large if the plan confirms the service+export footprints cannot be safely completed by one coding agent; /ship's decomposition workflow should be used when the approved plan marks it Large.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorRestructuring without user-facing behavior changetech-debt

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions