Skip to content

Commit 9763a93

Browse files
g-talbotclaude
andauthored
feat: port zonemap package and wire into Parquet pipeline (#6295)
* feat: port zonemap package and wire into Parquet pipeline Port the Go zonemap package (DFA-based prefix-preserving superset regex builder) to Rust and integrate it into the Parquet write pipeline. For each string-valued sort schema column, a compact regex is generated that accepts all column values (and possibly more). These regexes enable query-time split pruning: if a predicate cannot match any string accepted by the regex, the split can be skipped entirely. Zonemap module (quickwit-parquet-engine/src/zonemap/): - automaton.rs: DFA with weighted pruning and deterministic regex generation, including character class collapsing and suffix factoring - regex_builder.rs: PrefixPreservingRegexBuilder with progressive pruning during registration and final prune at build time - minmax.rs: FNV-1a hash-based MinMax builder for future range pruning - mod.rs: extract_zonemap_regexes() public API for Arrow RecordBatches Pipeline integration: - writer.rs: extract zonemap regexes in prepare_write() after row_keys, store as JSON in qh.zonemap_regexes Parquet KV metadata - split_writer.rs: capture zonemap_regexes in MetricsSplitMetadata - WriteMetadata type alias carries both row_keys and zonemap through write_to_bytes() and write_to_file_with_metadata() 34 tests covering automaton (regex, pruning, escaping, character classes, disjunctive clauses, long strings, post-prune behavior), regex builder (basic, pruning, progressive), MinMax (string/int hashing, reset, empty), Arrow extraction (basic, nulls, missing columns, special chars, disabled), and full pipeline integration (KV metadata, split writer, JSON roundtrip, write path consistency). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: gate PARQUET_META_ZONEMAP_REGEXES re-export behind #[cfg(test)] The pub(crate) re-export was unused in non-test builds, causing -D unused-imports to fail in CI. The constant is only needed by zonemap pipeline_tests, so gate it behind #[cfg(test)]. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix import ordering for nightly fmt group_imports Reorder pub use re-exports and use crate:: imports to satisfy cargo +nightly fmt with group_imports = StdExternalCrate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: expect infallible HashMap<String, String> JSON serialization serde_json::to_string on HashMap<String, String> cannot fail — silently swallowing the error with let Ok() would hide a real bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add missing Go test coverage and LSM cutoff support Port remaining Go builder_test.go cases: - TestBuildFragmentZoneMap: exact regex string verification - TestNilSortSchema: empty sort fields → no regexes - TestNonMutatedResult: builder reuse independence - TestInvalidUTF8: Unicode BMP handling (Rust variant) - TestZoneMapForIntColumns: int columns produce no regex - TestResetWithLsmComparisonCutoff: LSM cutoff truncation Also fixes extract_zonemap_regexes to respect lsm_comparison_cutoff from the sort schema — only columns before the cutoff get zonemaps, matching Go FragmentZoneMapBuilder.Reset() behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: use full 584-line integrations benchmark data file Include the complete benchmark_data/integrations file from the Go zonemap package and use it via include_str!() in the benchmark test. Verifies that 584 real integration names with max 64 transitions produces a valid pruned regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify exact regex strings and add LSM cutoff support Add exact regex string verification for long service name (matches Go TestBuildFragmentZoneMap: "^a_very_very_very_very_long_long_.+$") and metric_name single-value case ("^cpu\.usage$"). Also verifies service and env dict columns produce identical regexes for identical values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: update GAP-002, GAP-004, and ADR-002 with resolution status Mark completed items in GAP-002 (sort schema parser, configurable directions, timeseries_id, schema-driven sort, metadata storage) and GAP-004 (MetricsSplitMetadata fields, RowKeys, zonemap regexes, sorting_columns, KV metadata). Update ADR-002 Implementation Status to reflect the full PR stack (#6287-#6295). Remaining open items: per-index metastore storage (Phase 32), null ordering fix, Parquet column/offset index enabling, PostgreSQL migration for row_keys + zonemap columns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: nulls always sort last regardless of column direction Change nulls_first from true to false in sort_batch(), sorting_columns() metadata, and the SS-1 verification sort. Nulls now sort after all non-null values for both ascending and descending columns. This simplifies compaction: when a sort column is absent from a split, all rows are treated as null. With nulls-last, these rows cluster at the end and don't interfere with key-range comparisons between splits. Adds test_nulls_sort_last_ascending_and_descending which writes through the full pipeline with ascending and descending service columns, verifying null rows appear last in both cases. Updates ADR-002 and GAP-002 to mark the null ordering item resolved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply nightly fmt to nulls-last test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: use struct init instead of field reassign for TableConfig Fixes clippy::field_reassign_with_default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve merge conflicts after rebase — adapt to ParquetSplitMetadata API - Keep DDSketch sketch validation in prepare_write - Fix ParquetSplitWriter::new to pass ParquetSplitKind - Update MetricsSplitMetadata references to ParquetSplitMetadata - Remove .metadata indirection (write_split returns metadata directly) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add timeseries_id to remaining inline test batches after mandatory enforcement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add .unwrap() to constructor calls and fix WriteMetadata destructuring in zonemap tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: replace raw pointers with arena indices in automaton pruning The pruning algorithm used raw *mut State pointers in a BinaryHeap to enable mutable access to tree nodes during traversal. Replace with an arena (Vec<State>) indexed by usize, eliminating the unsafe block entirely. States are now allocated via alloc_state() and referenced by index throughout add(), prune(), and regex generation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle LargeUtf8 in register_string_values to match is_string_dict is_string_dict accepted Dictionary(_, LargeUtf8) and plain LargeUtf8, but register_string_values only handled Dictionary(_, Utf8) and Utf8. A LargeUtf8 column would pass the filter but produce an empty regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use debug_assert for max_num_transitions invariant instead of panic This is an internal invariant (not user input), so debug_assert is appropriate — it documents the contract for reviewers and catches violations in debug builds without crashing production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify all string column types produce zonemaps Exercises extract_zonemap_regexes with Utf8, LargeUtf8, Dictionary(Int32, Utf8), and Dictionary(Int32, LargeUtf8) columns. All four produce the same regex for the same input values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: merge duplicate arrow::datatypes imports for nightly fmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle all Arrow dictionary key types in zonemap register_string_values Previously only DictionaryArray<Int32Type> was handled. A dictionary with any other key type (Int8, UInt8, etc.) would pass is_string_column but silently produce an empty regex, breaking the superset guarantee. Uses a macro to expand all 8 key types (Int8..UInt64) without boilerplate. Test covers all 18 dictionary variants (8 key types × Utf8/LargeUtf8) plus plain Utf8 and LargeUtf8, asserting each produces the exact correct regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use [\s\S] instead of . in pruned regex wildcards to match newlines The '.' wildcard excludes \n in standard regex semantics. If a tag value contains a newline, a pruned regex using .+ would fail to match, breaking the superset guarantee and causing incorrect split pruning. [\s\S] matches any character including newlines without requiring dot-all mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify pruned regex matches values containing newlines Adds a test that registers values with embedded \n, prunes, compiles the generated regex, and asserts it matches all original values. This would fail with the old '.' wildcard since '.' doesn't match newlines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a6b5b73 commit 9763a93

17 files changed

Lines changed: 3217 additions & 73 deletions

File tree

docs/internals/adr/002-sort-schema-parquet-splits.md

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,9 @@ Each column has:
5252
| **Name** | Column name as it appears in the Parquet schema |
5353
| **Direction** | Ascending (`+`, default) or descending (`-`). `timestamp` defaults to descending |
5454
| **Type** | Inferred from Parquet schema: string/binary (lexicographic), integer types (numeric), float types (numeric, NaN sorts after all values per IEEE 754 total order) |
55-
| **Null handling** | Nulls sort **after** non-null values for ascending columns, **before** non-null values for descending columns |
55+
| **Null handling** | Nulls always sort **after** non-null values (`nulls_first: false`), regardless of column direction |
5656

57-
**Note on null handling:** The current implementation uses `nulls_first: true` for all columns. This must be changed to match the design: ascending columns should use `nulls_first: false` (nulls last), descending columns should use `nulls_first: true` (nulls first). This ensures nulls cluster at the end of each column's value range in both directions.
57+
**Note on null handling:** Nulls sort last for all columns. This simplifies compaction: when a sort column is absent from a split, all rows are treated as null for that column. With nulls-last, these rows cluster at the end and don't interfere with key-range comparisons between splits that do have the column. Implemented in PR #6295.
5858

5959
### 2. Schema Requirements
6060

@@ -169,22 +169,27 @@ Phase 4 of the locality compaction roadmap extends sorting to the Tantivy pipeli
169169

170170
| Component | Location | Status |
171171
|-----------|----------|--------|
172-
| Fixed sort at ingestion | `quickwit-parquet-engine/src/storage/writer.rs:84-109` | Done (Matthew Kim). Hardcoded sort on MetricName, TagService, TagEnv, TagDatacenter, TagRegion, TagHost, TimestampSecs |
173-
| Sort column definition | `quickwit-parquet-engine/src/schema/fields.rs:146-158` | Done. `ParquetField::sort_order()` returns fixed column list |
174-
| lexsort_to_indices usage | `quickwit-parquet-engine/src/storage/writer.rs` | Done. Arrow sort + take kernel applied in `sort_batch()` |
172+
| Fixed sort at ingestion | `quickwit-parquet-engine/src/storage/writer.rs` | Done (Matthew Kim). Replaced by configurable sort in PR #6287 |
173+
| Configurable sort schema | `quickwit-parquet-engine/src/table_config.rs` | Done (PR #6287). `TableConfig` with `effective_sort_fields()` override; `ParquetWriter` resolves sort fields dynamically |
174+
| Sort schema parser | `quickwit-parquet-engine/src/sort_fields/parser.rs` | Done (PR #6290). Parses `column\|...\|&metadata\|timestamp/V2` with directions, LSM cutoff, version |
175+
| Per-column sort direction | `sort_fields/parser.rs` + `storage/writer.rs` | Done (PR #6290 + #6287). Parser extracts `+`/`-` suffix; writer respects `descending` flag |
176+
| lexsort_to_indices usage | `quickwit-parquet-engine/src/storage/writer.rs` | Done. Arrow sort + take kernel with stable-sort tiebreaker |
177+
| Physical column ordering | `quickwit-parquet-engine/src/storage/writer.rs` | Done (PR #6287). Sort columns first, then sorted_series, then alphabetical |
178+
| timeseries_id computation | `quickwit-indexing/src/ingest/arrow_metrics.rs` | Done (PR #6286). SipHash-2-4 over canonicalized tag key/value pairs, computed at OTLP ingest |
179+
| sorted_series column | `quickwit-parquet-engine/src/sorted_series/mod.rs` | Done (PR #6290). Order-preserving binary encoding of sort schema tag values + timeseries_id |
180+
| RowKeys (min/max boundaries) | `quickwit-parquet-engine/src/row_keys/mod.rs` | Done (PR #6292). First/last row sort column values as proto, stored in KV metadata + MetricsSplitMetadata |
181+
| Zonemap regexes | `quickwit-parquet-engine/src/zonemap/mod.rs` | Done (PR #6295). Prefix-preserving superset regex per string sort column, stored in KV metadata + MetricsSplitMetadata |
182+
| Sort metadata in Parquet key_value_metadata | `quickwit-parquet-engine/src/storage/writer.rs` | Done (PR #6292 + #6295). `qh.sort_fields`, `qh.row_keys`, `qh.row_keys_json`, `qh.window_start`, `qh.window_duration_secs`, `qh.zonemap_regexes` |
183+
| Parquet native sorting_columns field | `quickwit-parquet-engine/src/storage/writer.rs` | Done (PR #6287). `sorting_columns()` sets column indices and directions |
184+
| Nulls-last ordering | `quickwit-parquet-engine/src/storage/writer.rs` | Done (PR #6295). `nulls_first: false` for all sort columns — nulls always sort after non-null values regardless of direction. Tested ascending + descending |
175185

176186
### Not Yet Implemented
177187

178188
| Component | Notes | Gap |
179189
|-----------|-------|-----|
180-
| Sort schema parser | Parse `column\|...\|timestamp&metadata/V2` format | [GAP-002](./gaps/002-fixed-sort-schema.md) |
181-
| Sort schema in metastore | Schema stored per-index in metastore, mutable at runtime, propagated to pipelines on change | [GAP-002](./gaps/002-fixed-sort-schema.md) |
182-
| Configurable sort directions | Currently all ascending. Need per-column `+`/`-` | [GAP-002](./gaps/002-fixed-sort-schema.md) |
183-
| Correct null ordering | Currently `nulls_first: true` for all. Need nulls-last for ascending | [GAP-002](./gaps/002-fixed-sort-schema.md) |
190+
| Sort schema in metastore | Schema stored per-index in metastore, mutable at runtime, propagated to pipelines on change. Currently `TableConfig::default()` is hardcoded in `indexing_pipeline.rs` | [GAP-002](./gaps/002-fixed-sort-schema.md) (Phase 32) |
184191
| Parquet column index + offset index emission | Enable page-level min/max stats at write time | [GAP-004](./gaps/004-incomplete-split-metadata.md) |
185-
| Sort metadata in PostgreSQL | sort_schema, per-column min/max/regex in MetricsSplitMetadata | [GAP-004](./gaps/004-incomplete-split-metadata.md) |
186-
| Sort metadata in Parquet key_value_metadata | sort_schema, min/max/regex embedded in file | [GAP-004](./gaps/004-incomplete-split-metadata.md) |
187-
| Parquet native sorting_columns field | Declare sort order in Parquet file metadata | [GAP-004](./gaps/004-incomplete-split-metadata.md) |
192+
| Sort metadata in PostgreSQL | Full migration for row_keys + zonemap columns in `metrics_splits` table | [GAP-004](./gaps/004-incomplete-split-metadata.md) |
188193

189194
## References
190195

docs/internals/adr/gaps/002-fixed-sort-schema.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
# GAP-002: Fixed Hardcoded Sort Schema
22

3-
**Status**: Open
3+
**Status**: Partially resolved
44
**Discovered**: 2026-02-19
55
**Context**: Codebase analysis during Phase 1 locality compaction design. Sort implementation by Matthew Kim provides the foundation but is not configurable.
6+
**Resolution**: PRs #6287#6292 replaced the hardcoded sort with a configurable `TableConfig` + sort schema parser. Remaining: per-index metastore storage, pipeline propagation, null ordering fix.
67

78
## Problem
89

@@ -95,14 +96,14 @@ All of these systems treat sort order as a configurable, per-table property.
9596

9697
## Next Steps
9798

98-
- [ ] Define sort schema parser for `column|...|timestamp&metadata/V2` format
99-
- [ ] Store sort schema as per-index property in the metastore (mutable at runtime)
100-
- [ ] Propagate sort schema changes from metastore to indexing pipelines on appropriate nodes
101-
- [ ] Replace `ParquetField::sort_order()` with schema-driven column selection
102-
- [ ] Fix null ordering: ascending columns use `nulls_first: false`, descending columns use `nulls_first: true`
103-
- [ ] Support per-column sort direction (`+`/`-` suffix)
104-
- [ ] Implement optional timeseries_id computation (xxHash64 or SipHash over canonicalized tags)
105-
- [ ] Store sort schema in MetricsSplitMetadata and Parquet file key_value_metadata
99+
- [x] Define sort schema parser for `column|...|timestamp&metadata/V2` format — PR #6290 (`sort_fields/parser.rs`), supports column names, `+`/`-` direction, `&` LSM cutoff, `/V2` version
100+
- [ ] Store sort schema as per-index property in the metastore (mutable at runtime) — future Phase 32
101+
- [ ] Propagate sort schema changes from metastore to indexing pipelines on appropriate nodes — future Phase 32
102+
- [x] Replace `ParquetField::sort_order()` with schema-driven column selection — PR #6287 (`writer.rs` uses `resolved_sort_fields` from `TableConfig.effective_sort_fields()`)
103+
- [x] Fix null ordering: nulls always sort last (`nulls_first: false`) regardless of column direction — PR #6295 (`sort_batch()`, `sorting_columns()`, SS-1 verification). Tested with ascending and descending columns in `test_nulls_sort_last_ascending_and_descending`
104+
- [x] Support per-column sort direction (`+`/`-` suffix) — PR #6290 (parser) + PR #6287 (writer respects `descending` flag)
105+
- [x] Implement optional timeseries_id computation — PR #6286 (SipHash-2-4 over canonicalized tags, computed at OTLP ingest in `arrow_metrics.rs`)
106+
- [x] Store sort schema in MetricsSplitMetadata and Parquet file key_value_metadata — PR #6292 (`qh.sort_fields` KV entry + `metadata.sort_fields`)
106107

107108
## References
108109

docs/internals/adr/gaps/004-incomplete-split-metadata.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
# GAP-004: Incomplete Split Metadata for Compaction and Query Pruning
22

3-
**Status**: Open
3+
**Status**: Partially resolved
44
**Discovered**: 2026-02-19
55
**Context**: Codebase analysis during Phase 1 locality compaction design
6+
**Resolution**: PRs #6287#6295 added sort_schema, window, min/max (RowKeys), regex (zonemap), and sorting_columns to MetricsSplitMetadata and Parquet KV metadata. Remaining: Parquet column index + offset index enabling, PostgreSQL migration for new columns.
67

78
## Problem
89

@@ -111,16 +112,16 @@ Rich per-split (per-file) metadata for query pruning is standard in modern colum
111112

112113
## Next Steps
113114

114-
- [ ] Add `sort_schema`, `window_start`, `window_duration_secs` to `MetricsSplitMetadata`
115-
- [ ] Add `SortColumnValue` tagged union type (string, i64, u64, f64, null)
116-
- [ ] Add `schema_column_min_values`, `schema_column_max_values`, `schema_column_regexes` to `MetricsSplitMetadata`
117-
- [ ] Create PostgreSQL migration adding new columns to `metrics_splits`
118-
- [ ] Update `PgMetricsSplit` and `InsertableMetricsSplit` in `postgres.rs`
115+
- [x] Add `sort_schema`, `window_start`, `window_duration_secs` to `MetricsSplitMetadata` — PR #6292 (`sort_fields`, `window` fields in `metadata.rs`)
116+
- [x] Add `SortColumnValue` tagged union type (string, i64, u64, f64, null) — PR #6292 (proto `ColumnValue` with `oneof` in `event_store_sortschema.proto`)
117+
- [x] Add `schema_column_min_values`, `schema_column_max_values`, `schema_column_regexes` to `MetricsSplitMetadata` — PR #6292 (`row_keys_proto: Option<Vec<u8>>` for min/max RowKeys), PR #6295 (`zonemap_regexes: HashMap<String, String>`)
118+
- [ ] Create PostgreSQL migration adding new columns to `metrics_splits` — partially done in PR #6292 (`InsertableMetricsSplit` has `row_keys`); full migration pending
119+
- [ ] Update `PgMetricsSplit` and `InsertableMetricsSplit` in `postgres.rs` — partially done in PR #6292; zonemap columns pending
119120
- [ ] Enable Parquet column index and offset index in writer properties
120-
- [ ] Set `sorting_columns` in Parquet file metadata based on sort schema
121-
- [ ] Write `sort_schema`, `schema_column_min_values`, `schema_column_max_values`, `schema_column_regexes`, `window_start`, `window_duration_secs` to Parquet `key_value_metadata`
122-
- [ ] Compute per-column min/max during split writing (scan sort + metadata-only columns)
123-
- [ ] Compute per-column regex during split writing (follow Husky implementation)
121+
- [x] Set `sorting_columns` in Parquet file metadata based on sort schema — PR #6287 (`sorting_columns()` method in `writer.rs`)
122+
- [x] Write `sort_schema`, `schema_column_min_values`, `schema_column_max_values`, `schema_column_regexes`, `window_start`, `window_duration_secs` to Parquet `key_value_metadata` — PR #6292 (`qh.sort_fields`, `qh.row_keys`, `qh.row_keys_json`, `qh.window_start`, `qh.window_duration_secs`), PR #6295 (`qh.zonemap_regexes`)
123+
- [x] Compute per-column min/max during split writing (scan sort + metadata-only columns) — PR #6292 (`extract_row_keys()` reads first/last row of sorted batch)
124+
- [x] Compute per-column regex during split writing (follow Husky implementation) — PR #6295 (`extract_zonemap_regexes()` with ported Go DFA automaton)
124125

125126
## References
126127

quickwit/quickwit-parquet-engine/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ ulid = { workspace = true }
3030

3131
[dev-dependencies]
3232
proptest = { workspace = true }
33+
regex = { workspace = true }
3334
tempfile = { workspace = true }
3435

3536
[features]

quickwit/quickwit-parquet-engine/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub mod split;
3131
pub mod storage;
3232
pub mod table_config;
3333
pub mod timeseries_id;
34+
pub mod zonemap;
3435

3536
#[cfg(any(test, feature = "testsuite"))]
3637
pub mod test_helpers;

quickwit/quickwit-parquet-engine/src/row_keys/tests.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ fn test_row_keys_in_parquet_kv_metadata() {
414414

415415
let temp_dir = std::env::temp_dir();
416416
let path = temp_dir.join("test_row_keys_kv.parquet");
417-
let (_, row_keys_proto) = writer
417+
let (_, (row_keys_proto, _zonemap_regexes)) = writer
418418
.write_to_file_with_metadata(&batch, &path, None)
419419
.unwrap();
420420

@@ -687,21 +687,10 @@ fn test_row_keys_consistent_across_all_storage_paths() {
687687
// -- Min row (first after sort): cpu.usage / api / ... --
688688
assert_eq!(col_string(&min_cols[0]), "cpu.usage", "min metric_name");
689689
// service: "api" is the smallest non-null service value.
690-
// Whether null sorts before "api" depends on nulls_first — the writer
691-
// uses nulls_first=true, so the null-service row (cpu.usage/null)
692-
// sorts before (cpu.usage/api). Let's check.
693-
// Actually with nulls_first=true, null < "api", so the first row
694-
// after sort is cpu.usage with service=null.
695-
//
696-
// But ColumnValue for null is { value: None }, so let's handle both.
697-
// The key question: does the row with service=null sort before service="api"?
698-
// With nulls_first=true: yes.
699-
if min_cols[1].value.is_none() {
700-
// First sorted row has null service (cpu.usage with no service).
701-
// This is correct with nulls_first=true.
702-
} else {
703-
assert_eq!(col_string(&min_cols[1]), "api", "min service");
704-
}
690+
// The writer uses nulls_first=false (nulls sort last), so null-service
691+
// rows sort after all non-null values. The first sorted row for
692+
// cpu.usage has service="api".
693+
assert_eq!(col_string(&min_cols[1]), "api", "min service");
705694

706695
// timeseries_id at index 6 — should be an integer.
707696
assert!(

quickwit/quickwit-parquet-engine/src/storage/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,6 @@ mod writer;
2020

2121
pub use config::{Compression, ParquetWriterConfig};
2222
pub use split_writer::ParquetSplitWriter;
23+
#[cfg(test)]
24+
pub(crate) use writer::PARQUET_META_ZONEMAP_REGEXES;
2325
pub use writer::{ParquetWriteError, ParquetWriter};

quickwit/quickwit-parquet-engine/src/storage/split_writer.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,15 @@ impl ParquetSplitWriter {
147147
let mut metadata = builder.build();
148148

149149
// Write with compaction metadata embedded in Parquet KV metadata.
150-
// The writer sorts the batch and extracts RowKeys from the sorted
151-
// first/last rows, returning them alongside the byte count.
152-
let (size_bytes, row_keys_proto) =
153-
self.writer
154-
.write_to_file_with_metadata(batch, &file_path, Some(&metadata))?;
150+
// The writer sorts the batch and extracts RowKeys + zonemap regexes
151+
// from the sorted data, returning them alongside the byte count.
152+
let (size_bytes, (row_keys_proto, zonemap_regexes)) = self
153+
.writer
154+
.write_to_file_with_metadata(batch, &file_path, Some(&metadata))?;
155155

156156
metadata.size_bytes = size_bytes;
157157
metadata.row_keys_proto = row_keys_proto;
158+
metadata.zonemap_regexes = zonemap_regexes;
158159

159160
info!(
160161
split_id = %split_id,

0 commit comments

Comments
 (0)