feat(querier): mixed-schema scans via the §3.3 no-coercion adapter (RFC 0042 slice 3a) - #651
Conversation
…FC 0042 slice 3a) Files written under different promoted declarations can now share one scan. Two seams, both new in schema_adapt.rs: - merge_scanned_schemas replaces the bare Schema::try_merge: a declared promoted key's class fixes its union-schema column type; an undeclared promoted-column conflict resolves to Utf8 (the pre-0042 universal); non-promoted conflicts stay errors (RFC 0005 schema corruption). try_merge previously errored on any promoted type conflict, failing the whole query. - PromotedNoCoercionFactory (ListingTableConfig:: with_expr_adapter_factory): a promoted column whose file type differs from the scan schema's reads as absent — a typed NULL literal. DataFusion's default adapter inserts a cast instead, and Arrow's safe Utf8->Int64 cast PARSES string content — the §3.1/§3.3 forbidden coercion, where projection depends on the value rather than the variant. Everything else delegates to the default adapter. Querier grows with_promoted_attributes (default: the implicit set — pure schema-driven, the RFC 0022 behaviour, so every existing caller is unchanged); the server threads the deployment's declared set into both querier constructions. RFC0042.5 (scan half): three files — key unpromoted / string-promoted / i64-declared — scan without error; sum covers exactly the declared- class file, with a string-encoded "1000" in the mismatched Utf8 column as the no-coercion sentinel (a leaked default cast would parse it into the sum). Plus the undeclared-conflict-resolves-to-Utf8 case. The ==-JSON-arm half of the criterion lands with predicate typing (3b). Verified: cargo fmt --check, clippy --all-targets --all-features clean, nextest 1275/1275. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Implements RFC 0042 slice 3a on the querier scan path so partitions containing Parquet files written under different promoted-key declarations can be scanned together without coercing conflicting promoted column types. This is done by introducing a declared-aware schema merge plus a per-file “no-coercion” expression adapter, and threading the deployment’s declared promoted set into querier construction.
Changes:
- Add declared-aware scanned schema merging for promoted columns (declared keys fixed to declared types; undeclared promoted conflicts resolve to
Utf8; non-promoted conflicts still error). - Install a custom
ListingTableConfigphysical-expression adapter factory that reads type-mismatched promoted columns as absent (typed NULL) instead of allowing DataFusion’s default cast insertion. - Thread the deployment’s
PromotedAttributesinto querier construction and add RFC0042.5 integration tests covering mixed-schema scans.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-server/src/querier.rs | Threads the deployment promoted set into querier construction for HTTP/MCP router and serve. |
| crates/ourios-querier/src/lib.rs | Stores declared promoted set in Querier, uses new union merge, and installs the per-file expr adapter on listing tables. |
| crates/ourios-querier/src/schema_adapt.rs | New module implementing merge_scanned_schemas and the PromotedNoCoercionFactory adapter. |
| crates/ourios-querier/tests/it/rfc0042_mixed_schema.rs | Adds RFC0042.5 integration coverage for mixed-schema scans and the no-coercion sentinel. |
| crates/ourios-querier/tests/it/main.rs | Registers the new RFC0042 mixed-schema integration test module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…mismatch The no-coercion adapter compared Arrow types exactly, so Utf8View vs Utf8 (a memory-layout choice DataFusion makes at scan time, RFC0021.4) would have read a string promoted column - service.name included - as absent. Compare canonicalized classes instead (View/Large string and binary forms fold to their plain types); a view-vs-plain difference falls through to the default adapter's benign representation cast. Also strengthens the undeclared-conflict test to distinguish the Utf8 union from a wrong numeric one: sum over the conflicted column is an all-NULL group (the "junk" cell try_casts to NULL, the Int64 file reads as absent); a numeric union would have contributed 40. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
What (RFC 0042 slice 3a — the scan half of the querier work)
Files written under different promoted declarations can now share one scan without erroring or coercing. Two new seams in
schema_adapt.rs:1. Declared-aware union schema (
merge_scanned_schemas)Replaces the bare
Schema::try_merge, which errored the whole query on any promoted-column type conflict (a key promotedstringin one epoch andi64in another):Utf8(the pre-0042 universal; §3.3 reads non-matching files as NULL)2. The no-coercion per-file adapter (
PromotedNoCoercionFactory)Installed via
ListingTableConfig::with_expr_adapter_factory. A promoted column whose file type differs from the scan schema's reads as absent (typed NULL). This exists because DataFusion's default adapter inserts a cast on mismatch — and Arrow's safeUtf8 → Int64cast parses string content, making projection depend on the value rather than the variant: precisely the coercion §3.1/§3.3 forbid. Everything else (missing-column NULL fill, benign casts) delegates to the default adapter untouched.Threading
Querier::with_promoted_attributes(builder, like the sink/compactor); default = the implicit set → pure schema-driven scan, i.e. the RFC 0022 behaviour, so every existing caller and test is byte-for-byte unchanged. The server threads the deployment's declared set into both querier constructions.The test that earns its keep
RFC0042.5 (scan half): three files in one partition — key unpromoted / string-promoted / i64-declared — and the string-promoted file carries
"1000"as its value. The sum asserts 42 (the i64 file's40 + 2): if the default cast leaked anywhere,"1000"parses and the sum reads 1042. The sentinel is the negative control built into the assertion. Plus the undeclared-conflict case resolving toUtf8without error.Not in this slice (3b, next)
Predicate typing — numeric literals on numeric-class keys,
==JSON fallback arm oni64, float-equality typed-arm-only, regex compile errors (RFC0042.7/.4 and the==half of .5) — and cast-free aggregation on typed columns (RFC0042.3).Invariants
Read path only; promoted columns remain query-only projections (RFC 0017 untouched). Hazard #6: nothing DataFusion-specific leaks into the DSL surface — the adapter is invisible except as correct results.
Verification
cargo fmt --check· clippy clean ·cargo nextest run1275/1275.