From 1a74cf3a3ae0da0cc6139cad93d2bf37f046898e Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 5 Oct 2026 15:52:24 +0800 Subject: [PATCH 1/4] fix(cpp): preserve table time statistics in filtered metadata --- cpp/src/file/tsfile_io_reader.cc | 28 +++++------- cpp/test/reader/tsfile_reader_test.cc | 7 +++ cpp/test/tools/command_e2e_test.cc | 48 +++++++++++++++++++- cpp/test/tools/golden/table_stats_csv.txt | 2 +- cpp/test/tools/golden/table_stats_ndjson.txt | 2 +- cpp/test/tools/golden/table_stats_table.txt | 2 +- cpp/tools/commands/cmd_stats.cc | 17 +++++-- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/cpp/src/file/tsfile_io_reader.cc b/cpp/src/file/tsfile_io_reader.cc index 9b4d8aef4..17e07459c 100644 --- a/cpp/src/file/tsfile_io_reader.cc +++ b/cpp/src/file/tsfile_io_reader.cc @@ -393,24 +393,20 @@ void TsFileIOReader::revert_ssi(TsFileSeriesScanIterator* ssi) { int TsFileIOReader::get_device_timeseries_meta_without_chunk_meta( std::shared_ptr device_id, std::vector& timeseries_indexs, PageArena& pa) { - int ret = E_OK; - if (RET_FAIL(load_tsfile_meta_if_necessary())) { - return ret; - } + int ret = load_tsfile_meta_if_necessary(); + if (RET_FAIL(ret)) return ret; std::shared_ptr meta_index_entry; int64_t end_offset; - std::vector, int64_t>> - meta_index_entry_list; - if (RET_FAIL(load_device_index_entry( - std::make_shared(device_id), meta_index_entry, - end_offset))) { - } else if (RET_FAIL(load_all_measurement_index_entry( - meta_index_entry->get_offset(), end_offset, pa, - meta_index_entry_list))) { - } else if (RET_FAIL(do_load_all_timeseries_index(meta_index_entry_list, pa, - timeseries_indexs))) { - } - return ret; + ret = + load_device_index_entry(std::make_shared(device_id), + meta_index_entry, end_offset); + if (RET_FAIL(ret)) return ret; + + // Reuse the offset-based path so filtered metadata keeps the aligned time + // index alongside each value index. Without this wrapping, table stats + // cannot read the footer's time statistic and fall back to a full scan. + return get_device_timeseries_meta_by_offset( + meta_index_entry->get_offset(), end_offset, timeseries_indexs, pa); } int TsFileIOReader::get_device_timeseries_meta_by_offset( diff --git a/cpp/test/reader/tsfile_reader_test.cc b/cpp/test/reader/tsfile_reader_test.cc index 314d69c6d..a9ae1f84a 100644 --- a/cpp/test/reader/tsfile_reader_test.cc +++ b/cpp/test/reader/tsfile_reader_test.cc @@ -826,6 +826,13 @@ TEST_F(TsFileReaderTest, GetTimeseriesMetadataTableModelTypeAndDeviceFilter) { auto selected_list = selected_meta.begin()->second; std::unordered_map type_by_measurement; for (const auto& index : selected_list) { + auto* aligned = + dynamic_cast(index.get()); + ASSERT_NE(aligned, nullptr) + << "filtered metadata must preserve the shared time index"; + ASSERT_NE(aligned->time_ts_idx_, nullptr); + ASSERT_NE(aligned->time_ts_idx_->get_statistic(), nullptr); + EXPECT_EQ(aligned->time_ts_idx_->get_statistic()->get_count(), 5); type_by_measurement[index->get_measurement_name().to_std_string()] = index->get_data_type(); } diff --git a/cpp/test/tools/command_e2e_test.cc b/cpp/test/tools/command_e2e_test.cc index 083000991..a735f78d1 100644 --- a/cpp/test/tools/command_e2e_test.cc +++ b/cpp/test/tools/command_e2e_test.cc @@ -165,11 +165,55 @@ TEST(CliE2E, StatsReportsCountAndTimeRange) { std::string::npos) << out.str(); EXPECT_NE(out.str().find("table,table1,id1_field_1,id2_field_2,s1,INT64," - "5,\\N,0,4,0,40,0,40,\\N,statistics"), + "5,0,0,4,0,40,0,40,\\N,statistics"), std::string::npos) << out.str(); } +TEST(CliE2E, StatsReportsNumericNullCountForTableFields) { + // A FIELD whose value statistic is present but whose entity timeline + // statistic is unavailable must still get a numeric null_count by + // scanning (TsFile-143): null_count previously stayed empty even though + // the field's non-null count was known. + const std::string csv_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".csv"); + const std::string out_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".tsfile"); + { + std::ofstream csv(csv_path.c_str(), std::ios::binary); + ASSERT_TRUE(csv.good()); + csv << "time,site,temp,note\n" + "1000,beijing,20,n0\n" + "2000,shanghai,21,n1\n" + "3000,beijing,22,\\N\n" + "4000,shanghai,23,n3\n"; + } + std::ostringstream wout, werr; + ASSERT_EQ( + tsfile_cli::run_cli({"write", "--table", "sensors", "--tag", "site", + "STRING", "--field", "temp", "FLOAT", "--field", + "note", "TEXT", "-i", csv_path, "-o", out_path}, + wout, werr), + 0) + << werr.str(); + + std::ostringstream out, err; + ASSERT_EQ(tsfile_cli::run_cli({"stats", "-t", "sensors", "-m", "note", "-f", + "csv", out_path}, + out, err), + 0) + << err.str(); + EXPECT_NE(out.str().find("table,sensors,beijing,note,TEXT,1,1,1000,1000"), + std::string::npos) + << out.str(); + EXPECT_NE(out.str().find("table,sensors,shanghai,note,TEXT,2,0,2000,4000"), + std::string::npos) + << out.str(); + + std::remove(csv_path.c_str()); + std::remove(out_path.c_str()); +} + TEST(CliE2E, HeadProjectsAndLimits) { Fixture f; std::ostringstream out; @@ -692,7 +736,7 @@ TEST(CliE2E, MetadataTableFilterIsCaseInsensitive) { stats_out, stats_err), 0); EXPECT_NE(stats_out.str().find("table,table1,id1_field_1,id2_field_2,s1," - "INT64,5,\\N,0,4,0,40,0,40,\\N,statistics"), + "INT64,5,0,0,4,0,40,0,40,\\N,statistics"), std::string::npos) << stats_out.str(); } diff --git a/cpp/test/tools/golden/table_stats_csv.txt b/cpp/test/tools/golden/table_stats_csv.txt index d25d0085f..6c539d5dc 100644 --- a/cpp/test/tools/golden/table_stats_csv.txt +++ b/cpp/test/tools/golden/table_stats_csv.txt @@ -1,5 +1,5 @@ model,object,tag.site,tag.rack,tag.sensor,field,data_type,non_null_count,null_count,min_time,max_time,min,max,first,last,sum,stats_source -table,table1,\N,\N,\N,value,INT64,20,\N,0,19,0,19,0,19,\N,statistics +table,table1,\N,\N,\N,value,INT64,20,0,0,19,0,19,0,19,\N,statistics table,table2,\N,rack0,sensor0,reading,DOUBLE,0,1,0,0,\N,\N,\N,\N,\N,scan table,table2,"",rack1,sensor1,reading,DOUBLE,1,0,1,1,1.25,1.25,1.25,1.25,1.25,statistics table,table2,null,rack2,sensor2,reading,DOUBLE,1,0,2,2,2.5,2.5,2.5,2.5,2.5,statistics diff --git a/cpp/test/tools/golden/table_stats_ndjson.txt b/cpp/test/tools/golden/table_stats_ndjson.txt index 052a7b3ae..82f165d19 100644 --- a/cpp/test/tools/golden/table_stats_ndjson.txt +++ b/cpp/test/tools/golden/table_stats_ndjson.txt @@ -1,4 +1,4 @@ -{"model":"table","object":"table1","tag.site":null,"tag.rack":null,"tag.sensor":null,"field":"value","data_type":"INT64","non_null_count":"20","null_count":null,"min_time":"0","max_time":"19","min":"0","max":"19","first":"0","last":"19","sum":null,"stats_source":"statistics"} +{"model":"table","object":"table1","tag.site":null,"tag.rack":null,"tag.sensor":null,"field":"value","data_type":"INT64","non_null_count":"20","null_count":"0","min_time":"0","max_time":"19","min":"0","max":"19","first":"0","last":"19","sum":null,"stats_source":"statistics"} {"model":"table","object":"table2","tag.site":null,"tag.rack":"rack0","tag.sensor":"sensor0","field":"reading","data_type":"DOUBLE","non_null_count":"0","null_count":"1","min_time":"0","max_time":"0","min":null,"max":null,"first":null,"last":null,"sum":null,"stats_source":"scan"} {"model":"table","object":"table2","tag.site":"","tag.rack":"rack1","tag.sensor":"sensor1","field":"reading","data_type":"DOUBLE","non_null_count":"1","null_count":"0","min_time":"1","max_time":"1","min":1.25,"max":1.25,"first":1.25,"last":1.25,"sum":1.25,"stats_source":"statistics"} {"model":"table","object":"table2","tag.site":"null","tag.rack":"rack2","tag.sensor":"sensor2","field":"reading","data_type":"DOUBLE","non_null_count":"1","null_count":"0","min_time":"2","max_time":"2","min":2.5,"max":2.5,"first":2.5,"last":2.5,"sum":2.5,"stats_source":"statistics"} diff --git a/cpp/test/tools/golden/table_stats_table.txt b/cpp/test/tools/golden/table_stats_table.txt index e0d6d4207..e5eb67371 100644 --- a/cpp/test/tools/golden/table_stats_table.txt +++ b/cpp/test/tools/golden/table_stats_table.txt @@ -1,5 +1,5 @@ model object tag.site tag.rack tag.sensor field data_type non_null_count null_count min_time max_time min max first last sum stats_source -table table1 value INT64 20 0 19 0 19 0 19 statistics +table table1 value INT64 20 0 0 19 0 19 0 19 statistics table table2 rack0 sensor0 reading DOUBLE 0 1 0 0 scan table table2 rack1 sensor1 reading DOUBLE 1 0 1 1 1.25 1.25 1.25 1.25 1.25 statistics table table2 null rack2 sensor2 reading DOUBLE 1 0 2 2 2.5 2.5 2.5 2.5 2.5 statistics diff --git a/cpp/tools/commands/cmd_stats.cc b/cpp/tools/commands/cmd_stats.cc index 01bd5ba57..76207c97f 100644 --- a/cpp/tools/commands/cmd_stats.cc +++ b/cpp/tools/commands/cmd_stats.cc @@ -371,20 +371,29 @@ int collect_table_stats(const ParsedArgs& args, summary.entity_order.push_back(key); } - bool missing_value_statistics = false; + // A row count is required to derive a numeric null_count. It comes + // either from the entity timeline statistic or, when that statistic is + // absent/unreliable, from a row scan. Trigger the scan whenever any + // entity still lacks a timeline (even if every selected field carries a + // value statistic), otherwise null_count would stay empty. + bool need_scan = false; for (const std::string& key : summary.entity_order) { const auto it = summary.entities.find(key); if (it == summary.entities.end()) continue; + if (!it->second.has_timeline_statistic) { + need_scan = true; + break; + } for (size_t index : summary.field_indexes) { const std::string& field = summary.columns[index].name; if (it->second.fields.find(field) == it->second.fields.end()) { - missing_value_statistics = true; + need_scan = true; break; } } - if (missing_value_statistics) break; + if (need_scan) break; } - if (missing_value_statistics && !devices.empty()) { + if (need_scan && !devices.empty()) { std::vector query_columns; std::vector tag_result_indexes; for (size_t i = 0; i < measurements.size(); ++i) { From 877386907abaa2607860085d38f9367859151ddd Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 5 Oct 2026 20:08:12 +0800 Subject: [PATCH 2/4] style(cpp): preserve existing metadata reader error handling --- cpp/src/file/tsfile_io_reader.cc | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/cpp/src/file/tsfile_io_reader.cc b/cpp/src/file/tsfile_io_reader.cc index 17e07459c..37f1ea1d2 100644 --- a/cpp/src/file/tsfile_io_reader.cc +++ b/cpp/src/file/tsfile_io_reader.cc @@ -393,20 +393,23 @@ void TsFileIOReader::revert_ssi(TsFileSeriesScanIterator* ssi) { int TsFileIOReader::get_device_timeseries_meta_without_chunk_meta( std::shared_ptr device_id, std::vector& timeseries_indexs, PageArena& pa) { - int ret = load_tsfile_meta_if_necessary(); - if (RET_FAIL(ret)) return ret; + int ret = E_OK; + if (RET_FAIL(load_tsfile_meta_if_necessary())) { + return ret; + } std::shared_ptr meta_index_entry; int64_t end_offset; - ret = - load_device_index_entry(std::make_shared(device_id), - meta_index_entry, end_offset); - if (RET_FAIL(ret)) return ret; - // Reuse the offset-based path so filtered metadata keeps the aligned time // index alongside each value index. Without this wrapping, table stats // cannot read the footer's time statistic and fall back to a full scan. - return get_device_timeseries_meta_by_offset( - meta_index_entry->get_offset(), end_offset, timeseries_indexs, pa); + if (RET_FAIL(load_device_index_entry( + std::make_shared(device_id), meta_index_entry, + end_offset))) { + } else if (RET_FAIL(get_device_timeseries_meta_by_offset( + meta_index_entry->get_offset(), end_offset, + timeseries_indexs, pa))) { + } + return ret; } int TsFileIOReader::get_device_timeseries_meta_by_offset( From 447ba3e7b159f8353c8dbd3cc312dcfe095f9a6d Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 5 Oct 2026 21:40:11 +0800 Subject: [PATCH 3/4] fix(cpp): scan table statistics with inconsistent timeline counts --- cpp/src/file/tsfile_io_reader.cc | 23 +++++++ cpp/test/tools/command_e2e_test.cc | 101 ++++++++++++++++++++++++----- cpp/tools/commands/cmd_stats.cc | 10 +-- 3 files changed, 112 insertions(+), 22 deletions(-) diff --git a/cpp/src/file/tsfile_io_reader.cc b/cpp/src/file/tsfile_io_reader.cc index 37f1ea1d2..ab96a4a53 100644 --- a/cpp/src/file/tsfile_io_reader.cc +++ b/cpp/src/file/tsfile_io_reader.cc @@ -24,6 +24,9 @@ #include "common/allocator/alloc_base.h" #include "file/local_random_access_read_file.h" #include "reader/prepared_series.h" +#ifdef ENABLE_TEST +#include "utils/injection.h" +#endif using namespace common; @@ -409,6 +412,26 @@ int TsFileIOReader::get_device_timeseries_meta_without_chunk_meta( meta_index_entry->get_offset(), end_offset, timeseries_indexs, pa))) { } +#ifdef ENABLE_TEST + // Simulate incomplete footer metadata without altering the query path. + if (IS_SUCC(ret)) { + for (ITimeseriesIndex*& index : timeseries_indexs) { + auto* aligned = dynamic_cast(index); + if (aligned == nullptr || aligned->time_ts_idx_ == nullptr) { + continue; + } + DBUG_EXECUTE_IF("table_metadata_missing_timeline", + index = aligned->value_ts_idx_;); + Statistic* statistic = aligned->time_ts_idx_->get_statistic(); + if (statistic != nullptr) { + DBUG_EXECUTE_IF("table_metadata_zero_timeline", + statistic->count_ = 0;); + DBUG_EXECUTE_IF("table_metadata_short_timeline", + statistic->count_ = 1;); + } + } + } +#endif return ret; } diff --git a/cpp/test/tools/command_e2e_test.cc b/cpp/test/tools/command_e2e_test.cc index a735f78d1..7d84485bf 100644 --- a/cpp/test/tools/command_e2e_test.cc +++ b/cpp/test/tools/command_e2e_test.cc @@ -26,6 +26,7 @@ #include "cli/run_cli.h" #include "cli_test_util.h" +#include "utils/injection.h" namespace { @@ -59,6 +60,32 @@ struct DisjointTreeFixture { ~DisjointTreeFixture() { std::remove(path.c_str()); } }; +class InjectionGuard { + public: + explicit InjectionGuard(const char* point) : point_(point) { + if (point_ != nullptr) common::enable_injection(point_, 0); + } + ~InjectionGuard() { + if (point_ != nullptr) common::disable_injection(point_); + } + + private: + const char* point_; +}; + +class TableStatsTimelineTest : public ::testing::TestWithParam { + protected: + std::string csv_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".csv"); + std::string out_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".tsfile"); + + void TearDown() override { + std::remove(csv_path.c_str()); + std::remove(out_path.c_str()); + } +}; + class FlushFailingStreamBuf : public std::stringbuf { protected: int sync() override { return -1; } @@ -170,15 +197,10 @@ TEST(CliE2E, StatsReportsCountAndTimeRange) { << out.str(); } -TEST(CliE2E, StatsReportsNumericNullCountForTableFields) { - // A FIELD whose value statistic is present but whose entity timeline - // statistic is unavailable must still get a numeric null_count by - // scanning (TsFile-143): null_count previously stayed empty even though - // the field's non-null count was known. - const std::string csv_path = - tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".csv"); - const std::string out_path = - tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".tsfile"); +TEST_P(TableStatsTimelineTest, StatsReportsNumericNullCountForTableFields) { + // -1 removes the timeline; 0 and 1 undercount its two rows. In each case, + // the field statistics remain available and scanning must recover the + // actual row counts. A count of 2 exercises the intact footer path. { std::ofstream csv(csv_path.c_str(), std::ios::binary); ASSERT_TRUE(csv.good()); @@ -197,23 +219,66 @@ TEST(CliE2E, StatsReportsNumericNullCountForTableFields) { 0) << werr.str(); + const char* injection = nullptr; + if (GetParam() == -1) { + injection = "table_metadata_missing_timeline"; + } else if (GetParam() == 0) { + injection = "table_metadata_zero_timeline"; + } else if (GetParam() == 1) { + injection = "table_metadata_short_timeline"; + } + InjectionGuard guard(injection); + { + storage::TsFileReader reader; + ASSERT_EQ(reader.open(out_path), common::E_OK); + auto metadata = + reader.get_timeseries_metadata(reader.get_all_devices("sensors")); + ASSERT_EQ(metadata.size(), 2); + for (const auto& device : metadata) { + ASSERT_EQ(device.second.size(), 2); + for (const auto& index : device.second) { + auto* aligned = + dynamic_cast(index.get()); + storage::ITimeseriesIndex* value_index = index.get(); + if (GetParam() == -1) { + ASSERT_EQ(aligned, nullptr); + } else { + ASSERT_NE(aligned, nullptr); + ASSERT_NE(aligned->time_ts_idx_, nullptr); + ASSERT_NE(aligned->time_ts_idx_->get_statistic(), nullptr); + ASSERT_EQ( + aligned->time_ts_idx_->get_statistic()->get_count(), + GetParam()); + value_index = aligned->value_ts_idx_; + } + ASSERT_NE(value_index, nullptr); + ASSERT_NE(value_index->get_statistic(), nullptr); + ASSERT_GT(value_index->get_statistic()->get_count(), 0); + } + } + } + std::ostringstream out, err; ASSERT_EQ(tsfile_cli::run_cli({"stats", "-t", "sensors", "-m", "note", "-f", "csv", out_path}, out, err), 0) << err.str(); - EXPECT_NE(out.str().find("table,sensors,beijing,note,TEXT,1,1,1000,1000"), - std::string::npos) - << out.str(); - EXPECT_NE(out.str().find("table,sensors,shanghai,note,TEXT,2,0,2000,4000"), - std::string::npos) - << out.str(); - - std::remove(csv_path.c_str()); - std::remove(out_path.c_str()); + EXPECT_TRUE(err.str().empty()) << err.str(); + // Value aggregates still come from statistics; only row_count falls back + // to scanning. The sparse field must retain a positive null_count. + EXPECT_EQ(out.str(), + "model,object,tag.site,field,data_type,non_null_count,null_count," + "min_time,max_time,min,max,first,last,sum,stats_source\n" + "table,sensors,beijing,note,TEXT,1,1,1000,1000,\\N,\\N,n0,n0," + "\\N,statistics\n" + "table,sensors,shanghai,note,TEXT,2,0,2000,4000,\\N,\\N,n1,n3," + "\\N,statistics\n"); } +INSTANTIATE_TEST_SUITE_P(TimelineMetadata, TableStatsTimelineTest, + ::testing::Values(-1, 0, 1, 2)); + TEST(CliE2E, HeadProjectsAndLimits) { Fixture f; std::ostringstream out; diff --git a/cpp/tools/commands/cmd_stats.cc b/cpp/tools/commands/cmd_stats.cc index 76207c97f..a5abacea1 100644 --- a/cpp/tools/commands/cmd_stats.cc +++ b/cpp/tools/commands/cmd_stats.cc @@ -373,9 +373,9 @@ int collect_table_stats(const ParsedArgs& args, // A row count is required to derive a numeric null_count. It comes // either from the entity timeline statistic or, when that statistic is - // absent/unreliable, from a row scan. Trigger the scan whenever any - // entity still lacks a timeline (even if every selected field carries a - // value statistic), otherwise null_count would stay empty. + // absent/unreliable, from a row scan. Scan when a timeline is missing + // or its row count is below a selected field's non-null count, even if + // every selected field carries a value statistic. bool need_scan = false; for (const std::string& key : summary.entity_order) { const auto it = summary.entities.find(key); @@ -386,7 +386,9 @@ int collect_table_stats(const ParsedArgs& args, } for (size_t index : summary.field_indexes) { const std::string& field = summary.columns[index].name; - if (it->second.fields.find(field) == it->second.fields.end()) { + const auto field_it = it->second.fields.find(field); + if (field_it == it->second.fields.end() || + it->second.row_count < field_it->second.count) { need_scan = true; break; } From 4c5042f3a7f0b3b6537d4f6cfb2dd75cd6426740 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Tue, 6 Oct 2026 11:03:11 +0800 Subject: [PATCH 4/4] fix(cpp): scope table statistics fix to footer metadata --- cpp/src/file/tsfile_io_reader.cc | 28 +--------- cpp/test/tools/command_e2e_test.cc | 85 ++++-------------------------- cpp/tools/commands/cmd_stats.cc | 21 ++------ 3 files changed, 17 insertions(+), 117 deletions(-) diff --git a/cpp/src/file/tsfile_io_reader.cc b/cpp/src/file/tsfile_io_reader.cc index ab96a4a53..49f5fd9f8 100644 --- a/cpp/src/file/tsfile_io_reader.cc +++ b/cpp/src/file/tsfile_io_reader.cc @@ -24,9 +24,6 @@ #include "common/allocator/alloc_base.h" #include "file/local_random_access_read_file.h" #include "reader/prepared_series.h" -#ifdef ENABLE_TEST -#include "utils/injection.h" -#endif using namespace common; @@ -402,9 +399,8 @@ int TsFileIOReader::get_device_timeseries_meta_without_chunk_meta( } std::shared_ptr meta_index_entry; int64_t end_offset; - // Reuse the offset-based path so filtered metadata keeps the aligned time - // index alongside each value index. Without this wrapping, table stats - // cannot read the footer's time statistic and fall back to a full scan. + // Keep the shared time index in filtered metadata so table statistics + // can derive null counts from the footer. if (RET_FAIL(load_device_index_entry( std::make_shared(device_id), meta_index_entry, end_offset))) { @@ -412,26 +408,6 @@ int TsFileIOReader::get_device_timeseries_meta_without_chunk_meta( meta_index_entry->get_offset(), end_offset, timeseries_indexs, pa))) { } -#ifdef ENABLE_TEST - // Simulate incomplete footer metadata without altering the query path. - if (IS_SUCC(ret)) { - for (ITimeseriesIndex*& index : timeseries_indexs) { - auto* aligned = dynamic_cast(index); - if (aligned == nullptr || aligned->time_ts_idx_ == nullptr) { - continue; - } - DBUG_EXECUTE_IF("table_metadata_missing_timeline", - index = aligned->value_ts_idx_;); - Statistic* statistic = aligned->time_ts_idx_->get_statistic(); - if (statistic != nullptr) { - DBUG_EXECUTE_IF("table_metadata_zero_timeline", - statistic->count_ = 0;); - DBUG_EXECUTE_IF("table_metadata_short_timeline", - statistic->count_ = 1;); - } - } - } -#endif return ret; } diff --git a/cpp/test/tools/command_e2e_test.cc b/cpp/test/tools/command_e2e_test.cc index 7d84485bf..fae33226d 100644 --- a/cpp/test/tools/command_e2e_test.cc +++ b/cpp/test/tools/command_e2e_test.cc @@ -26,7 +26,6 @@ #include "cli/run_cli.h" #include "cli_test_util.h" -#include "utils/injection.h" namespace { @@ -60,32 +59,6 @@ struct DisjointTreeFixture { ~DisjointTreeFixture() { std::remove(path.c_str()); } }; -class InjectionGuard { - public: - explicit InjectionGuard(const char* point) : point_(point) { - if (point_ != nullptr) common::enable_injection(point_, 0); - } - ~InjectionGuard() { - if (point_ != nullptr) common::disable_injection(point_); - } - - private: - const char* point_; -}; - -class TableStatsTimelineTest : public ::testing::TestWithParam { - protected: - std::string csv_path = - tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".csv"); - std::string out_path = - tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".tsfile"); - - void TearDown() override { - std::remove(csv_path.c_str()); - std::remove(out_path.c_str()); - } -}; - class FlushFailingStreamBuf : public std::stringbuf { protected: int sync() override { return -1; } @@ -197,10 +170,13 @@ TEST(CliE2E, StatsReportsCountAndTimeRange) { << out.str(); } -TEST_P(TableStatsTimelineTest, StatsReportsNumericNullCountForTableFields) { - // -1 removes the timeline; 0 and 1 undercount its two rows. In each case, - // the field statistics remain available and scanning must recover the - // actual row counts. A count of 2 exercises the intact footer path. +TEST(CliE2E, StatsReportsNumericNullCountForTableFields) { + // Filtered metadata must preserve the shared time statistic so the + // footer's row count and each field's non-null count determine null_count. + const std::string csv_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".csv"); + const std::string out_path = + tsfile_cli_test::unique_temp_path("tsfile_cli_null_count", ".tsfile"); { std::ofstream csv(csv_path.c_str(), std::ios::binary); ASSERT_TRUE(csv.good()); @@ -219,45 +195,6 @@ TEST_P(TableStatsTimelineTest, StatsReportsNumericNullCountForTableFields) { 0) << werr.str(); - const char* injection = nullptr; - if (GetParam() == -1) { - injection = "table_metadata_missing_timeline"; - } else if (GetParam() == 0) { - injection = "table_metadata_zero_timeline"; - } else if (GetParam() == 1) { - injection = "table_metadata_short_timeline"; - } - InjectionGuard guard(injection); - { - storage::TsFileReader reader; - ASSERT_EQ(reader.open(out_path), common::E_OK); - auto metadata = - reader.get_timeseries_metadata(reader.get_all_devices("sensors")); - ASSERT_EQ(metadata.size(), 2); - for (const auto& device : metadata) { - ASSERT_EQ(device.second.size(), 2); - for (const auto& index : device.second) { - auto* aligned = - dynamic_cast(index.get()); - storage::ITimeseriesIndex* value_index = index.get(); - if (GetParam() == -1) { - ASSERT_EQ(aligned, nullptr); - } else { - ASSERT_NE(aligned, nullptr); - ASSERT_NE(aligned->time_ts_idx_, nullptr); - ASSERT_NE(aligned->time_ts_idx_->get_statistic(), nullptr); - ASSERT_EQ( - aligned->time_ts_idx_->get_statistic()->get_count(), - GetParam()); - value_index = aligned->value_ts_idx_; - } - ASSERT_NE(value_index, nullptr); - ASSERT_NE(value_index->get_statistic(), nullptr); - ASSERT_GT(value_index->get_statistic()->get_count(), 0); - } - } - } - std::ostringstream out, err; ASSERT_EQ(tsfile_cli::run_cli({"stats", "-t", "sensors", "-m", "note", "-f", "csv", out_path}, @@ -265,8 +202,6 @@ TEST_P(TableStatsTimelineTest, StatsReportsNumericNullCountForTableFields) { 0) << err.str(); EXPECT_TRUE(err.str().empty()) << err.str(); - // Value aggregates still come from statistics; only row_count falls back - // to scanning. The sparse field must retain a positive null_count. EXPECT_EQ(out.str(), "model,object,tag.site,field,data_type,non_null_count,null_count," "min_time,max_time,min,max,first,last,sum,stats_source\n" @@ -274,10 +209,10 @@ TEST_P(TableStatsTimelineTest, StatsReportsNumericNullCountForTableFields) { "\\N,statistics\n" "table,sensors,shanghai,note,TEXT,2,0,2000,4000,\\N,\\N,n1,n3," "\\N,statistics\n"); -} -INSTANTIATE_TEST_SUITE_P(TimelineMetadata, TableStatsTimelineTest, - ::testing::Values(-1, 0, 1, 2)); + std::remove(csv_path.c_str()); + std::remove(out_path.c_str()); +} TEST(CliE2E, HeadProjectsAndLimits) { Fixture f; diff --git a/cpp/tools/commands/cmd_stats.cc b/cpp/tools/commands/cmd_stats.cc index a5abacea1..01bd5ba57 100644 --- a/cpp/tools/commands/cmd_stats.cc +++ b/cpp/tools/commands/cmd_stats.cc @@ -371,31 +371,20 @@ int collect_table_stats(const ParsedArgs& args, summary.entity_order.push_back(key); } - // A row count is required to derive a numeric null_count. It comes - // either from the entity timeline statistic or, when that statistic is - // absent/unreliable, from a row scan. Scan when a timeline is missing - // or its row count is below a selected field's non-null count, even if - // every selected field carries a value statistic. - bool need_scan = false; + bool missing_value_statistics = false; for (const std::string& key : summary.entity_order) { const auto it = summary.entities.find(key); if (it == summary.entities.end()) continue; - if (!it->second.has_timeline_statistic) { - need_scan = true; - break; - } for (size_t index : summary.field_indexes) { const std::string& field = summary.columns[index].name; - const auto field_it = it->second.fields.find(field); - if (field_it == it->second.fields.end() || - it->second.row_count < field_it->second.count) { - need_scan = true; + if (it->second.fields.find(field) == it->second.fields.end()) { + missing_value_statistics = true; break; } } - if (need_scan) break; + if (missing_value_statistics) break; } - if (need_scan && !devices.empty()) { + if (missing_value_statistics && !devices.empty()) { std::vector query_columns; std::vector tag_result_indexes; for (size_t i = 0; i < measurements.size(); ++i) {