Repository navigation
fix(cpp): preserve table time statistics in filtered metadata - #989
Merged
ColinLeeo merged 4 commits intoOct 6, 2026
Merged
Conversation
ColinLeeo
added a commit
to ColinLeeo/tsfile
that referenced
this pull request
Oct 5, 2026
Revert the statistics changes from dbccee3, including their later formatting, now maintained in apache#989. Keep PR apache#970 focused on table read error propagation and preserve its subsequent error handling changes. Validation: Release build, 136 reader/CLI/statistics/golden/read-failure tests, targeted Spotless, and whitespace checks passed.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unreliable zero-count timelines can still bypass scanning and produce negative null counts, and the fallback branch lacks effective regression coverage.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Preserves aligned time statistics in filtered C++ metadata so CLI table statistics can report numeric null counts.
Changes:
- Reuses offset-based metadata loading to retain shared time indexes.
- Adds scan fallback logic for missing timelines.
- Updates regression tests and golden outputs.
| File | Description |
|---|---|
cpp/src/file/tsfile_io_reader.cc |
Preserves aligned time metadata. |
cpp/tools/commands/cmd_stats.cc |
Expands scan fallback conditions. |
cpp/test/reader/tsfile_reader_test.cc |
Verifies filtered time indexes. |
cpp/test/tools/command_e2e_test.cc |
Adds sparse-field coverage. |
cpp/test/tools/golden/table_stats_csv.txt |
Updates CSV null counts. |
cpp/test/tools/golden/table_stats_ndjson.txt |
Updates NDJSON null counts. |
cpp/test/tools/golden/table_stats_table.txt |
Updates table null counts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Filtered table metadata discarded the shared time index, so
tsfile-cli statscould report an emptynull_countfor a normally written table even though its footer contained the entity row count. Reuse the existing offset-based metadata reader to retain the aligned time index. The existing statistics calculation can then subtract each field's non-null count from the footer's row count.Regression coverage verifies that filtered metadata retains the shared time statistic and checks complete CSV output for normally written sparse fields, including both zero and nonzero null counts. CSV, NDJSON, and table golden outputs are updated accordingly. The production change is confined to the metadata reader; CLI scan conditions remain at their baseline behavior.
Validation:
TsFile_Testandtsfile_cliin an isolated Release build with LZ4 enabled and the other optional codecs and ANTLR4 disabled.