feat: extract and populate RowKeys from sorted batches#6292
Merged
Conversation
alanfgates
approved these changes
Apr 13, 2026
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b444d2da2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
mattmkim
force-pushed
the
gtt/sorted-series-key
branch
from
652d128 to
069a5a1
Compare
g-talbot
force-pushed
the
gtt/sorted-series-key
branch
from
069a5a1 to
7444e26
Compare
g-talbot
force-pushed
the
gtt/row-keys-extraction
branch
10 times, most recently
from
7c7beb1 to
edb1aec
Compare
Compute sort-key boundaries (first/last row values) during Parquet write and store them as RowKeys proto in both the Parquet KV metadata and MetricsSplitMetadata. The compactor uses these ranges to determine key-range overlap between splits and to select merge boundaries. - New row_keys module: extract_row_keys() reads sort schema column values at rows 0 and N-1, mapping Arrow types to ColumnValue proto - prepare_write() now computes row_keys after sorting and injects qh.row_keys (base64) + qh.row_keys_json into Parquet KV metadata - write_to_bytes/write_to_file_with_metadata return row_keys proto bytes alongside the write result - split_writer captures and stores row_keys_proto on metadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
End-to-end test with 8 rows across 4 series (including null tags, mixed metrics, wide timestamp range). Writes through the full split_writer pipeline and verifies the RowKeys proto is byte-identical in all three places: 1. MetricsSplitMetadata.row_keys_proto (feeds Postgres) 2. Parquet file KV metadata (qh.row_keys, base64-encoded) 3. InsertableMetricsSplit.row_keys (Postgres column) Also checks: min/max column values match expected sort order, all_inclusive_max equals max, JSON metadata is parseable, and the split is not marked expired. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…data API - Keep DDSketch sketch validation in prepare_write - Fix ParquetSplitWriter::new to pass ParquetSplitKind - Remove Postgres path test (InsertableParquetSplit moved to quickwit-metastore) - Update MetricsSplitMetadata references to ParquetSplitMetadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…row_keys tests These constructors now return Result after the sorted-series-key merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…raction The sort schema parser accepts "timestamp" as a legacy alias, but the Arrow batch only contains "timestamp_secs". Without normalization, index_of fails and the code encodes None for the timestamp boundary, losing time range information from RowKeys. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
build_compaction_key_value_metadata already emits qh.row_keys when split_metadata.row_keys_proto is present. prepare_write then appended another entry from freshly computed values, producing duplicates. Now strips any pre-existing qh.row_keys and qh.row_keys_json before pushing the authoritative values from the sorted batch. Also adds a test assertion verifying exactly one qh.row_keys entry exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
g-talbot
force-pushed
the
gtt/row-keys-extraction
branch
from
df99c89 to
c6d1681
Compare
mattmkim
reviewed
Apr 21, 2026
mattmkim
approved these changes
Apr 21, 2026
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
3 participants
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
row_keysmodule:extract_row_keys()reads sort schema column values at the first and last rows of a sorted batch, building asortschema::RowKeysproto withmin_row_values,max_row_values, andall_inclusive_max_row_valuesprepare_write()now computes RowKeys after sorting and injectsqh.row_keys(base64) +qh.row_keys_jsoninto Parquet KV metadatawrite_to_bytes/write_to_file_with_metadatareturn the RowKeys proto bytes alongside the write resultsplit_writercaptures the returned bytes and stores them onMetricsSplitMetadata.row_keys_protoDictionary(Int32,Utf8)/Utf8→TypeString,Int64→TypeInt,UInt64→TypeInt(cast),Float64→TypeFloat; null values →ColumnValue { value: None }Stacked on #6290 (sorted_series column).
Test plan
quickwit-parquet-enginetests pass🤖 Generated with Claude Code