docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream) - #41
Merged
Conversation
Opens Phase 2 with the on-disk schema RFC the CLAUDE.md §3.5 invariant requires before any ourios-parquet code lands. Pins: - The data-file Parquet schema (column-by-column mapping of RFC 0001 §6.1's MinedRecord onto Parquet types, with tenant + time as Hive-style partition keys). - The AnyValue encoding rule (OTLP-canonical JSON in a BYTE_ARRAY, not a recursive typed STRUCT — rejected for MVP because Parquet's flat-nested model can't represent the discriminated union's recursion faithfully without capping depth). - The audit-event file schema — the cross-RFC contract from RFC 0001 §9 that ties §6.4 widening events to a separate file series. - The writer's row-group / file-size targets per hazards.md H4, the compression codec (ZSTD-3), and the per-column encoding policy (notably no dictionary on the body column — the §3.2 cardinality invariant forbids it). - The reader's forward- and backward-compatibility contract (unknown columns ignored, missing OPTIONAL columns surface as None, missing baseline REQUIRED columns error). - The schema-evolution rules anchored to CLAUDE.md §3.5. Status: drafted. §5 acceptance criteria are written so reviewers can flip to specified if the criteria pass review; a follow-on code PR will add test stubs and move the status to red, then implementation PRs move it to green. Doc-only — per the split-doc-spec convention, no scaffolding or code rides this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new draft RFC that specifies the on-disk Parquet contract (schemas for data + audit files, partition layout, encoding/compression policy, and reader compatibility rules) as the Phase 2 prerequisite before landing any ourios-parquet implementation.
Changes:
- Add RFC 0005 describing Parquet schemas (data + audit), Hive-style partitioning, writer sizing/encoding targets, and reader forward/backward compatibility rules.
- Register RFC 0005 in the mdBook table of contents.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/SUMMARY.md | Adds the RFC 0005 entry to the docs navigation. |
| docs/rfcs/0005-parquet-storage.md | Introduces the draft RFC defining Parquet storage schema + contracts for upcoming ourios-parquet work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jensholdgaard
added a commit
that referenced
this pull request
May 19, 2026
Four substantive comments on PR #41, all addressed: 1. tenant_id was REQUIRED but also "not stored inside row groups" — contradictory. Now documented as a Hive partition column (not a row-level column), synthesised by the reader from the partition path. The §3.8 schema-evolution rules explicitly do not apply to partition columns; §3.4 pins the partition contract separately. Same treatment in §3.7's audit schema. 2. params and separators were OPTIONAL but described as always present (mirroring RFC 0001's Vec<...> fields). Now REQUIRED with the explicit rule that NULL is not a valid encoding; the list may be empty. 3. The §3.5 "Compression codec" bullet conflated compression (ZSTD) with encoding (RLE, dictionary). Now states the codec policy cleanly (ZSTD-3 across the board) and explicitly defers per-column encoding to §3.6. 4. positions_widened was OPTIONAL but described as "empty list for variants that don't widen positions" — same issue as #2. Now REQUIRED with documented empty-list semantics for TemplateTypeExpanded and TemplateWideningRejectedDegenerate, plus NULL-handling clarified for the other audit columns. RFC0005.1 updated to scope the round-trip property to row-level columns; partition-column synthesis becomes an "And" clause delegating the partition contract to RFC0005.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/rfcs/0005-parquet-storage.md:799
- This scenario defines partition mismatch in terms of the UTC buckets from
time_unix_nano, but §3.4 defines the partition tuple astime_unix_nanowith fallback toobserved_time_unix_nano(and then the epoch bucket). Update the scenario wording to match the actual derivation used for partitioning/validation so tests don’t encode the wrong rule.
> **Scenario RFC0005.11 — Row-vs-path validation on partition mismatch**
> - **Given** a Parquet file whose row-level `tenant_id` (or UTC
> year / month / day / hour from `time_unix_nano`) disagrees with
> the partition-path segments the file lives under
> - **When** the reader opens the file via `Reader::open_partition`
> - **Then** the reader returns a hard error naming the offending
> row, the row's value, and the partition path's value
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/rfcs/0005-parquet-storage.md:800
- Scenario RFC0005.11 describes partition validation in terms of UTC buckets derived from
time_unix_nanoonly, but §3.4 specifies a fallback toobserved_time_unix_nanowhentime_unix_nano == 0. Please align the scenario wording with the §3.4 derivation rule so the acceptance test covers the intended behavior (including the unknown-time sentinel case).
> **Scenario RFC0005.11 — Row-vs-path validation on partition mismatch**
> - **Given** a Parquet file whose row-level `tenant_id` (or UTC
> year / month / day / hour from `time_unix_nano`) disagrees with
> the partition-path segments the file lives under
> - **When** the reader opens the file via `Reader::open_partition`
> - **Then** the reader returns a hard error naming the offending
> row, the row's value, and the partition path's value
> - **And** no records are surfaced from the file
Four substantive comments on PR #41, all addressed: 1. tenant_id was REQUIRED but also "not stored inside row groups" — contradictory. Now documented as a Hive partition column (not a row-level column), synthesised by the reader from the partition path. The §3.8 schema-evolution rules explicitly do not apply to partition columns; §3.4 pins the partition contract separately. Same treatment in §3.7's audit schema. 2. params and separators were OPTIONAL but described as always present (mirroring RFC 0001's Vec<...> fields). Now REQUIRED with the explicit rule that NULL is not a valid encoding; the list may be empty. 3. The §3.5 "Compression codec" bullet conflated compression (ZSTD) with encoding (RLE, dictionary). Now states the codec policy cleanly (ZSTD-3 across the board) and explicitly defers per-column encoding to §3.6. 4. positions_widened was OPTIONAL but described as "empty list for variants that don't widen positions" — same issue as #2. Now REQUIRED with documented empty-list semantics for TemplateTypeExpanded and TemplateWideningRejectedDegenerate, plus NULL-handling clarified for the other audit columns. RFC0005.1 updated to scope the round-trip property to row-level columns; partition-column synthesis becomes an "And" clause delegating the partition contract to RFC0005.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nine new comments on the previous fix commit (8543543), all addressed: 5. Added §3.0 Terminology note explicitly defining MinedRecord as the planned Rust type name for RFC 0001 §6.1's record schema. Avoids cross-RFC ambiguity until a 0001 patch adopts the name. 6. Renamed partition path segment from `tenant=` to `tenant_id=` throughout, matching the Hive convention (path key = column name). Stale references in RFC0005.5 and RFC0005.7 caught and fixed too. 7. Added §3.7 event-kind mapping table linking the INT32 ordinal stored on disk, RFC 0001 §6.4's snake_case `event_type` string, and the Rust variant name. The three surfaces now have a single normative correspondence. 8. Rewrote RFC0005.1 — the "out of scope but synthesised" contradiction is replaced with a clear row-level-only equality clause; the row-vs-path validation moves to a new RFC0005.11 (its own scenario + test). 9. Standardised on MiB/GiB throughout (§3.5, RFC0005.6, testing notes) and noted that the RFC chose binary units because Parquet metadata's byte counts are unprefixed binary bytes. 10. Split RFC0005.8 — compression codec (`ZSTD`) and encoding (no `PLAIN_DICTIONARY`/`RLE_DICTIONARY`) are now two distinct "Then" clauses naming the distinct Parquet-metadata fields. 11. Dropped the "CI invokes it as a nightly job" claim for RFC0005.6 (the workflow has no `schedule:` trigger today). Replaced with "run manually via cargo test --ignored" + a new §7 open question committing the scheduled-CI workflow PR as a separate follow-up. 12. Rewrote the "memory-captured rule" line in §3.10 as a self- contained rationale: pre-abstracting before the second consumer is visible picks the wrong axis. No external memory reference. 13. The big one — reconciled with RFC 0001 §6.1 and docs/talks/ 0001-template-miner.md ("tenant_id is present on every row … we trust the row"). tenant_id is now a REQUIRED row-level column (data + audit) and is **also** replicated in the partition path; the row is authoritative; the reader validates row-vs-path and errors on mismatch. §3.9 gains a third bullet pinning that contract; RFC0005.11 covers it as a test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Thirteen new substantive comments. Stale carry-overs #1 and #2 folded into the same commit since the summary's MinedRecord reference and §3.4's tenant_id= naming were the lingering hooks. 3, 12. §3.8 vs §3.9 internal contradiction: §3.8 rule 1 forbids REQUIRED additive columns, but §3.9 defaulted them. Dropped the REQUIRED-added-in-amendment bullet from §3.9 with an explicit cross-reference to §3.8 rule 1. 4. URL-encoding spec was "URL-encoded" without a normative variant. Pinned to RFC 3986 percent-encoding with explicit overrides: UTF-8 byte input, no Unicode normalisation, escape everything outside the unreserved set (including `/`, `=`, `%`). Malformed-escape decoding is a hard read error. 5, 13. UUIDv7 conflict: §3.4 normatively required UUIDv7 but §7 listed it as an open question. Resolved by pinning UUIDv7 in §3.4 (matters for partition-lexsort = creation-order) and removing the open question. 6. RFC0005.7 referenced `TemplateWidened` (the Rust variant name) while RFC 0001 uses `template_widened` (snake_case event_type). Rewrote the Given clause to use the RFC 0001 string and parenthetically map to the Rust variant. 7. "§3.2 cardinality invariant" was ambiguous — §3.2 in this RFC is the schema, the invariant lives in CLAUDE.md §3.2 / hazards H2. Qualified all four cardinality-invariant references with `CLAUDE.md` §3.2. 8. TIMESTAMP logical type used `isAdjusted=true` (not a Parquet spec field). Replaced with `isAdjustedToUTC=true` (Parquet's actual flag name); two occurrences fixed via replace-all. 9. trace_id/span_id listed `BYTES` as Parquet logical type, which isn't a real Parquet logical type. Switched trace_id to `UUID` (matches 16-byte fixed-len) and span_id to "no logical type" (Parquet has no 8-byte opaque-id annotation; physical type alone is the contract). 10. §3.7 audit schema was missing RFC 0001 §6.4 fields. Added: - `triggering_line_hash` (FIXED_LEN_BYTE_ARRAY(16), blake3 of L_raw, REQUIRED) - `triggering_line_sample` (STRING, first 256 B of L_raw, OPTIONAL) - `slots_expanded` (LIST<STRUCT<slot_index, types_added: LIST<INT32>>>, REQUIRED) — replaces the singleton `type_added`/`slot_index` columns since RFC 0001 specifies `Vec<SlotExpansion>` (a single attach can grow multiple slots) 11. RFC 0001 §9 names `event_type` (snake_case STRING) as the drift-query predicate-pushdown column. RFC 0005 had replaced that with `event_kind` ordinal. Resolution: store BOTH — the INT32 ordinal for writer/reader internal use, and the STRING for RFC 0001's contract. Writer must keep them in sync per the mapping table. 14. RFC0005.7 said "full §3.7 payload" then enumerated a subset. Rephrased to "every row-level column declared in §3.7's audit- schema table" with explicit NULL-on-OPTIONAL semantics. 15. RFC0005.8's "file size doesn't balloon proportionally" clause wasn't testable. Replaced with a concrete assertion: the body column chunk's `dictionary_page_offset` is unset — no dictionary page exists on disk for the column. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…it stream) fixup! attributes/resource_attributes REQUIRED with empty-Vec ↔ [] mapping
jensholdgaard
force-pushed
the
feat/rfc-0005-parquet-storage
branch
from
May 20, 2026 22:26
87cc1bd to
a8ee006
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.
Summary
Opens Phase 2 with the on-disk Parquet schema RFC the
CLAUDE.md§3.5 invariant requires before anyourios-parquetcode lands. Status: drafted — review is invited on §5 acceptance criteria so the status can flip tospecifiedon merge.The RFC pins:
MinedRecordRust type, see §3.0), withtenant_idas a row-level REQUIRED column and the leading Hive partition key (tenant_id=<id>/year=…/month=…/day=…/hour=…/); the row is authoritative, the reader validates row-vs-path on mismatchtriggering_line_hash,triggering_line_sample, andslots_expandedper RFC 0001 §6.4; bothevent_kind(ordinal) andevent_type(snake_case STRING) are persisted, the STRING for §6.7 drift-query predicate-pushdownhazards.mdH4 (MiB/GiB throughout), compression codec (ZSTD-3 across the board), file naming pinned to UUIDv7 (RFC 9562), partition value encoding pinned to RFC 3986 percent-encoding over UTF-8 bytes, and per-column encoding policy — notably no dictionary onbody(theCLAUDE.md§3.2 cardinality invariant forbids it)None, missing baseline REQUIRED columns error, partition row-vs-path mismatch is a hard read error)CLAUDE.md§3.5 (additive amendments are always OPTIONAL; rename/type-change forbidden in-place)Notable design decisions called out in §4
AnyValueas canonical JSON inBYTE_ARRAY, not a typed recursive STRUCT. Parquet's flat-nested model can't represent OTLP's discriminated union's recursion faithfully without capping depth; the typed-attribute query path is a future RFC gated on a concrete consumer.Open questions (§7)
Six items deferred (the writer PR or the A1/B1/B2 thesis-gate measurements will resolve them): compression-codec level (ZSTD-3 vs ZSTD-22), bloom-filter sizing on
template_id, audit-event retention asymmetry, partition-discovery API surface on the reader, concurrent-writer atomicity convention, scheduled-CI cadence for the slow#[ignore]d tests.File naming was previously listed as open; the RFC body now pins UUIDv7 in §3.4 and the open question was removed.
Test plan
mdbook buildsucceeds with the new RFC and nav entrydrafted→specifiedflip)ourios-parquetcrate → writer → reader → audit stream (one each, per the §6.2 phased-execution rule)🤖 Generated with Claude Code