feat(storage): effective-timestamp column + windowing fallback (RFC0005.13) - #179
Conversation
…on derivation (RFC0005.13) RFC 0005 §3.2 amendment 2026-06-11 (merged #178): the writer derives effective_time_unix_nano (OPTIONAL INT64 timestamp, stats-bearing, dictionary off per §3.6) from the same function the §3.4 partition tuple uses — choose_partition_timestamp is now the public effective_time_unix_nano(), so the stored column and the partition bucket can never disagree. The wire time_unix_nano stays verbatim (RFC0001.10). Storage-half RFC0005.13 assertions land in tests/effective_timestamp.rs; the schema pin (RFC0005.10) is updated in lockstep with the RFC table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…k (RFC0005.13) Both window paths (QueryRequest.time_range and the DSL range() stage) now compile through a shared time_window_filter: the half-open [from, to) bounds apply to effective_time_unix_nano (RFC 0002 §6.2 amendment 2026-06-11). The RFC 0005 §3.9 rule-2 carve-out is explicit: when the column is absent from the union schema the window filters time_unix_nano exactly as before, and in a mixed scan the NULL-filled pre-amendment rows fall back to time_unix_nano via an effective-IS-NULL disjunct — never predicate-false, so old files are never silently hidden. The OR shape (not coalesce) keeps the window inside DataFusion's pruning grammar; the new RFC0005.13 test pins row-group pruning on the stored column alongside the window-hit, pre-amendment, and mixed-scan obligations. Bare ts is untouched (RFC0001.10 — rfc0001_time_preserved passes unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements RFC 0005 §3.2 effective timestamp support across Parquet storage, benchmark tooling, and query filtering. Effective timestamps fall back to observed timestamps when wire timestamps are zero, enabling time-window queries and B1 eligibility to correctly handle observed-only telemetry records while maintaining pre-amendment backward compatibility. ChangesRFC 0005 §3.2 Effective Timestamp Implementation
Sequence DiagramsequenceDiagram
participant App as Application
participant Emit as Parquet Emitter
participant Store as Store Builder
participant Query as Query Executor
participant Storage as Parquet Storage
App->>Emit: MinedRecord(time=0, observed=T)
Emit->>Emit: compute effective_time_unix_nano() → T
Emit->>Store: pass effective span
Store->>Store: track min/max effective, exclude zero-effective
Emit->>Storage: write time_unix_nano=0, effective_time_unix_nano=T
Query->>Query: parse range(lo, hi)
Query->>Storage: filter WHERE effective >= lo AND effective < hi
Storage-->>Query: observed-only record (effective=T matches)
Query-->>App: result row_count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR implements RFC0005.13’s “effective timestamp” (wire time_unix_nano unless it is 0, then fall back to observed_time_unix_nano) as a stored Parquet column and updates query windowing + bench bookkeeping to use it, while preserving backward compatibility with pre-amendment files via a NULL-aware fallback.
Changes:
- ourios-parquet: Add OPTIONAL
effective_time_unix_nanoto the data schema and populate it via a sharedeffective_time_unix_nano()derivation used for both partitioning and writing. - ourios-querier: Route both
QueryRequest.time_rangeand DSLrange(...)through a sharedtime_window_filterovereffective_time_unix_nano, with a mixed-scan NULL fallback totime_unix_nanofor pre-amendment files. - ourios-bench: Switch span/eligibility/reference spooling from wire timestamps to effective timestamps so observed-only corpora remain benchmark-eligible.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-querier/tests/rfc0005_13.rs | Adds RFC0005.13 querier-path tests for observed-only windowing, pre-amendment fallback (alone + mixed scan), and row-group pruning behavior. |
| crates/ourios-querier/src/lib.rs | Documents QueryRequest.time_range as effective-time bounds; introduces shared time_window_filter and uses it in the request filter path. |
| crates/ourios-querier/src/compile.rs | Updates DSL compilation to apply the shared effective-time window filter rather than filtering directly on time_unix_nano. |
| crates/ourios-parquet/tests/schema_pin.rs | Updates the pinned RFC0005.10 schema field list to include effective_time_unix_nano. |
| crates/ourios-parquet/tests/effective_timestamp.rs | Adds RFC0005.13 storage-path tests asserting stored effective timestamp, partition derivation consistency, and wire-time preservation. |
| crates/ourios-parquet/src/writer.rs | Disables dictionary encoding for the new effective-time column alongside other timestamp columns. |
| crates/ourios-parquet/src/record_batch.rs | Adds the effective-time builder column and populates it via the shared derivation during batch construction. |
| crates/ourios-parquet/src/reader.rs | Updates reader test scaffolding to account for the newly added effective-time column. |
| crates/ourios-parquet/src/partition.rs | Extracts/renames partition timestamp selection into public effective_time_unix_nano() and uses it in PartitionKey::derive. |
| crates/ourios-parquet/src/lib.rs | Adds the effective-time column constant and inserts the column into the published data_schema() in the intended position. |
| crates/ourios-bench/src/store.rs | Moves store span tracking, B1 eligibility guard, and reference spooling to effective timestamps; adds an observed-only eligibility test. |
| crates/ourios-bench/benches/b2.rs | Updates B2’s window selection to use the effective-time span. |
| crates/ourios-bench/benches/b1.rs | Updates B1’s query window and skip conditions to use effective-time span and the new zero-effective guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…RFC 0005 §3.2 rule 7)
The store builders' bookkeeping now derives from
ourios_parquet::effective_time_unix_nano — the same derivation the
writer stores and the partition tuple uses — instead of the wire
time_unix_nano: the corpus span (min/max, renamed
{min,max}_effective_time_unix_nano), the B1 zero-timestamp guard
(renamed zero_effective_ts_rows: counts only rows with neither
timestamp), and the reference corpus's hour spool. An observed-only
corpus (timeUnixNano absent, observedTimeUnixNano set — ~15 % of the
v5/v6 OTel-Demo corpora) is now B1-eligible; only genuinely timeless
rows disqualify. New unit test pins the eligibility outputs the
benches/b1.rs severity_query guard checks.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6420dd2 to
de853de
Compare
Implements the effective-timestamp amendment merged in #178 (RFC 0005 §3.2/§3.4/§3.6/§3.9 + RFC 0002 §6.2, main @ 407c12a) across
ourios-parquet,ourios-querier, andourios-bench. This unblocks B1, the last unmeasured thesis gate: ~15 % of the v5/v6 OTel-Demo corpora are observed-only-timestamp records, which were previously unaddressable by time and tripped the bench's zero-timestamp guard.What lands
ourios-parquet— the stored column via one shared derivationchoose_partition_timestampis extracted as the publiceffective_time_unix_nano()(RFC 0005 §3.2 rule 1). Both the §3.4 partition tuple (PartitionKey::derive) and the new stored column (the record-batch writer) call this one function, so the spec's "the partition tuple and the stored column never disagree" rule is structural, not a convention.effective_time_unix_nanocolumn: OPTIONALTIMESTAMP(NANOS, UTC)/INT64, placed afterobserved_time_unix_nano, stats-bearing with dictionary off (§3.6 — same encoding row astime_unix_nano). The writer always populates it;NULLexists only in pre-amendment files.MinedRecordis unchanged (RFC 0003 §6.6 note: receiver contract untouched); the reader maps the column to nothing — it is derivable, not carried, and outside the RFC0005.1 round-trip surface.ourios-querier— the window + the §3.9 carve-outQueryRequest.time_rangeand the DSLrange(...)stage) compile through one sharedtime_window_filter: half-open[from, to)overeffective_time_unix_nano(RFC 0002 §6.2 amendment). Baretsis untouched — it still resolves to the verbatim wiretime_unix_nano(RFC0001.10).time_unix_nanodirectly, exactly as before;(eff >= lo AND eff < hi) OR (eff IS NULL AND ts >= lo AND ts < hi): post-amendment writers never store NULL, soIS NULLidentifies exactly the old rows needing the fallback. TheORshape (notcoalesce) keeps the predicate inside DataFusion's pruning grammar — min/max stats prune the effective branch, null counts collapse the fallback branch (RFC 0005 §3.2 rule 3, the B1 mechanism); the new test pins row-group pruning on the stored column.hour_partition_in_windowneeded no change — §3.4 partitioning already used the same fallback.ourios-bench— B1 eligibility keys off the effective span (§3.2 rule 7){min,max}_effective_time_unix_nano), the zero-timestamp guard (zero_effective_ts_rows: counts only rows with neither timestamp), and the reference corpus's hour spool all derive from the sharedeffective_time_unix_nano()— the bench can never disagree with what the query window filters. Observed-only corpora are now B1-eligible.RFC0005.13 acceptance (both halves)
crates/ourios-parquet/tests/effective_timestamp.rs+crates/ourios-querier/tests/rfc0005_13.rs): atime_unix_nano = 0/observed_time_unix_nano = Trecord storeseffective == T(read raw via theparquetcrate), lands under the partition tuple derived fromT, keeps the wiretime_unix_nano = 0verbatim, and arange(...)window containingTreturns it (and a window over the epoch does not).rfc0005_13.rs): a pre-amendment-shaped file (the writer's batch with the column projected away, laid down via the rawArrowWriterper the RFC0007.4 pattern) answers the same window aseffective := time_unix_nano— alone and mixed with a post-amendment file in one scan. No error, no hidden rows.zero_effective_ts_rows == 0, span from observed values, reference fully spooled).CLAUDE.md §3.5 / §4-adjacent claim (reviewers: please check this one)
The schema change is additive-OPTIONAL with an explicit migration story: old files need no rewrite, and — this is the load-bearing claim — pre-amendment files keep answering time-window queries identically to before this PR, because the §3.9 rule-2 read default substitutes
effective := time_unix_nanoper file instead of compiling the window tofalse. The mixed-scan NULL case (DataFusion's schema union) is the spot where a naive implementation silently hides all historical data from every query;rfc0005_13_pre_amendment_file_windows_on_time_unix_nanopins that it does not happen. Readers tolerate the absent column (old files) and ignore the unknown column (the pre-amendment reader reading new files) per RFC0007.4, which stays green.Hazard 4.4 (small files) untouched; hazard 4.6: the new filter stays inside the compile layer — no DataFusion type or SQL crosses the public surface.
Verification (all run locally, all green)
cargo test --all-features— 79 suites, 613 tests, 0 failed (incl.rfc0001_time_preservedand the RFC0007 structural tests, unchanged)cargo fmt --all --checkcargo clippy --all-targets --all-features -- -D warningscargo doc --workspace --no-deps --all-featurescargo bench -p ourios-bench --no-run🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests