feat(storage): implement parquet, register clickhouse, checksum results - #15
Merged
Merged
Conversation
ekalinin
force-pushed
the
feat/storage-formats
branch
from
August 17, 2026 19:26
a015c1e to
5dfb02e
Compare
ekalinin
force-pushed
the
feat/transport-auth
branch
from
August 17, 2026 19:26
102c3a3 to
3bedb44
Compare
result_format: parquet was serialized as JSONL, so clients received a .parquet file with JSON lines inside and `format: "parquet"` in the metadata. It now produces a real Parquet file: the head of the stream is sampled to infer a type per column, everything past the bounded sample is streamed, SQL NULL round-trips through optional fields, and a round-trip test opens the result with a parquet reader. Two properties are documented in the encoder: columns come out in alphabetical order because parquet-go sorts schema group fields, and duplicate SQL column names get a numeric suffix. The ClickHouse ResultStore was written but never registered in the executable, so storage_backend: clickhouse failed with "unknown storage backend" only after the SQL had already run. It is now built from storage.clickhouse, registered, and closed on shutdown. A default_storage that cannot be built is rejected at config load, not after the first query. The FS store is only created when the configuration asks for it: doing it unconditionally meant MkdirAll on every start, which fails under a read-only root filesystem even when results go to S3. ResultRef.Checksum was always empty. It is now a sha256 computed from the same bytes on their way to storage, so it costs one pass and never needs the result read back.
ekalinin
force-pushed
the
feat/transport-auth
branch
from
August 18, 2026 09:47
3bedb44 to
cb77494
Compare
ekalinin
force-pushed
the
feat/storage-formats
branch
from
August 18, 2026 09:47
5dfb02e to
0c7432b
Compare
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
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.
Parquet
result_format: parquetwas serialized as JSONL: clients received a.parquetfile with JSON lines inside andformat: "parquet"in the metadata. Nothing could open it.A real encoder on
parquet-go:RowStreamonly reports column names;parquet-godefaultsMaxRowsPerRowGrouptomath.MaxInt64and buffers pages on the heap, so without the flush nothing reached the output writer untilCloseand the whole result - a size the client picks with its own SQL - sat in RSS;BIGINT UNSIGNEDaboveMaxInt64does not come back negative;Close, which writes the footer, means an unreadable file and fails the query.Two properties are documented in the code: columns come out in alphabetical order because
parquet-gosorts schema group fields by name (readers address columns by name, so this is a layout detail rather than a data one), and duplicate column names, which SQL allows, get a numeric suffix. The suffix is checked against the real column names before it is handed out -SELECT 1 AS a, 2 AS a, 3 AS a_1otherwise produced two fields calleda_1, and since aparquet.Groupis a map the schema came out one field short of the row being written, which panicked with an out-of-range column index and took the process down with every query on it.Tests open the result with a parquet reader and check the values, including NULL, a timestamp, the type fallback, an unsigned value, the colliding-names case and the sampling boundary, and assert the file is written as several row groups rather than one.
ClickHouse
The backend was written but never registered, so
storage_backend: clickhousefailed withunknown storage backendonly after the SQL had already run. It is now built fromstorage.clickhouse, registered, and closed on shutdown.That combination also made
parqueton the ClickHouse backend reachable for the first time, and it cannot work: the store splits the stream into lines and joins them back with"\n", which is exact for JSONL and CSV and destroys a binary format - the footerPAR1came back asAR1\n. A backend now declares what it can hold losslessly through an optionalFormatChecker, and the pair is refused at submission time rather than producing a file whose own checksum no longer matches.A
default_storagethat cannot be built is now rejected at config load rather than after the first query, and it is captured byQueryManagerat construction: the registry is built once inmainand never grows, so honouring a reloaded value would point every new query at a backend that was never built. A backend the configuration does ask for is fatal when it cannot be built, for all three alike - starting with a warning left the process answering 400 for that backend for the rest of its life, long after the dependency came back.The FS store is only created when the configuration asks for it: doing it unconditionally meant
MkdirAllon every start, which fails under a read-only root filesystem even when results go to S3.Checksums
ResultRef.Checksumwas always empty, so a download could not be verified against what was written. It is now a sha256 computed from the same bytes on their way to storage - one pass, and the result is never read back.Verification
go test -race ./...,golangci-lint run ./...- clean. Tests compare the checksum against one recomputed from the stored file and assert that aparquetresult really starts withPAR1.