Skip to content

feat(query): implement a few missing and useless options + fix query show#89

Open
redox wants to merge 1 commit into
mainfrom
su/fix-query
Open

feat(query): implement a few missing and useless options + fix query show#89
redox wants to merge 1 commit into
mainfrom
su/fix-query

Conversation

@redox

@redox redox commented Jul 25, 2026

Copy link
Copy Markdown
Member

Extend query with lakehouse QueryRequest / output options aligned to the OpenAPI spec, resulting in things like:

altertable query "select * from events LIMIT 10" --catalog whatever --schema main --format parquet > /tmp/events.parquet

New query flags

  • --compute-size (XS|S|M|L|XL|AUTO, default AUTO; omitted with --session-id unless an explicit size is set; reject AUTO + --session-id)
  • --dialect, --catalog, --schema (sent only when set)
  • --format csv|jsonl|parquet|markdowncsv/jsonl/parquet request API format and stream response bytes; markdown stays CLI-rendered from NDJSON
  • --output <path> — write result to a file (stdout by default)

Validation & output

  • Reject API-native formats with --json/--agent or presentation flags (--layout, --columns, --max-width, --pager)
  • Add executeLakehouseQueryBytes and writeQueryDestination for binary-safe streaming
  • Add OutputSink.writeBytes so parquet/csv/jsonl redirects are not corrupted by console.log (UTF-8 decode + trailing newline)

Also, fix query show so it always requires a query id and hits GET /query/:query_id.

query show / query cancel

  • Remove soleDirectOperands: ["show"], so bare altertable query show no longer runs SQL SHOW (which dumped ducklake metadata).
  • query show <query-id>GET /query/:query_id; query cancel <query-id> --session-id …DELETE /query/:query_id (unchanged).

… show`

Extend `query` with lakehouse `QueryRequest` / output options aligned to the OpenAPI spec.

```
altertable query "select * from events LIMIT 10" --catalog whatever --schema main --format parquet > /tmp/events.parquet
```

**New `query` flags**
- `--compute-size` (`XS|S|M|L|XL|AUTO`, default `AUTO`; omitted with `--session-id` unless an explicit size is set; reject `AUTO` + `--session-id`)
- `--dialect`, `--catalog`, `--schema` (sent only when set)
- `--format csv|jsonl|parquet|markdown` — `csv`/`jsonl`/`parquet` request API `format` and stream response bytes; `markdown` stays CLI-rendered from NDJSON
- `--output <path>` — write result to a file (stdout by default)

**Validation & output**
- Reject API-native formats with `--json`/`--agent` or presentation flags (`--layout`, `--columns`, `--max-width`, `--pager`)
- Add `executeLakehouseQueryBytes` and `writeQueryDestination` for binary-safe streaming
- Add `OutputSink.writeBytes` so parquet/csv/jsonl redirects are not corrupted by `console.log` (UTF-8 decode + trailing newline)

Also, fix `query show` so it always requires a query id and hits `GET /query/:query_id`.

**`query show` / `query cancel`**
- Remove `soleDirectOperands: ["show"]`, so bare `altertable query show` no longer runs SQL `SHOW` (which dumped ducklake metadata).
- `query show <query-id>` → `GET /query/:query_id`; `query cancel <query-id> --session-id …` → `DELETE /query/:query_id` (unchanged).

@albert20260301 albert20260301 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required

  1. Please reconcile the request contract with the tracked Lakehouse spec before merge. specs/lakehouse/SPEC.md lists the optional QueryRequest fields and does not define dialect or format. Those fields underpin the new flags and raw-response behavior, while the PR body says they are OpenAPI-aligned. Update the spec (or link the spec change) to define the request/response contract, or narrow this PR to the documented fields.

  2. writeQueryDestination currently consumes the whole ReadableStream with new Response(stream).arrayBuffer() before writing it. That is binary-safe, but it is not streaming and can exhaust memory for the Parquet/large-result use case this PR introduces. Please pipe chunks to the file/stdout sink instead, with a multi-chunk regression test.

@redox

redox commented Jul 25, 2026

Copy link
Copy Markdown
Member Author

Required

  1. Please reconcile the request contract with the tracked Lakehouse spec before merge. specs/lakehouse/SPEC.md lists the optional QueryRequest fields and does not define dialect or format. Those fields underpin the new flags and raw-response behavior, while the PR body says they are OpenAPI-aligned. Update the spec (or link the spec change) to define the request/response contract, or narrow this PR to the documented fields.

@albert20260301 good idea, go open a PR on https://github.com/altertable-ai/altertable-client-specs to reflect our updated https://api.altertable.ai/openapi/lakehouse.json

@albert20260301

Copy link
Copy Markdown
Contributor

@redox I opened altertable-ai/altertable-client-specs#39 from the current live Lakehouse OpenAPI contract, covering the dialect and format fields and removing stale visible.

The binary-output implementation still needs to write the response stream incrementally (with a multi-chunk regression test) before this CLI PR is ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants