You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tsfile.apache.org by ho...@apache.org on 2026/04/30 07:07:39 UTC
(tsfile) branch fix/cpp-ts2diff created (now 247b4ed4)
This is an automated email from the ASF dual-hosted git repository.
hongzhi-gao pushed a change to branch fix/cpp-ts2diff
in repository https://gitbox.apache.org/repos/asf/tsfile.git
at 247b4ed4 Fix TS_2DIFF float/double page compatibility and harden gtest zip validation.
This branch includes the following new commits:
new 247b4ed4 Fix TS_2DIFF float/double page compatibility and harden gtest zip validation.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(tsfile) 01/01: Fix TS_2DIFF float/double page compatibility and harden gtest zip validation.
Posted by ho...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
hongzhi-gao pushed a commit to branch fix/cpp-ts2diff
in repository https://gitbox.apache.org/repos/asf/tsfile.git
commit 247b4ed4c04314c10f9e32c81c6dea73b0398dc2
Author: 761417898 <76...@qq.com>
AuthorDate: Thu Apr 30 15:07:24 2026 +0800
Fix TS_2DIFF float/double page compatibility and harden gtest zip validation.
Align C++ float/double TS_2DIFF flush/read behavior with Java overflow-page layout, and prevent 0-byte/corrupt googletest archives from being treated as successful downloads during test configuration.
---
cpp/src/common/allocator/byte_stream.h | 15 ++++
cpp/src/encoding/ts2diff_decoder.h | 54 +++++++++++++
cpp/src/encoding/ts2diff_encoder.h | 130 ++++++++++++++++++++++++++++++++
cpp/test/CMakeLists.txt | 73 ++++++++++++++----
cpp/test/encoding/ts2diff_codec_test.cc | 77 +++++++++++++++++++
5 files changed, 336 insertions(+), 13 deletions(-)
diff --git a/cpp/src/common/allocator/byte_stream.h b/cpp/src/common/allocator/byte_stream.h
index 6d08744c..dddc00b8 100644
--- a/cpp/src/common/allocator/byte_stream.h
+++ b/cpp/src/common/allocator/byte_stream.h
@@ -347,6 +347,21 @@ class ByteStream {
FORCE_INLINE uint32_t total_size() const { return total_size_.load(); }
FORCE_INLINE uint32_t read_pos() const { return read_pos_; };
+ /**
+ * Seek the read cursor to an absolute offset. Re-anchors read_page_ for
+ * multi-page streams.
+ */
+ void set_read_pos(uint32_t pos) {
+ ASSERT(pos <= total_size());
+ read_pos_ = pos;
+ Page* p = head_.load();
+ uint32_t skipped = 0;
+ while (p != nullptr && skipped + page_size_ <= pos) {
+ skipped += page_size_;
+ p = p->next_.load();
+ }
+ read_page_ = p;
+ }
FORCE_INLINE void wrapped_buf_advance_read_pos(uint32_t size) {
if (size + read_pos_ > total_size_.load()) {
read_pos_ = total_size_.load();
diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h
index 32584546..258f0433 100644
--- a/cpp/src/encoding/ts2diff_decoder.h
+++ b/cpp/src/encoding/ts2diff_decoder.h
@@ -30,6 +30,43 @@
#include "utils/util_define.h"
namespace storage {
+
+namespace ts2diff_java_detail {
+
+// Java FloatEncoder overflow flush uses Integer.MAX_VALUE - 1; FloatDecoder
+// then reads BitMaps and Float.intBitsToFloat for float/double TS_2DIFF pages.
+constexpr uint32_t kJavaFloatTs2DiffOverflowMagic =
+ 2147483646u; // Integer.MAX_VALUE - 1
+
+inline int consume_float_double_ts2diff_prefix(common::ByteStream& in) {
+ int ret = common::E_OK;
+ uint32_t mark = in.read_pos();
+ uint32_t tag = 0;
+ if (RET_FAIL(common::SerializationUtil::read_var_uint(tag, in))) {
+ return ret;
+ }
+ if (tag == kJavaFloatTs2DiffOverflowMagic) {
+ uint32_t n = 0;
+ if (RET_FAIL(common::SerializationUtil::read_var_uint(n, in))) {
+ return ret;
+ }
+ int bm_len = static_cast<int>(n) / 8 + 1;
+ uint32_t skip_to = in.read_pos() + static_cast<uint32_t>(bm_len * 2);
+ in.set_read_pos(skip_to);
+ uint32_t inner_max_point = 0;
+ if (RET_FAIL(common::SerializationUtil::read_var_uint(inner_max_point,
+ in))) {
+ return ret;
+ }
+ (void)inner_max_point;
+ } else {
+ in.set_read_pos(mark);
+ }
+ return common::E_OK;
+}
+
+} // namespace ts2diff_java_detail
+
template <typename T>
class TS2DIFFDecoder : public Decoder {
public:
@@ -295,6 +332,16 @@ FORCE_INLINE int FloatTS2DIFFDecoder::read_int64(int64_t& ret_value,
}
FORCE_INLINE int FloatTS2DIFFDecoder::read_float(float& ret_value,
common::ByteStream& in) {
+ int ret = common::E_OK;
+ // Each encoder flush() emits a self-contained Java-compatible segment.
+ // TS_2DIFFDecoder resets current_index_ to 0 at block boundaries; align
+ // the outer FloatEncoder wrapper before reading each new inner block.
+ if (current_index_ == 0) {
+ if (RET_FAIL(
+ ts2diff_java_detail::consume_float_double_ts2diff_prefix(in))) {
+ return ret;
+ }
+ }
ret_value = decode(in);
return common::E_OK;
}
@@ -325,6 +372,13 @@ FORCE_INLINE int DoubleTS2DIFFDecoder::read_float(float& ret_value,
}
FORCE_INLINE int DoubleTS2DIFFDecoder::read_double(double& ret_value,
common::ByteStream& in) {
+ int ret = common::E_OK;
+ if (current_index_ == 0) {
+ if (RET_FAIL(
+ ts2diff_java_detail::consume_float_double_ts2diff_prefix(in))) {
+ return ret;
+ }
+ }
ret_value = decode(in);
return common::E_OK;
}
diff --git a/cpp/src/encoding/ts2diff_encoder.h b/cpp/src/encoding/ts2diff_encoder.h
index 8c5ddafc..bd51d2cc 100644
--- a/cpp/src/encoding/ts2diff_encoder.h
+++ b/cpp/src/encoding/ts2diff_encoder.h
@@ -22,6 +22,8 @@
#include <sys/types.h>
+#include <vector>
+
#include "common/allocator/alloc_base.h"
#include "common/allocator/byte_stream.h"
#include "encoder.h"
@@ -275,6 +277,7 @@ class FloatTS2DIFFEncoder : public TS2DIFFEncoder<int32_t> {
int32_t value_int = common::float_to_int(value);
return TS2DIFFEncoder<int32_t>::do_encode(value_int, out_stream);
}
+ int flush(common::ByteStream& out_stream) override;
int encode(bool value, common::ByteStream& out_stream);
int encode(int32_t value, common::ByteStream& out_stream);
int encode(int64_t value, common::ByteStream& out_stream);
@@ -288,6 +291,7 @@ class DoubleTS2DIFFEncoder : public TS2DIFFEncoder<int64_t> {
int64_t value_long = common::double_to_long(value);
return TS2DIFFEncoder<int64_t>::do_encode(value_long, out_stream);
}
+ int flush(common::ByteStream& out_stream) override;
int encode(bool value, common::ByteStream& out_stream);
int encode(int32_t value, common::ByteStream& out_stream);
int encode(int64_t value, common::ByteStream& out_stream);
@@ -402,5 +406,131 @@ FORCE_INLINE int DoubleTS2DIFFEncoder::encode(double value,
return do_encode(value, out);
}
+// Align with Java FloatEncoder TS_2DIFF page layout (overflow flush): outer
+// header + BitMaps so FloatDecoder uses Float.intBitsToFloat on every value
+// (C++ stores IEEE float bits, not scaled decimals).
+FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) {
+ int ret = common::E_OK;
+ if (write_index_ == -1) {
+ return common::E_OK;
+ }
+ const int num_values = write_index_ + 1;
+ common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false);
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(0u, inner))) {
+ return ret;
+ }
+ SIMDOps<int32_t>::rebase(delta_arr_, delta_arr_min_, write_index_);
+ int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_);
+ if (RET_FAIL(common::SerializationUtil::write_ui32(
+ static_cast<uint32_t>(write_index_), inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_ui32(
+ static_cast<uint32_t>(bit_width), inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_ui32(
+ static_cast<uint32_t>(delta_arr_min_), inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_ui32(
+ static_cast<uint32_t>(first_value_), inner))) {
+ return ret;
+ }
+ for (int i = 0; i < write_index_; i++) {
+ write_bits(delta_arr_[i], bit_width, inner);
+ }
+ flush_remaining(inner);
+ reset();
+
+ constexpr uint32_t kJavaFloatTs2DiffOverflowMagic =
+ 2147483646u; // Java Integer.MAX_VALUE - 1
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(
+ kJavaFloatTs2DiffOverflowMagic, out_stream))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(
+ static_cast<uint32_t>(num_values), out_stream))) {
+ return ret;
+ }
+ int bm_len = num_values / 8 + 1;
+ std::vector<uint8_t> under(static_cast<size_t>(bm_len), 0);
+ std::vector<uint8_t> over(static_cast<size_t>(bm_len), 0);
+ for (int i = 0; i < num_values; i++) {
+ over[static_cast<size_t>(i / 8)] |= static_cast<uint8_t>(1u << (i % 8));
+ }
+ if (RET_FAIL(out_stream.write_buf(under.data(),
+ static_cast<uint32_t>(bm_len)))) {
+ return ret;
+ }
+ if (RET_FAIL(
+ out_stream.write_buf(over.data(), static_cast<uint32_t>(bm_len)))) {
+ return ret;
+ }
+ if (RET_FAIL(merge_byte_stream(out_stream, inner, true))) {
+ return ret;
+ }
+ return ret;
+}
+
+FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) {
+ int ret = common::E_OK;
+ if (write_index_ == -1) {
+ return common::E_OK;
+ }
+ const int num_values = write_index_ + 1;
+ common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false);
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(0u, inner))) {
+ return ret;
+ }
+ SIMDOps<int64_t>::rebase(delta_arr_, delta_arr_min_, write_index_);
+ int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_);
+ if (RET_FAIL(common::SerializationUtil::write_i32(write_index_, inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_i32(bit_width, inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_i64(delta_arr_min_, inner))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_i64(first_value_, inner))) {
+ return ret;
+ }
+ for (int i = 0; i < write_index_; i++) {
+ write_bits(delta_arr_[i], bit_width, inner);
+ }
+ flush_remaining(inner);
+ reset();
+
+ constexpr uint32_t kJavaFloatTs2DiffOverflowMagic = 2147483646u;
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(
+ kJavaFloatTs2DiffOverflowMagic, out_stream))) {
+ return ret;
+ }
+ if (RET_FAIL(common::SerializationUtil::write_var_uint(
+ static_cast<uint32_t>(num_values), out_stream))) {
+ return ret;
+ }
+ int bm_len = num_values / 8 + 1;
+ std::vector<uint8_t> under(static_cast<size_t>(bm_len), 0);
+ std::vector<uint8_t> over(static_cast<size_t>(bm_len), 0);
+ for (int i = 0; i < num_values; i++) {
+ over[static_cast<size_t>(i / 8)] |= static_cast<uint8_t>(1u << (i % 8));
+ }
+ if (RET_FAIL(out_stream.write_buf(under.data(),
+ static_cast<uint32_t>(bm_len)))) {
+ return ret;
+ }
+ if (RET_FAIL(
+ out_stream.write_buf(over.data(), static_cast<uint32_t>(bm_len)))) {
+ return ret;
+ }
+ if (RET_FAIL(merge_byte_stream(out_stream, inner, true))) {
+ return ret;
+ }
+ return ret;
+}
+
} // end namespace storage
#endif // ENCODING_TS2DIFF_ENCODER_H
diff --git a/cpp/test/CMakeLists.txt b/cpp/test/CMakeLists.txt
index 2be9c1b2..9b84bf88 100644
--- a/cpp/test/CMakeLists.txt
+++ b/cpp/test/CMakeLists.txt
@@ -18,7 +18,6 @@ under the License.
]]
cmake_minimum_required(VERSION 3.11)
project(TsFile_CPP_TEST)
-include(FetchContent)
set(CMAKE_VERBOSE_MAKEFILE ON)
@@ -33,36 +32,84 @@ set(DOWNLOADED 0)
set(GTEST_URL "")
set(TIMEOUT 30)
-if (EXISTS ${GTEST_ZIP_PATH})
+# Treat only a real ZIP as valid (local header magic PK\x03\x04 -> hex 504b0304).
+# EXISTS alone is wrong: failed downloads often leave a 0-byte file.
+# Do not use plain file(READ)+string LENGTH on binary: CMake may report length > LIMIT.
+set(GTEST_ZIP_LOCAL_VALID 0)
+if (EXISTS "${GTEST_ZIP_PATH}")
+ file(READ "${GTEST_ZIP_PATH}" GTEST_ZIP_HEX_PROBE LIMIT 4 HEX)
+ string(STRIP "${GTEST_ZIP_HEX_PROBE}" GTEST_ZIP_HEX_PROBE)
+ string(TOLOWER "${GTEST_ZIP_HEX_PROBE}" GTEST_ZIP_HEX_PROBE)
+ if (GTEST_ZIP_HEX_PROBE MATCHES "^504b03")
+ set(GTEST_ZIP_LOCAL_VALID 1)
+ else ()
+ message(
+ WARNING
+ "Local googletest zip is empty or not a zip (${GTEST_ZIP_PATH}); "
+ "will try download."
+ )
+ file(REMOVE "${GTEST_ZIP_PATH}")
+ endif ()
+endif ()
+
+if (GTEST_ZIP_LOCAL_VALID)
message(STATUS "Using local gtest zip file: ${GTEST_ZIP_PATH}")
set(DOWNLOADED 1)
set(GTEST_URL ${GTEST_ZIP_PATH})
else ()
- message(STATUS "Local gtest zip file not found, trying to download from network...")
+ message(STATUS "Local gtest zip missing or invalid, trying to download from network...")
endif ()
if (NOT DOWNLOADED)
foreach (URL ${GTEST_URL_LIST})
message(STATUS "Trying to download from ${URL}")
- file(DOWNLOAD ${URL} "${CMAKE_SOURCE_DIR}/third_party/googletest-release-1.12.1.zip" STATUS DOWNLOAD_STATUS TIMEOUT ${TIMEOUT})
+ file(DOWNLOAD ${URL} "${GTEST_ZIP_PATH}" STATUS DOWNLOAD_STATUS TIMEOUT
+ ${TIMEOUT})
list(GET DOWNLOAD_STATUS 0 DOWNLOAD_RESULT)
- if (${DOWNLOAD_RESULT} EQUAL 0)
- set(DOWNLOADED 1)
- set(GTEST_URL ${GTEST_ZIP_PATH})
- break()
+ if (${DOWNLOAD_RESULT} EQUAL 0 AND EXISTS "${GTEST_ZIP_PATH}")
+ file(READ "${GTEST_ZIP_PATH}" GTEST_ZIP_HEX_PROBE LIMIT 4 HEX)
+ string(STRIP "${GTEST_ZIP_HEX_PROBE}" GTEST_ZIP_HEX_PROBE)
+ string(TOLOWER "${GTEST_ZIP_HEX_PROBE}" GTEST_ZIP_HEX_PROBE)
+ if (GTEST_ZIP_HEX_PROBE MATCHES "^504b03")
+ set(DOWNLOADED 1)
+ set(GTEST_URL ${GTEST_ZIP_PATH})
+ break()
+ else ()
+ message(WARNING "Download from ${URL} did not yield a valid zip; trying next URL...")
+ file(REMOVE "${GTEST_ZIP_PATH}")
+ endif ()
endif ()
endforeach ()
endif ()
if (${DOWNLOADED})
message(STATUS "Successfully get googletest from ${GTEST_URL}")
- FetchContent_Declare(
- googletest
- URL ${GTEST_URL}
- )
set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
- FetchContent_MakeAvailable(googletest)
+ # Extract GitHub release zip via CMake (top folder googletest-release-1.12.1/).
+ # Avoid FetchContent here: deferred populate / wrong extract dir broke configure.
+ set(_gtest_stage "${CMAKE_BINARY_DIR}/googletest-extract")
+ set(GTEST_SRC_ROOT "${_gtest_stage}/googletest-release-1.12.1")
+ if (NOT EXISTS "${GTEST_SRC_ROOT}/CMakeLists.txt")
+ file(REMOVE_RECURSE "${_gtest_stage}")
+ file(MAKE_DIRECTORY "${_gtest_stage}")
+ execute_process(
+ COMMAND ${CMAKE_COMMAND} -E tar xf "${GTEST_ZIP_PATH}"
+ WORKING_DIRECTORY "${_gtest_stage}"
+ RESULT_VARIABLE _gtest_tar_result
+ )
+ if (NOT _gtest_tar_result EQUAL 0)
+ message(FATAL_ERROR "Failed to extract googletest zip: ${GTEST_ZIP_PATH}")
+ endif ()
+ endif ()
+ if (NOT EXISTS "${GTEST_SRC_ROOT}/CMakeLists.txt")
+ message(
+ FATAL_ERROR
+ "googletest zip layout unexpected (missing ${GTEST_SRC_ROOT}/CMakeLists.txt)."
+ )
+ endif ()
+ add_subdirectory("${GTEST_SRC_ROOT}" "${CMAKE_BINARY_DIR}/googletest-build"
+ EXCLUDE_FROM_ALL)
set(TESTS_ENABLED ON PARENT_SCOPE)
else ()
message(WARNING "Failed to download googletest from all provided URLs, setting TESTS_ENABLED to OFF")
diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc
index be16d4af..bfa58b4e 100644
--- a/cpp/test/encoding/ts2diff_codec_test.cc
+++ b/cpp/test/encoding/ts2diff_codec_test.cc
@@ -19,7 +19,10 @@
#include <gtest/gtest.h>
#include <bitset>
+#include <chrono>
+#include <cstring>
#include <random>
+#include <vector>
#include "encoding/ts2diff_decoder.h"
#include "encoding/ts2diff_encoder.h"
@@ -59,6 +62,80 @@ class TS2DIFFCodecTest : public ::testing::Test {
LongTS2DIFFDecoder* decoder_long_;
};
+// Float/double TS_2DIFF: page layout must match Java FloatEncoder (overflow
+// flush) so cross-language readers see Float.intBitsToFloat values.
+class FloatDoubleTS2DIFFCodecTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ encoder_float_ = new FloatTS2DIFFEncoder();
+ decoder_float_ = new FloatTS2DIFFDecoder();
+ encoder_double_ = new DoubleTS2DIFFEncoder();
+ decoder_double_ = new DoubleTS2DIFFDecoder();
+ }
+
+ void TearDown() override {
+ if (encoder_float_ != nullptr) {
+ encoder_float_->destroy();
+ delete encoder_float_;
+ encoder_float_ = nullptr;
+ }
+ if (encoder_double_ != nullptr) {
+ encoder_double_->destroy();
+ delete encoder_double_;
+ encoder_double_ = nullptr;
+ }
+ delete decoder_float_;
+ decoder_float_ = nullptr;
+ delete decoder_double_;
+ decoder_double_ = nullptr;
+ }
+
+ FloatTS2DIFFEncoder* encoder_float_{nullptr};
+ DoubleTS2DIFFEncoder* encoder_double_{nullptr};
+ FloatTS2DIFFDecoder* decoder_float_{nullptr};
+ DoubleTS2DIFFDecoder* decoder_double_{nullptr};
+};
+
+TEST_F(FloatDoubleTS2DIFFCodecTest, TestFloatRoundTrip) {
+ common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false);
+ const int row_num = 1000;
+ std::vector<float> data(row_num);
+ for (int i = 0; i < row_num; i++) {
+ data[i] = static_cast<float>(i) * 0.125f + 0.25f;
+ }
+ for (int i = 0; i < row_num; i++) {
+ EXPECT_EQ(encoder_float_->encode(data[i], out_stream), common::E_OK);
+ }
+ EXPECT_EQ(encoder_float_->flush(out_stream), common::E_OK);
+
+ float x = 0.f;
+ for (int i = 0; i < row_num; i++) {
+ EXPECT_EQ(decoder_float_->read_float(x, out_stream), common::E_OK);
+ EXPECT_FLOAT_EQ(x, data[i]) << "row " << i;
+ }
+ EXPECT_FALSE(decoder_float_->has_remaining(out_stream));
+}
+
+TEST_F(FloatDoubleTS2DIFFCodecTest, TestDoubleRoundTrip) {
+ common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false);
+ const int row_num = 800;
+ std::vector<double> data(row_num);
+ for (int i = 0; i < row_num; i++) {
+ data[i] = static_cast<double>(i) * 0.0625 + 0.5;
+ }
+ for (int i = 0; i < row_num; i++) {
+ EXPECT_EQ(encoder_double_->encode(data[i], out_stream), common::E_OK);
+ }
+ EXPECT_EQ(encoder_double_->flush(out_stream), common::E_OK);
+
+ double y = 0.;
+ for (int i = 0; i < row_num; i++) {
+ EXPECT_EQ(decoder_double_->read_double(y, out_stream), common::E_OK);
+ EXPECT_DOUBLE_EQ(y, data[i]) << "row " << i;
+ }
+ EXPECT_FALSE(decoder_double_->has_remaining(out_stream));
+}
+
TEST_F(TS2DIFFCodecTest, TestIntEncoding1) {
common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false);
const int row_num = 10000;