sqlflow serve: named SQL over HTTP - #278
Merged
Merged
Conversation
A pipeline ends at a sink and the rows need an API. This designs a serve command: a separate config with commands and datasets, bearer tokens as identities, typed params bound through prepared statements, a row cap and a timeout, and a reserved rate-limit policy that is refused until enforced. Verified on DuckDB 1.5.2 that the postgres extension pushes filters and projections down but not GROUP BY, and that a filter against a Postgres view pushes into the view. The example config is shaped by that.
The spec left binding and cancellation to the plan and asserted that $from fails to parse. A probe against DuckDB 1.5.2 and arrow-adbc 1.6.0 settled all three, and two of the answers were not the ones the spec planned for: - DuckDB binds by position. The parameter schema names fields 0 and 1, and a bound record's field names are ignored. The server now rewrites $name to $N itself and checks DuckDB's parameter count against its own. - The Go driver manager ignores the context and exposes no cancel. A timeout returns 504 while the query runs on and holds the lock. - $from prepares fine. The CLI's EXECUTE call syntax rejected it. The reserved-word rule is gone. Also from the probe: SetSqlQuery binds eagerly, so a missing view fails at startup; Arrow renders HUGEINT through a float, so decimals encode as exact strings; results stream, so the row cap stops the query. Serve examples move out of dev/config/examples, where two sweeps decode every file as a pipeline. --metrics is dropped until serve records an instrument.
DuckDB binds ADBC parameters by position. Its parameter schema names them 0, 1 with type null, and a bound record's field names are ignored, so a statement using $since before $until binds whatever arrives first to $since. A Go probe against DuckDB 1.5.2 and arrow-adbc 1.6.0 showed it. sqlparams.Rewrite numbers each $name in order of first appearance, skipping strings, quoted identifiers, comments and dollar quotes, and rejects a positional $1, which DuckDB refuses to mix with names. user.config.serve_reserved refuses a rate_limit this version parses and does not enforce. user.config.serve_dataset names a broken dataset rule. Both exit 10, and codes.golden records them. cli.serve joins features.yml, requiring unit and release coverage. If the scanner misnumbers a placeholder, a request binds a value to the wrong column and answers 200 with wrong rows.
ServeConf is its own file type: commands plus a serve block. It decodes strictly through the same decodeStrict LoadRendered now uses, so a typo in a serve file is an error rather than a dropped setting. Check returns every rule violation with its YAML path: unique names, sql xor grains, every $name declared and every param used by every statement, unique non-empty tokens, exact lowercase origins, and a non-zero rate_limit refused. Paths are copied with slices.Concat. Built with append, two faults in one param shared a backing array and the second overwrote the first one's path; the test for that fails on the append version. If a grain ignoring a param were accepted, a request filtering on that param would get unfiltered rows.
internal/schema generates serve.json from ServeConf beside config.json, and a golden test holds it current. validate checks a file with a top-level serve key and no pipeline key against serve.json, then runs the serve rules, each at its line. A file with both keys is a pipeline exactly as before, so serve reports as unknown. A rule the schema already reported at the same key is not repeated. An empty token rendered from an unset variable is demoted to a warning like any other unsupplied value, so CI without secrets passes. config example --serve renders the serve skeleton. The renderer now prints a map's value keys under a <name> placeholder; without that, grains: printed empty. config_example.golden is unchanged. dev/config/serve holds a local-table example and the bluesky Postgres example. They live outside dev/config/examples because two sweeps decode every file there as a pipeline. If validate chose the pipeline schema for a serve file, every serve file would fail with pipeline is required.
readRows encodes each value while its record is current and stops at the first row past max_rows, which stops DuckDB's stream. A zoned timestamp renders in UTC whatever the session zone, a decimal as its exact digits, and NaN or infinity as strings. Arrow's own rendering printed a New York session's midnight UTC as 20:00-04:00 and a HUGEINT as 1.7e+38; the tests fail when those cases are removed. prepare numbers a statement, binds it once at startup so a missing table fails there, and compares DuckDB's parameter count to the scanner's. An E'' string with an escaped quote makes them differ, and the test shows prepare refusing it. Each request plans a fresh statement and binds a typed record, so an absent param is a typed null. The Go driver manager cannot cancel a query. executor bounds the caller's wait: at the deadline run returns, and the query keeps the lock until it finishes. The timeout test uses a 1 s cross join, because range(200000000) took 320 ms here and would race a 100 ms deadline on a larger runner. If a query goroutine wrote the response, a timed-out request would race the handler's own 504.
…shape
New checks the rules and prepares every statement, so a dataset that
cannot answer stops the process at start. Handler serves /healthz,
/v1/datasets and /v1/datasets/{name}, all GET, with CORS, the method
check and auth ahead of anything that waits on the connection's lock.
A token is compared with subtle.ConstantTimeCompare against every
configured token, with no early return. Params parse against their
declared type in sorted key order; an unknown name is 400 rather than
a silent NULL. Every refusal is {"error": {"code", "message"}}, and
the tests assert each code's status and that its message names the
cause. One log line per request carries the token's name, never its
value.
Serve drains in-flight requests for 5 s on shutdown. The drain test
cancels during a 1 s query and gets its 200.
If identify compared with ==, response time would leak a token a byte
at a time.
serve loads and checks the config before opening DuckDB, runs its commands through core.InitCommands, prepares every statement, listens on http.addr, and serves until SIGINT or SIGTERM. The server closes before the connection, so a query still holding the lock finishes first. A failed ATTACH stays uncoded and exits 1, as in run, so a supervisor retries a database that is not up yet. The tests serve a config on port 0 and answer /healthz and a dataset, refuse a rate_limit with exit 10, and refuse an empty token that validate only warns about. Built and run by hand against dev/config/serve/local.table.yml: health, a grain with a param, a dataset without grains, a 401 without a token, and a clean exit on SIGTERM. If serve demoted an empty token as validate does, it would start and refuse every request.
Every file in dev/config/serve validates with no variables set, loads with the variables a deploy supplies, and builds a real server when its commands can run offline. local.table.yml builds; bluesky.postgres.yml skips at INSTALL postgres under enable_external_access=false, as the Postgres pipeline examples do. The leak test sends 500 requests of 1000 rows through the handler and fails above 8 MiB of resident growth. It grows about 1 MiB as written. Retaining each record batch in readRows fails it. Retaining the reader does not, because an exhausted stream holds almost nothing. If the encoder kept a record alive past its reader, every response would pin DuckDB's buffers for the life of the process.
The README documents the config, the routes, the response and error shapes, and the five things to know before deploying: attach READ_ONLY, aggregate in a backend view because DuckDB does not push GROUP BY, bound pg_connection_limit, a timeout leaves the query running, and one connection serves every request. The release test starts serve from the image against dev/config/serve/local.table.yml and checks /healthz, the listing, a grain with a param, and a 401. That is the proof the command exists in the artifact users pull, which no unit test gives. If the command were not registered in the image's binary, every serve deploy would fail at start with an unknown command.
Five departures, each found while building it: origins must be
lowercase because the comparison is exact; a 401 carries
WWW-Authenticate and the Bearer scheme is case-insensitive; every
response says no-store, and a 503 health body is {"status":"unavailable"};
the log line carries path, and a caller that hangs up logs 499; and the
timeout test uses a cross join, because range(200000000) finished in
320 ms and would race a 100 ms deadline on a larger runner.
If the spec kept the old test query, a reader re-deriving the test from
it would write a flaky one.
make coverage-matrix ran every suite against an image built from this branch: go test -short -race, the integration pass, and all 18 release tests. No test failed. status/features.yml gains one line, cli.serve covered at unit and release, and matrix.md is regenerated from it. If the release test did not run, cli.serve would report release: missing and the merge gate would fail.
query_failed returned DuckDB's error text, and DuckDB's Postgres extension puts the whole connection string in a connection error. A probe proxied serve's Postgres connection, killed the proxy mid-run, and got back, with the demo token anyone can read from the page: Unable to connect to Postgres at "postgresql://postgres:postgres@..." The body now names the dataset and grain only. The log keeps DuckDB's error, passed through Redact, which replaces the password in a URI's userinfo or a libpq password= keyword with ***. A failed ATTACH at start is redacted too, since it goes to the same log. The new test sends a cast error that echoes a URI with a password. It fails against the old body, and the re-run probe against the rebuilt binary returns 'dataset probe failed; the server log has the database's error' with the logged URI reading postgres:***@. run logs a failed ATTACH unredacted today. That is a follow-up. If this regressed, one Postgres outage would publish the database password to every visitor of the demo page.
#275 squash-merged this branch's first two commits, so both sides added docs/superpowers/specs/2026-09-12-serve-design.md. main's copy is byte-identical to ac97dbc. This branch's copy is that plus 40ec437 and 00432fe, which record what implementation changed and the 500 body that no longer carries DuckDB's error, so the conflict resolves to this branch's copy. #277's README, example and manager changes merged cleanly. The cli, config, schema, validate, managers and serve packages pass under -race after the merge.
…ates CHANGELOG.md: main now keeps an Unreleased section. serve's entries join its Added list, and serve's limits sit under Known limits, rather than under a v1.3.0 heading of their own. The version is named when it is tagged. internal/errs/errs_test.go: both sides appended a test at the end of the file. Both are kept. internal/validate/validate.go merged without a conflict and with a bug: #273's checkDrainDeadline ran on every file, so a serve file reported pipeline.drain_deadline: pass for a pipeline it does not have. A check that could not apply must not read as a pass. Each file now gets its own checks: serve rules for a serve file, the drain deadline for a pipeline. TestValidateServe_AValidServeFilePasses now asserts no pipeline check reports on a serve file; it failed on the merged code and passes now. docs/coverage/matrix.md merged as text into a page that no longer matched its status files, and test_the_committed_page_is_current failed. make coverage-page regenerated it: 39 features, cli.serve covered at unit and release, 0 gaps. go vet ./..., go test -short -race ./... and pytest tests/tooling pass on the merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
sqlflow serve: named SQL over HTTP, from the same config machineryrunuses. A serve config declares datasets, each a fixed statement with typed parameters, or one statement per grain.serveattaches the data throughcommands:, prepares every statement at startup, and answers each request by binding its parameters. Nothing in a request becomes SQL text.Spec:
docs/superpowers/specs/2026-09-12-serve-design.md. This branch carries the spec commits from #275, so this PR supersedes it.How it is built
internal/sqlparamsnumbers$nameplaceholders. DuckDB binds ADBC parameters by position and ignores field names, so the server numbers them itself and compares its count with DuckDB's at startup. AnE'…'string with an escaped quote fools the scanner, and the test shows the count check refusing to start.internal/config/serve.goholds the types, a strict loader, and the rules the schema cannot state.serve.jsonis generated from the types besideconfig.json, andvalidatepicks the schema by top-level key. A pipeline file validates exactly as before.internal/serveencodes rows, runs statements, and serves the routes. Each request plans a fresh statement, so a table that changes under the process is read as it is now.internal/cli/serveis the command.Behaviour worth reviewing
504, and the query keeps the connection's lock until it finishes. The README says so.20:00-04:00and aHUGEINTas1.7e+38. The encoder renders a zoned timestamp in UTC and a decimal as exact digits. The tests fail with either case removed.subtle.ConstantTimeCompareagainst every token. The log carries a token's name, never its value.rate_limitis parsed and refused withuser.config.serve_reserved, so a config written now keeps its shape.Evidence
go test -short -race ./...passes.uv run --locked pytest tests/tooling -qpasses, andmake coverage-checkis clean:cli.servecovered at unit and release.test_cli_serve_answers_requests_from_the_image.TestCliServe_DoesNotLeakNativeMemorygrows about 1 MiB over 500k rows. Retaining each record batch in the encoder fails it.local.table.yml: health, a grain with a param, a dataset without grains, a 401, a clean exit on SIGTERM.sqlflow validate dev/config/serve/bluesky.postgres.ymlpasses with warnings and no variables set.make soak(10 min,run, as a regression check on the shared loader): PASS, flat over 105,286,753 messages at +0.022 B/msg against a 1.0 threshold.servehas no sustained-load soak yet; its memory gate is the leak test.Not in this PR
Rate limiting, caching, a connection pool, per-token allowlists, pagination, TLS, and
--metrics. Each is a follow-up in the spec.The first consumer is turbolytics/sql-flow-bluesky-demo#2, which waits on a v1.3.0 release.