Skip to content

feat: support arbitrary input/output formats (experimental, HTTP only)#1928

Open
kavirajk wants to merge 12 commits into
mainfrom
kavirajk/support-arbitrary-format
Open

feat: support arbitrary input/output formats (experimental, HTTP only)#1928
kavirajk wants to merge 12 commits into
mainfrom
kavirajk/support-arbitrary-format

Conversation

@kavirajk

Copy link
Copy Markdown
Contributor

Summary

Changes:

Two new methods on driver.Conn, marked experimental:

// stream query results in any ClickHouse format
QueryFormat(ctx, "Parquet", "SELECT * FROM events") (io.ReadCloser, error)

// insert pre-formatted data from any io.Reader
InsertFormat(ctx, "CSV", "INSERT INTO events", file) error

Today the driver forces everything through the Native format: it overrides default_format on HTTP and rewrites any FORMAT clause in INSERT statements. That means you can't ask ClickHouse for CSV, JSONEachRow, Parquet, etc., even though the server over HTTP happily produces and parses all ~70 formats by itself.

With this PR the driver just gets out of the way: the server does all encoding/decoding, and the client streams raw bytes. Query results come back as an io.ReadCloser you read the formatted bytes from.
inserts take an io.Reader (a file, an HTTP body, a strings.Reader) as payload. No new dependencies.

The immediate use case is Parquet — exporting query results as Parquet files and ingesting Parquet files directly, without going through Rows/Batch.

Why HTTP only?

Over the native TCP protocol the server always speaks Native blocks, so any other format would have to be encoded/decoded client-side (this is what clickhouse-client does internally).

I prototyped that with pluggable codecs (as separate package) with CSV/JSONEachRow/Parquet implementations — but it means shipping and forever maintaining format encoders in Go, plus heavy dependencies (arrow-go pulls in grpc, thrift, flatbuffers).

We deliberately dropped it from this release; the direction we want to explore instead is server-side formatting over the native protocol.

Over native TCP, both methods return a clear sentinel error (ErrFormatNativeUnsupported) pointing users at HTTP.

Checklist

Delete items not relevant to your PR:

kavirajk added 11 commits July 6, 2026 11:58
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Client-side codecs translating between native blocks and named ClickHouse
formats, for use over the native TCP protocol where the server always
exchanges Native blocks.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
…ization

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
HTTP: pure passthrough - the server encodes/parses the requested format;
the response streams through an exception-scanning reader.
Native TCP: client-side codecs from Options.FormatCodecs convert between
Native blocks and the requested format, mirroring clickhouse-client.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
A shared block-to-record converter backs both codecs; any Arrow-adjacent
format can be added as a thin codec on top. Parquet decoding buffers the
payload (footer-at-end format constraint); ArrowStream is fully streaming.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Covers round-trips in all four built-in formats on both protocols,
FORMAT-clause replacement, unregistered-format errors, multi-block and
malformed inserts, mid-stream HTTP exceptions, cancellation, gzip, and
cross-protocol Parquet interop.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Also replace the single example with three complete ones: CSV ingest,
JSONEachRow streaming decode, and Parquet file export/import.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
…lease

- Both APIs are marked Experimental and return ErrFormatNativeUnsupported
  over the native protocol, where the server only exchanges Native blocks.
- Remove the client-side codec layer (lib/format, Options.FormatCodecs) and
  the arrow-go dependency tree; server-side formatting over the native
  protocol is the direction under discussion instead.
- Validate format names before interpolating them into the query text.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@kavirajk
kavirajk marked this pull request as ready for review July 20, 2026 13:43
@kavirajk
kavirajk requested a review from chernser as a code owner July 20, 2026 13:43
Comment thread conn_http_format.go
// parse a mid-stream exception once its marker is seen.
const exceptionScanLimit = 32 << 10

var exceptionMarker = []byte("__exception__")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Should fix — marker scan truncates valid streams containing exception

queryFormat wraps every response in exceptionScanReader, which scans the whole (successful) stream for the literal bytes __exception__. A valid result that legitimately contains that sequence is silently cut off at the match and the remainder is parsed as a server exception — so valid data is truncated and a spurious error is returned on a query that actually succeeded.

  • Not just binary: a String/Nullable(String) column holding the text __exception__ triggers it on CSV/JSONEachRow too; Parquet/ArrowStream payloads can carry the bytes anywhere.
  • The failure is silent (partial data + fabricated ClickHouse exception), and it hits the headline Parquet path.

The doc comment acknowledges this, but since it corrupts valid success-path data, please decide explicitly: e.g. default these calls to wait_end_of_query=1 (server buffers and reports errors via HTTP status, no in-band marker) unless the caller opts into streaming, or gate the marker scan so it cannot silently truncate. At minimum surface the limitation in the exported QueryFormat doc, not only here.

@github-actions

Copy link
Copy Markdown

🤖 Claude review

Adds experimental QueryFormat/InsertFormat on driver.Conn so callers can stream results in any ClickHouse format (CSV, JSONEachRow, Parquet, …) and insert pre-formatted io.Reader payloads over HTTP. Native TCP returns the ErrFormatNativeUnsupported sentinel. The code is clean, resource lifecycle is careful (release-exactly-once on every path, connection held until Close), and test coverage is strong: round-trips per format, mid-stream exception, cancellation, compression, large insert, format-name validation, and the native-unsupported sentinel.

My verdict is needs discussion, driven by one design limitation rather than a defect in the implementation.

Key concern:

  • The mid-stream exception detector scans every successful stream for the literal bytes __exception__. Any valid result carrying those bytes — a String column with that text, or arbitrary bytes inside a Parquet/ArrowStream payload — is silently truncated and turned into a spurious error (inline on conn_http_format.go). This is documented as a known limitation, but it affects the correctness of valid data on the success path, including the headline Parquet use case, so it warrants an explicit decision before shipping.

Minor:

  • Close drains the full remaining response body for connection reuse, so closing a large export early (without cancelling the context) blocks until the whole result downloads — worth a doc note pointing users at context cancellation to abort.
  • go.mod/go.sum bump go-spew and go-difflib to unrelated pseudo-versions; looks like go mod tidy churn unrelated to the feature.

Blind spots: could not run the integration tests against a live server; the marker-collision and compression+mid-stream-exception paths are not exercised by tests. Adding these two methods to the public driver.Conn interface is source-breaking for any external implementer/mock, which is acceptable here but worth a release note.

Verdict: 💬 Needs discussion

General findings

  • ⚠️ Should fix — Close drains the full body; document how to abort early
    httpFormatStream.Close calls discardAndClose(s.body), which drains the entire remaining response so the HTTP connection stays reusable. For a large export (a multi-GB Parquet stream), reading a prefix and then calling Close blocks until the whole result has been downloaded. That is a surprising footgun for a streaming API. The TestFormatCancellation test shows the intended escape hatch — cancel the context, then Close returns fast — so document on the exported QueryFormat that early abort must go through context cancellation, not just Close.
  • 💡 Nit — unrelated go.mod/go.sum dependency bumps
    go.mod/go.sum change github.com/davecgh/go-spew and github.com/pmezard/go-difflib to unrelated pseudo-versions with no connection to this feature — likely go mod tidy churn from a different toolchain. Consider reverting to keep the diff focused, or confirm the bump is intended.

Inline comments are attached to the relevant lines. This summary updates in place on re-review.

Comment thread lib/driver/driver.go
// HTTP protocol, where the server encodes the stream and every
// server-supported format works; over the native protocol it returns
// clickhouse.ErrFormatNativeUnsupported.
QueryFormat(ctx context.Context, format string, query string, args ...any) (io.ReadCloser, error)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: feels like some glue word missing like QueryAsFormat, InsertAsFormat.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My initial idea was QueryWithFormat and InsertWithFormat. Later decided to keep it simple. Goes well with other apis

Query() and QueryFormat()
Insert() and InsertFormat()

Comment thread conn_http_format.go
// their tail is the beginning of an exception marker split across reads.
func (r *exceptionScanReader) safeLen() int {
if r.srcDone || r.err != nil {
return len(r.pending)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is the branch for done or error. Then how would length be used in this case ?Should it return 0 ?

Comment thread conn_http_format.go
switch h.compression {
case CompressionGZIP, CompressionDeflate, CompressionBrotli:
headers["Accept-Encoding"] = h.compression.String()
case CompressionZSTD, CompressionLZ4:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lz4 and zstd can be used in http compression.
in java we have dedicated flag http_compression to understand what compression should be used.

Comment thread conn_http_format.go
return nil, err
}

rw := h.compressionPool.Get()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

how do you detect compression?
would it work with uncompressed response?

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
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