Repository navigation
feat(typelayer)!: validate, coerce and filter through chtypes - #882
Draft
EricAndrechek wants to merge 1 commit into
Draft
EricAndrechek wants to merge 1 commit into
EricAndrechek wants to merge 1 commit into
Conversation
Ingest validation, type coercion, DEFAULT evaluation and row-level-security evaluation now run through chtypes (github.com/wave-rf/chtypes/go/v2), which runs ClickHouse's own parser and evaluator in-process. WaveHouse's Go-side canonicalizer, row evaluator and compact encoder are deleted. - Ingest: each body is judged by ClickHouse's parser for the tenant's ClickHouse line, on a per-tenant server handle that carries the server's time zone. A malformed record refuses the body with ClickHouse's own code, and the exported row is exactly what the worker inserts. - Column policy: a role's writable columns are the INSERT's column list and its column grant (insertable_columns), and an _eq check's value fills an omitted field through column_defaults. A column the role may not write keeps its own DEFAULT, so a generator DEFAULT draws a value (#816). - Stream row filters and insert checks evaluate as chtypes filters. - chtypes artifacts are fetched per ClickHouse minor line and platform into a cache on first use, under strict cache checks; CI caches them, and the images carry the chtypes CLI. - cgo is required; releases ship Linux amd64 and arm64 only; Go 1.27.3. - Built on the chtypes v2 dev SDK 2.0.0-dev.11; it moves to 2.0.0 when that is released. BREAKING: see CHANGELOG.md, Unreleased. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
|
Important Review skippedToo many files! This PR contains 191 files, which is 91 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters: 1
📒 Files selected for processing: 191
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
github-actions
Bot
added
documentation
📚 Docs preview✅ Live: https://pr-882-wavehouse-docs.wave-rf.workers.dev (follows this PR)
|
Contributor
Code coverage overviewLanguages: Go Go
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. |
This branch was successfully deployed
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.
Summary
WaveHouse stops modelling ClickHouse itself. Ingest validation, coercion,
DEFAULTevaluation, insert checks and row-level security now run through chtypes (github.com/wave-rf/chtypes/go/v2), a cgodlopenof a per-ClickHouse-line library that runs the server's own parser and analyzer in-process. Ingest hands the request body to ClickHouse's own reader and refuses it the way anINSERTwould; the row the worker inserts is the library's export of what it judged./v1/queryand pipes ask ClickHouse to render their results. The Go that approximated ClickHouse (the canonicalizer, row evaluator and compact encoder) is deleted.This is one squashed commit on current
main, so the diff below is exactly what changes. It supersedes #712, which carries the same work on chtypes v1 with its full review history and is left open until this one is accepted.Status: draft. It builds on the chtypes v2 dev SDK
2.0.0-dev.11, which fetches only from chtypes' dev channel. It moves to chtypes2.0.0(the stable fetch contract: release key, mirrors,--lock/--frozen) when that ships, and the docs that describe the dev channel move with it.What changed since #712
DEFAULT timezone()is answered, and no table depends on which tenant bound first (typelayer: the process image zone depends on which tenant binds first #726). Zone spellings no longer need to agree across tenants on one line (docs(typelayer): tenants on one ClickHouse line must report the same zone spelling (UTC ≠ Etc/UTC) #717).INSERT's column list and chtypes'insertable_columns; an_eqcheck's value fills an omitted field throughcolumn_defaults, spelled in the body format's own field encoding. A column the role may not write keeps its ownDEFAULT, soid UUID DEFAULT generateUUIDv4()now draws a value instead of declining the batch. Per-role compiled schemas and their 256-entry cache are gone: every role's insert runs on the table's one compiled handle.flatten_nested=0compile as the oneNested(...)column the server stores, instead of being split inton.a/n.b.scripts/fetch-chtypes.sh --resolveasks the SDK (chtypes resolve) for the build it would fetch, so the CI cache key names the channel actually used./v1/queryand pipes no longer echo the ClickHouse request URL (the tenant database, the role's caps and the bound filter values); the caller sees only what failed.<dir>/v2-dev), the mirror and trust overrides have no effect until chtypes 2.0.0, a cached build makes one revocation-status request per process, andupgrading.mdgains a section for the next release (move ClickHouse to a published line first; 26.8 is the one tested).mainup to docs(release): prepare the 0.2.1 changelog and upgrade guide #877 is included (query metrics, trusted proxies, storage metrics, the 0.2.1 changelog); query metrics are recorded on the HTTP reader the query paths now use.Documented contract changes (please review these first)
The full list, with before/after behaviour, is in
CHANGELOG.mdunder Unreleased. The ones a client notices:400 clickhouse.rejected, e.g. code 27 or 117), wheremainanswered per record.117("Unknown field"), not WaveHouse's403.DEFAULTs, NaN/Inf as"nan"/"inf"/"-inf"on every surface, itsDateTimerendering).WH_CHTYPES_AUTOFETCH,WH_CHTYPES_CACHE,WH_CHTYPES_ARTIFACTS_URL). An ingest-only or sweeper-only process needs none.JSONEachRow(a trailingFORMAT JSON,FORMAT CSV, …) now fail every call (502 clickhouse.misconfigured, not retryable); 0.2.1's native protocol ignored the clause. Remove it from the pipe.pkg/pipes.BindParamsreturns(string, error), the statement with its values inlined and escaped, where it returned(string, []any, error); an outside module importingpkg/pipesneeds a one-line change.503on ingest, withheld rows on row-filtered streams).Verification
make cipassed on this exact tree on darwin-arm64 against chtypes dev build20261021.130847and ClickHouse 26.8.15.10: Go unit 4,000, integration 218 + 99 + 6 + 1 + 346, e2e 96, TS SDK 270, every coverage gate, and the checkout stayed clean. It also passed on linux-arm64 against the same build at a revision before the last review fixes (an error-mapping change, one pinned read setting, tests and docs).make cion linux-arm64 against every chtypes v2 dev build since20261012.142240, and its type-layer, API, stream, ingest and app suites pass on darwin-arm64.DEFAULTcolumn gets its records accepted with a drawn value (unit and against a real ClickHouse server), per-formatcolumn_defaultsspelling (JSON, CSV, TSV, positional CSV) with quotes, commas, tabs, newlines and backslashes, an unreadable claim value failing the check closed in every format, andflatten_nested=0tables.Ingest benchmark,
mainvs this PR (measured)A linux-arm64 machine (Neoverse V2 cores), ClickHouse 26.8.15.10, chtypes dev build
20261021.130847, concurrency 1 / 8 / 32, each point the median of 3 × 60 s after a warm-up with the build order rotated: 36 points, 0 errors, and rows stored equal rows accepted. The compared trees differ only by this PR. The bench table has aMap(String, String)column, the shape chtypes' batch-path speedup targets.main(c = 1 / 8 / 32)mainSyncAlways)main(167.9 / 141.0 / 125.3 against 127.6 / 101.2 / 87.4 µs). feat(typelayer)!: validate, coerce and filter through chtypes #712's earlier run on dev build20261012.142240measured 54–61 µs more, so the gap shrank by about a third; most of the cost is inside the chtypes library call (profiled in feat(typelayer)!: validate, coerce and filter through chtypes #712's run; not re-profiled here).main's 15.Known limitations
Each is tracked; none is a regression against
mainunless it says so.CHECKconstraints are not evaluated at ingest (typelayer: table CHECK constraints are not evaluated at ingest #727): the type layer compiles each table on a neutralMergeTree, so a violating record is accepted, can reach the stream, and is parked by the worker when the server refuses it, as onmain. Rolecheckclauses are unaffected.DEFAULT now()/rand()column the role may not write fails every record with422, and a row filter on aMATERIALIZED now()column withholds every row.generateSnowflakeID(),rowNumberInAllBlocks()): a body in which any record omits such a column is declined (422); chtypes' opt-in deferral is not used yet.main(see the benchmark).main, whose Go evaluator compiled nothing; the cost is inferred, not yet measured.Follow-ups (not in this PR)
Epic #827 collects them, each blocked by #826, which this PR closes.
Closes #826 (the gate its follow-ups in #827 are blocked by).
Fixes #387, fixes #516, fixes #547, fixes #558, fixes #722 (as in #712), fixes #726, fixes #717, fixes #816.
🤖 Generated with Claude Code