From d318226b13b265ab3bf7e635ea99a6fa3134972b Mon Sep 17 00:00:00 2001 From: ColinLee Date: Thu, 29 Oct 2026 15:24:38 +0800 Subject: [PATCH 1/2] [C++] Keep scanning after a fully filtered aligned page (#1038) The single-column aligned reader turned a successful decode that produced no rows into E_NO_MORE_DATA. The series scan iterator reads that as "chunk exhausted": it jumps to the next chunk, or ends the series when the current chunk is the last one, so matching pages after a fully filtered page were never read. A TimeIn query over a table with a missing measurement could therefore silently drop rows and still report E_OK. An empty result only means every row of the page was filtered out, so return E_OK and let the iterator's existing empty-block handling continue within the chunk. True end of stream is still signalled by has_more_data(). Add a table-level regression test that reproduces the reported layout (three devices, three chunks, page size 5, a never-written measurement and a missing column on one device) and covers empty pages with and without a successor chunk, plus all-matching and no-matching controls. --- cpp/src/reader/aligned_chunk_reader.cc | 7 +- .../table_view/tsfile_reader_table_test.cc | 109 ++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/cpp/src/reader/aligned_chunk_reader.cc b/cpp/src/reader/aligned_chunk_reader.cc index 38252744b..a11970640 100644 --- a/cpp/src/reader/aligned_chunk_reader.cc +++ b/cpp/src/reader/aligned_chunk_reader.cc @@ -1020,9 +1020,10 @@ int AlignedChunkReader::decode_tv_buf_into_tsblock_by_datatype( ret = E_NOT_SUPPORT; ASSERT(false); } - if (ret_tsblock->get_row_count() == 0 && ret == E_OK) { - ret = E_NO_MORE_DATA; - } + // An empty result only means every row of this page was filtered out; the + // chunk may still hold further pages. Reporting E_NO_MORE_DATA here made + // the series iterator advance past the rest of the chunk (or end the + // series when no successor chunk existed). return ret; } diff --git a/cpp/test/reader/table_view/tsfile_reader_table_test.cc b/cpp/test/reader/table_view/tsfile_reader_table_test.cc index dfbff0576..07be3cd91 100644 --- a/cpp/test/reader/table_view/tsfile_reader_table_test.cc +++ b/cpp/test/reader/table_view/tsfile_reader_table_test.cc @@ -352,6 +352,115 @@ TEST_P(TsFileTableReaderTimeFilterTest, QueryContinuesAfterEmptyPageInChunk) { } } +// Regression for #1038: a page whose rows are all rejected by the time filter +// must not end the series. Requesting the never-written measurement "ghost" +// (and omitting "v" on one device) forces the per-column aligned read path, +// where a decode that produced no rows used to be reported as +// E_NO_MORE_DATA, so the rest of the chunk — including the matching final +// page — was skipped. +TEST_P(TsFileTableReaderTimeFilterTest, + TimeInContinuesAfterFullyFilteredAlignedPage) { + g_config_value_.page_writer_max_point_num_ = 5; + g_config_value_.tsblock_max_memory_ = 4096; + + std::vector columns; + columns.emplace_back("id", STRING, UNCOMPRESSED, PLAIN, + ColumnCategory::TAG); + for (const char* name : {"v", "w", "ghost"}) { + columns.emplace_back(name, INT64, UNCOMPRESSED, PLAIN, + ColumnCategory::FIELD); + } + TableSchema table_schema("t", columns); + TsFileTableWriter writer(&write_file_, &table_schema); + for (int chunk = 0; chunk < 3; ++chunk) { + for (int device = 0; device < 3; ++device) { + const bool has_v = device != 1; + std::vector names{"id", "w"}; + std::vector types{STRING, INT64}; + std::vector categories{ColumnCategory::TAG, + ColumnCategory::FIELD}; + if (has_v) { + names.emplace_back("v"); + types.push_back(INT64); + categories.push_back(ColumnCategory::FIELD); + } + Tablet tablet("t", names, types, categories, 7); + for (int row = 0; row < 7; ++row) { + const int64_t time = chunk * 7 + row; + ASSERT_EQ(tablet.add_timestamp(row, time), common::E_OK); + const std::string id = "d" + std::to_string(device); + ASSERT_EQ(tablet.add_value(row, "id", id.c_str()), + common::E_OK); + if (time % 3 != 0) { + ASSERT_EQ(tablet.add_value(row, "w", + int64_t(device * 1000 + time)), + common::E_OK); + } + if (has_v && time % 4 != 0) { + ASSERT_EQ(tablet.add_value(row, "v", + int64_t(device * 100 + time)), + common::E_OK); + } + } + ASSERT_EQ(writer.write_table(tablet), common::E_OK); + // Each flush ends a chunk; the third one has no successor chunk. + ASSERT_EQ(writer.flush(), common::E_OK); + } + } + ASSERT_EQ(writer.close(), common::E_OK); + + LocalRandomAccessReadFile read_file; + ASSERT_EQ(read_file.open(file_name_), common::E_OK); + // A 4096-byte budget holds the decoder's 129-row batch for these blocks. + TableQueryExecutor executor(&read_file); + const std::vector query_columns{"v", "ghost", "w", "v"}; + auto collect_times = [&](Filter* time_filter, std::vector& times) { + ResultSet* result = nullptr; + ASSERT_EQ(executor.query("t", query_columns, time_filter, nullptr, + nullptr, 0, -1, result), + common::E_OK); + bool has_next = false; + int ret = common::E_OK; + while ((ret = result->next(has_next)) == common::E_OK && has_next) { + times.push_back(result->get_value(1)); + } + EXPECT_EQ(ret, common::E_OK); + executor.destroy_query_data_set(result); + }; + + const std::vector> selected_time_lists{ + // Fully filtered pages in earlier chunks (with successors) and in the + // final chunk followed by the matching page [19, 20]. + {0, 2, 3, 6, 7, 10, 13, 19, 20}, + // Only the matching page of the final chunk is selected. + {19, 20}, + // Matching rows in the first chunk only; every later page is empty. + {0}}; + for (const auto& selected_times : selected_time_lists) { + SCOPED_TRACE(::testing::Message() + << "parallel=" << GetParam() << ", selected_times=" + << ::testing::PrintToString(selected_times)); + std::vector times; + collect_times(new TimeIn(selected_times, false), times); + std::vector expected; + for (int device = 0; device < 3; ++device) { + expected.insert(expected.end(), selected_times.begin(), + selected_times.end()); + } + EXPECT_EQ(times, expected); + } + + // All-matching control: every row of every device is returned. + std::vector times; + collect_times(nullptr, times); + EXPECT_EQ(times.size(), 63u); + + // No-matching control: the series is empty and still terminates. + times.clear(); + collect_times(new TimeIn({100}, false), times); + EXPECT_TRUE(times.empty()); +} + INSTANTIATE_TEST_SUITE_P(SerialAndParallel, TsFileTableReaderTimeFilterTest, ::testing::Bool()); From 7526b0033ebadbf4b47da5632230a446d9464531 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Thu, 29 Oct 2026 17:38:59 +0800 Subject: [PATCH 2/2] [C++] Link the coverage runtime into the test executable TsFile_Test is compiled with -fprofile-arcs/-ftest-coverage, but #1016 changed libtsfile's coverage runtime dependency to PRIVATE, so it no longer propagated to consumers. The Release LTO link then failed with undefined __gcov_init/__gcov_exit/__gcov_merge_add, which has kept the codecov job red since 2026-10-16. Link the coverage runtime into TsFile_Test itself. --- cpp/test/CMakeLists.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cpp/test/CMakeLists.txt b/cpp/test/CMakeLists.txt index fa160bc61..a9e1fa439 100644 --- a/cpp/test/CMakeLists.txt +++ b/cpp/test/CMakeLists.txt @@ -283,6 +283,15 @@ endif () if (${COV_ENABLED}) message("Enable code cov...") add_compile_options(-fprofile-arcs -ftest-coverage) + # This executable is instrumented too, so it needs the coverage runtime on + # its own link line. libtsfile links it PRIVATE, which does not propagate + # to consumers, and the linker does not search a shared library's own + # dependencies for these symbols. + if (APPLE) + set(TSFILE_TEST_COV_LINK_LIB --coverage) + else () + set(TSFILE_TEST_COV_LINK_LIB gcov) + endif () endif () add_executable(TsFile_Test ${TEST_SRCS}) @@ -333,6 +342,9 @@ endif () if (BUILD_TOOLS) target_link_libraries(TsFile_Test tsfile_cli_obj) endif () +if (${COV_ENABLED}) + target_link_libraries(TsFile_Test ${TSFILE_TEST_COV_LINK_LIB}) +endif () set_target_properties(TsFile_Test PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${LIB_TSFILE_SDK_DIR})