fix

Author: lucasfang

.pre-commit-config.yaml

@@ -84,6 +84,5 @@ repos:
         exclude: |
           (?x)^(
             (third_party)/.*|
-            (src/paimon/common)/.*|
             (src/paimon/core)/.*
           )$

src/paimon/common/data/binary_array_test.cpp

@@ -38,7 +38,7 @@ TEST(BinaryArrayTest, TestBinaryArraySimple) {
     std::srand(seed);
     std::vector<int64_t> values(100, 0);
     for (auto& value : values) {
-        value = rand() % 10000000 + 1;
+        value = paimon::test::RandomNumber(0, 10000000);
     }
     auto binary_array = BinaryArray::FromLongArray(values, pool.get());
     ASSERT_EQ(values.size(), binary_array.Size()) << "seed:" << seed;

src/paimon/common/data/binary_row_test.cpp

@@ -43,7 +43,7 @@ class BinaryRowTest : public testing::Test {
     void AssertTestWriterRow(const BinaryRow& row) {
         ASSERT_EQ(row.GetString(0).ToString(), "1");
         ASSERT_EQ(row.GetInt(8), 88);
-        ASSERT_EQ(row.GetShort(11), (short)292);
+        ASSERT_EQ(row.GetShort(11), (int16_t)292);
         ASSERT_EQ(row.GetLong(10), 284);
         ASSERT_EQ(row.GetByte(2), (char)99);
         ASSERT_EQ(row.GetDouble(6), (double)87.1);
@@ -101,7 +101,7 @@ TEST_F(BinaryRowTest, TestSetAndGet) {
 
     ASSERT_EQ(row.GetInt(1), 11);
     ASSERT_TRUE(row.IsNullAt(0));
-    ASSERT_EQ(row.GetShort(5), (short)55);
+    ASSERT_EQ(row.GetShort(5), (int16_t)55);
     ASSERT_EQ(row.GetLong(2), 22L);
     ASSERT_TRUE(row.GetBoolean(4));
     ASSERT_EQ(row.GetByte(6), (char)66);
@@ -433,7 +433,7 @@ TEST_F(BinaryRowTest, TestZeroOutPaddingString) {
 
     writer.Reset();
     for (int32_t i = 0; i < bytes_size; i++) {
-        (*bytes)[i] = rand() % 256;
+        (*bytes)[i] = paimon::test::RandomNumber(0, 255);
     }
     writer.WriteBinary(0, *bytes);
     writer.Reset();
@@ -443,7 +443,7 @@ TEST_F(BinaryRowTest, TestZeroOutPaddingString) {
 
     writer.Reset();
     for (int32_t i = 0; i < bytes_size; i++) {
-        (*bytes)[i] = rand() % 256;
+        (*bytes)[i] = paimon::test::RandomNumber(0, 255);
     }
     writer.WriteBinary(0, *bytes);
     writer.Reset();
@@ -557,15 +557,15 @@ TEST_F(BinaryRowTest, TestBinaryRowSerializer) {
     int32_t bytes_size = 1024;
     std::shared_ptr<Bytes> bytes = Bytes::AllocateBytes(1024, pool.get());
     for (int32_t i = 0; i < bytes_size; i++) {
-        (*bytes)[i] = rand() % 256;
+        (*bytes)[i] = paimon::test::RandomNumber(0, 255);
     }
     int32_t str_size = 1024;
     std::string test_string1, test_string2;
     test_string1.reserve(str_size);
     test_string2.reserve(str_size);
     for (int32_t j = 0; j < str_size; j++) {
-        test_string1 += rand() % 26 + 'a';
-        test_string2 += rand() % 26 + 'a';
+        test_string1 += paimon::test::RandomNumber(0, 25) + 'a';
+        test_string2 += paimon::test::RandomNumber(0, 25) + 'a';
     }
     std::shared_ptr<Bytes> bytes1 = Bytes::AllocateBytes(test_string1, pool.get());
     std::shared_ptr<Bytes> bytes2 = Bytes::AllocateBytes(test_string2, pool.get());

src/paimon/common/data/binary_row_writer_test.cpp

@@ -205,12 +205,12 @@ TEST(BinaryRowWriterTest, TestFieldSetter) {
                                                         /*use_view=*/true));
 
     ASSERT_EQ(DataDefine::GetVariantValue<bool>(getter0(row)), true);
-    ASSERT_EQ(DataDefine::GetVariantValue<char>(getter1(row)), (char)1);
-    ASSERT_EQ(DataDefine::GetVariantValue<int16_t>(getter2(row)), (int16_t)2);
-    ASSERT_EQ(DataDefine::GetVariantValue<int32_t>(getter3(row)), (int32_t)3);
-    ASSERT_EQ(DataDefine::GetVariantValue<int64_t>(getter4(row)), (int64_t)4);
-    ASSERT_EQ(DataDefine::GetVariantValue<float>(getter5(row)), (float)5.5);
-    ASSERT_EQ(DataDefine::GetVariantValue<double>(getter6(row)), (double)6.66);
+    ASSERT_EQ(DataDefine::GetVariantValue<char>(getter1(row)), static_cast<char>(1));
+    ASSERT_EQ(DataDefine::GetVariantValue<int16_t>(getter2(row)), static_cast<int16_t>(2));
+    ASSERT_EQ(DataDefine::GetVariantValue<int32_t>(getter3(row)), static_cast<int32_t>(3));
+    ASSERT_EQ(DataDefine::GetVariantValue<int64_t>(getter4(row)), static_cast<int64_t>(4));
+    ASSERT_EQ(DataDefine::GetVariantValue<float>(getter5(row)), static_cast<float>(5.5));
+    ASSERT_EQ(DataDefine::GetVariantValue<double>(getter6(row)), static_cast<double>(6.66));
 
     ASSERT_EQ(DataDefine::GetVariantValue<BinaryString>(getter7(row)).ToString(), data);
     ASSERT_EQ(*DataDefine::GetVariantValue<std::shared_ptr<Bytes>>(getter8(row)),

src/paimon/common/data/binary_section_test.cpp

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include "binary_section.h"
+#include "paimon/common/data/binary_section.h"
 
 #include "gtest/gtest.h"
 #include "paimon/memory/memory_pool.h"

src/paimon/common/data/binary_string_test.cpp

@@ -371,8 +371,8 @@ TEST_F(BinaryStringTest, TestCopy) {
     std::shared_ptr<Bytes> bytes = Bytes::AllocateBytes(s, pool.get());
     BinaryString binary_string = BinaryString::FromBytes(bytes, 0, 6);
     BinaryString copy_binary_string = binary_string.Copy(pool.get());
-    ASSERT_TRUE(binary_string == copy_binary_string);
-    ASSERT_TRUE(copy_binary_string.ByteAt(2) == 'h');
+    ASSERT_EQ(binary_string, copy_binary_string);
+    ASSERT_EQ(copy_binary_string.ByteAt(2), 'h');
 }
 
 TEST_F(BinaryStringTest, TestByteAt) {
@@ -384,8 +384,8 @@ TEST_F(BinaryStringTest, TestByteAt) {
     std::vector<MemorySegment> mem_segs({MemorySegment::Wrap(bytes1), MemorySegment::Wrap(bytes2)});
     auto binary_string = BinaryString::FromAddress(mem_segs, /*offset=*/2,
                                                    /*num_bytes=*/str1.length() + str2.length() - 2);
-    ASSERT_TRUE(binary_string.ByteAt(0) == 'l');
-    ASSERT_TRUE(binary_string.ByteAt(5) == 'r');
+    ASSERT_EQ(binary_string.ByteAt(0), 'l');
+    ASSERT_EQ(binary_string.ByteAt(5), 'r');
 }
 
 TEST_F(BinaryStringTest, TestNumChars) {

src/paimon/common/data/data_define.h

@@ -79,7 +79,8 @@ class DataDefine {
                     return arg ? "true" : "false";
                 } else if constexpr (std::is_same_v<T, std::shared_ptr<Bytes>>) {
                     return std::string(arg->data(), arg->size());
-                } else if constexpr (std::is_same_v<T, BinaryString> ||
+                } else if constexpr (std::is_same_v<T,  // NOLINT(readability/braces)
+                                                    BinaryString> ||
                                      std::is_same_v<T, Timestamp> || std::is_same_v<T, Decimal>) {
                     return arg.ToString();
                 } else if constexpr (std::is_same_v<T, std::shared_ptr<InternalRow>>) {

src/paimon/common/data/data_getters.h

@@ -16,10 +16,13 @@
 
 #pragma once
 
+#include <memory>
+
 #include "paimon/common/data/binary_string.h"
 #include "paimon/common/data/data_define.h"
 #include "paimon/data/decimal.h"
 #include "paimon/data/timestamp.h"
+
 namespace paimon {
 class Bytes;
 class Decimal;

src/paimon/common/data/decimal_test.cpp

@@ -98,7 +98,7 @@ TEST(DecimalTest, TestCompatibleWithJava) {
     auto file_system = std::make_unique<LocalFileSystem>();
     auto file_name = paimon::test::GetDataDir() + "/decimal_bytes.data";
     uint64_t file_length = file_system->GetFileStatus(file_name).value()->GetLen();
-    ASSERT_TRUE(file_length > 0);
+    ASSERT_GT(file_length, 0);
     ASSERT_OK_AND_ASSIGN(auto input_stream, file_system->Open(file_name));
     auto data_bytes = Bytes::AllocateBytes(file_length, pool.get());
     ASSERT_OK(input_stream->Read(data_bytes->data(), file_length));

src/paimon/common/data/generic_row.h

@@ -73,7 +73,7 @@ class GenericRow : public InternalRow {
     /// GenericRow is corrupted. See `InternalRow` for more information about internal data
     /// structures. The field value can be null for representing nullability.
     void SetField(int32_t pos, const VariantType& value) {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         fields_[pos] = value;
     }
 
@@ -82,7 +82,7 @@ class GenericRow : public InternalRow {
     /// for more information about internal data structures.
     /// The returned field value can be null for representing nullability.
     const VariantType& GetField(int32_t pos) const {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return fields_[pos];
     }
 
@@ -99,7 +99,7 @@ class GenericRow : public InternalRow {
     }
 
     bool IsNullAt(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::IsVariantNull(fields_[pos]);
     }
 
@@ -108,22 +108,22 @@ class GenericRow : public InternalRow {
     }
 
     bool GetBoolean(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<bool>(fields_[pos]);
     }
 
     char GetByte(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<char>(fields_[pos]);
     }
 
     int16_t GetShort(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<int16_t>(fields_[pos]);
     }
 
     int32_t GetInt(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<int32_t>(fields_[pos]);
     }
 
@@ -132,57 +132,57 @@ class GenericRow : public InternalRow {
     }
 
     int64_t GetLong(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<int64_t>(fields_[pos]);
     }
 
     float GetFloat(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<float>(fields_[pos]);
     }
 
     double GetDouble(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<double>(fields_[pos]);
     }
 
     BinaryString GetString(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<BinaryString>(fields_[pos]);
     }
 
     std::string_view GetStringView(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<std::string_view>(fields_[pos]);
     }
 
     Decimal GetDecimal(int32_t pos, int32_t precision, int32_t scale) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<Decimal>(fields_[pos]);
     }
 
     Timestamp GetTimestamp(int32_t pos, int32_t precision) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<Timestamp>(fields_[pos]);
     }
 
     std::shared_ptr<Bytes> GetBinary(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<std::shared_ptr<Bytes>>(fields_[pos]);
     }
 
     std::shared_ptr<InternalArray> GetArray(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<std::shared_ptr<InternalArray>>(fields_[pos]);
     }
 
     std::shared_ptr<InternalMap> GetMap(int32_t pos) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<std::shared_ptr<InternalMap>>(fields_[pos]);
     }
 
     std::shared_ptr<InternalRow> GetRow(int32_t pos, int32_t num_fields) const override {
-        assert((size_t)pos < fields_.size());
+        assert(static_cast<size_t>(pos) < fields_.size());
         return DataDefine::GetVariantValue<std::shared_ptr<InternalRow>>(fields_[pos]);
     }