feat(ingest): add hotdata ingest raw-sql verb - #241
Conversation
Front-door for POST /ingest/queries/raw (dlthubworker#160): run a source-native
SQL query verbatim against a SQL datasource — joins, GROUP BY, CTEs, window
functions, engine-specific functions execute at the source — and ingest the
result into --table in a fresh managed database. Complements `new-import` (the
restricted projection grammar).
hotdata ingest raw-sql --source <name|id> --table <result> [--limit N] "<sql>"
- client: RawSqlIngest request + create_raw_query (POST /queries/raw); requires a
workspace API key like the other enqueue routes.
- command: `raw-sql` subcommand + handler, mirroring new-import's submit/drain/
poll flow. Skill doc updated.
The server enforces read-only single-statement + dialect-aware validation; the
endpoint is live in prod.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| Agent tips: | ||
| - Import SQL is a **restricted grammar**: `SELECT <cols|*> FROM <datasource>[.<table>] [WHERE …] [LIMIT n]` — no joins/GROUP BY. Run real analytics with `hotdata query` against the imported database afterward. | ||
| - `new-import` SQL is a **restricted grammar**: `SELECT <cols|*> FROM <datasource>[.<table>] [WHERE …] [LIMIT n]` — no joins/GROUP BY. For those, either use `raw-sql` (pushes the full query down to a SQL source) or run `hotdata query` against the imported database afterward. | ||
| - `raw-sql` is SQL-datasources-only and read-only (a single SELECT; writes/DDL are refused). Prefer a **read-only source credential**. Cast ambiguous/mixed-precision decimal columns explicitly in the query. |
There was a problem hiding this comment.
nit: (not blocking) create_raw_query calls require_api_key(), so raw-sql is an enqueue verb — but the auth paragraph at skills/hotdata/SKILL.md:238 still lists only new-datasource, new-import, trigger-import, delete-datasource as needing a workspace API key. An agent reading that line will assume a login session works for raw-sql and hit NeedsApiKey. Worth adding raw-sql to that list.
Also, --limit isn't mentioned anywhere in the skill doc — the example only shows --source/--table.
| /// Ingest the result of a source-native SQL query (SQL datasources only). | ||
| /// The SQL runs verbatim in the source dialect; the server enforces a single | ||
| /// read-only statement. | ||
| pub fn create_raw_query(&self, req: &RawSqlIngest) -> Result<IngestAck, IngestError> { |
There was a problem hiding this comment.
nit: (not blocking) No test for this endpoint. The module's mockito suite pins the wire contract for the other enqueue routes (create_source_sends_api_key_bearer_and_workspace_header, enqueue_with_session_jwt_fails_fast_without_http); create_raw_query adds a new path (/ingest/queries/raw), a new body shape, and the skip_serializing_if on limit — all currently unverified. A single mock test asserting the path + Matcher::Json body with and without --limit, plus a NeedsApiKey case in the existing JWT test, would cover it.
super nit: (not blocking) the Surface: list in the module docs (lines 25–30) enumerates every method; create_raw_query isn't in it.
| let spinner = util::spinner("submitting raw-sql import…"); | ||
| let ack = client.create_raw_query(&req).unwrap_or_else(|e| { | ||
| spinner.finish_and_clear(); | ||
| e.exit() | ||
| }); |
There was a problem hiding this comment.
nit: (not blocking) new_import retries the enqueue once on a 404 (src/commands/ingest.rs:939-949) because the by-name datasource lookup reads snapshots that lag writes, so a submit right after new-datasource 404s briefly. raw-sql resolves --source by name the same way, so it has the same window but no retry — new-datasource … && ingest raw-sql --source <name> can fail where new-import would succeed. Reusing the same retry-once loop here would make the two front doors behave alike.
super nit: (not blocking) the tail of this fn (spinner → --wait poll → render_done → else drain + render_ack) is a verbatim copy of new_import's. A shared submit_and_track(&client, ack, output, poll) helper would keep the two from drifting. Also, the module header's command list (lines 5-11) doesn't mention raw-sql.
|
approved |
2 participants
CLI front-door for
POST /ingest/queries/raw(companion to dlthubworker#160, now merged + deployed to prod).Runs a source-native SQL query verbatim against a SQL datasource in its own dialect — joins,
GROUP BY, CTEs, window functions, engine-specific functions all execute at the source; only the result set transfers, landing in--tablein a fresh managed database. Complementsnew-import(the restrictedSELECT <cols|*> FROM <ds>[.<table>] [WHERE] [LIMIT]grammar).src/client/ingest.rs):RawSqlIngestrequest +create_raw_query(POST /queries/raw). Requires a workspace API key like the other enqueue routes.src/commands/ingest.rs):raw-sqlsubcommand + handler, mirroringnew-import's submit → drain → poll flow (--waitsupported).skills/hotdata/SKILL.md) updated with the verb + when to use it vsnew-import.Server-side safety (in dlthubworker#160): single read-only statement enforced, validated in the source dialect, decimals typed from
cursor.description. Verified live in prod: aGROUP BYonneon_tpchdrained todonewith the correct pushed-down aggregation.cargo check+cargo fmtclean; no new clippy warnings.