Skip to content

feat(storage): implement parquet, register clickhouse, checksum results - #15

Merged
ekalinin merged 1 commit into
feat/transport-authfrom
feat/storage-formats
Aug 20, 2026
Merged

feat(storage): implement parquet, register clickhouse, checksum results#15
ekalinin merged 1 commit into
feat/transport-authfrom
feat/storage-formats

Conversation

@ekalinin

@ekalinin ekalinin commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Stacked on #14.

Parquet

result_format: parquet was serialized as JSONL: clients received a .parquet file with JSON lines inside and format: "parquet" in the metadata. Nothing could open it.

A real encoder on parquet-go:

  • the head of the stream (128 rows) is sampled to infer a type per column, because Parquet needs its schema up front while a RowStream only reports column names;
  • everything past the sample is streamed, and a row group is flushed every 50k rows, so memory stays bounded and the write overlaps reading from the database. parquet-go defaults MaxRowsPerRowGroup to math.MaxInt64 and buffers pages on the heap, so without the flush nothing reached the output writer until Close and the whole result - a size the client picks with its own SQL - sat in RSS;
  • a column whose sampled values disagree falls back to text, which can represent all of them;
  • a value that does not fit the type its column was given fails the query instead of being written as NULL: a file that opens, looks valid and has values missing is the worst outcome for a proxy that materializes a result exactly once (I4);
  • unsigned integers get the unsigned logical type, so a MySQL BIGINT UNSIGNED above MaxInt64 does not come back negative;
  • SQL NULL round-trips through optional fields;
  • an error from 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-go sorts 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_1 otherwise produced two fields called a_1, and since a parquet.Group is 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: 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.

That combination also made parquet on 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 footer PAR1 came back as AR1\n. A backend now declares what it can hold losslessly through an optional FormatChecker, and the pair is refused at submission time rather than producing a file whose own checksum no longer matches.

A default_storage that cannot be built is now rejected at config load rather than after the first query, and it is captured by QueryManager at construction: the registry is built once in main and 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 MkdirAll on every start, which fails under a read-only root filesystem even when results go to S3.

Checksums

ResultRef.Checksum was 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 a parquet result really starts with PAR1.

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
ekalinin force-pushed the feat/transport-auth branch from 3bedb44 to cb77494 Compare August 18, 2026 09:47
@ekalinin
ekalinin force-pushed the feat/storage-formats branch from 5dfb02e to 0c7432b Compare August 18, 2026 09:47
@ekalinin
ekalinin merged commit f81707c into feat/transport-auth Aug 20, 2026
2 checks passed
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.

1 participant