Skip to content

Commit 1bee4c5

Browse files
HeartLinkedHeartLinked
committed
fix ci
1 parent 4cd69a1 commit 1bee4c5

File tree

3 files changed

+92
-95
lines changed

3 files changed

+92
-95
lines changed

‎src/iceberg/util/conversions.cc‎

Lines changed: 41 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ std::vector<uint8_t> WriteLittleEndian(T value) {
4242
/// \brief Read a value in little-endian format from the data.
4343
template <EndianConvertible T>
4444
Result<T> ReadLittleEndian(std::span<const uint8_t> data) {
45-
if (data.size() < sizeof(T)) [[unlikely]] {
45+
if (data.size() != sizeof(T)) [[unlikely]] {
4646
return InvalidArgument("Insufficient data to read {} bytes, got {}", sizeof(T),
4747
data.size());
4848
}
@@ -58,6 +58,35 @@ Result<std::vector<uint8_t>> ToBytesImpl(const Literal::Value& value) {
5858
return WriteLittleEndian(std::get<CppType>(value));
5959
}
6060

61+
template <>
62+
Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBoolean>(const Literal::Value& value) {
63+
return std::vector<uint8_t>{std::get<bool>(value) ? static_cast<uint8_t>(0x01)
64+
: static_cast<uint8_t>(0x00)};
65+
}
66+
67+
template <>
68+
Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kString>(const Literal::Value& value) {
69+
const auto& str = std::get<std::string>(value);
70+
return std::vector<uint8_t>(str.begin(), str.end());
71+
}
72+
73+
template <>
74+
Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBinary>(const Literal::Value& value) {
75+
return std::get<std::vector<uint8_t>>(value);
76+
}
77+
78+
template <>
79+
Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kFixed>(const Literal::Value& value) {
80+
if (std::holds_alternative<std::vector<uint8_t>>(value)) {
81+
return std::get<std::vector<uint8_t>>(value);
82+
} else {
83+
std::string actual_type =
84+
std::visit([](auto&& arg) -> std::string { return typeid(arg).name(); }, value);
85+
return InvalidArgument("Invalid value type for Fixed literal, got type: {}",
86+
actual_type);
87+
}
88+
}
89+
6190
#define DISPATCH_LITERAL_TO_BYTES(type_id) \
6291
case type_id: \
6392
return ToBytesImpl<type_id>(value);
@@ -75,33 +104,10 @@ Result<std::vector<uint8_t>> Conversions::ToBytes(const PrimitiveType& type,
75104
DISPATCH_LITERAL_TO_BYTES(TypeId::kTimestampTz)
76105
DISPATCH_LITERAL_TO_BYTES(TypeId::kFloat)
77106
DISPATCH_LITERAL_TO_BYTES(TypeId::kDouble)
78-
case TypeId::kBoolean: {
79-
return std::vector<uint8_t>{std::get<bool>(value) ? static_cast<uint8_t>(0x01)
80-
: static_cast<uint8_t>(0x00)};
81-
}
82-
83-
case TypeId::kString: {
84-
const auto& str = std::get<std::string>(value);
85-
return std::vector<uint8_t>(str.begin(), str.end());
86-
}
87-
88-
case TypeId::kBinary: {
89-
return std::get<std::vector<uint8_t>>(value);
90-
}
91-
92-
case TypeId::kFixed: {
93-
if (std::holds_alternative<std::array<uint8_t, 16>>(value)) {
94-
const auto& fixed_bytes = std::get<std::array<uint8_t, 16>>(value);
95-
return std::vector<uint8_t>(fixed_bytes.begin(), fixed_bytes.end());
96-
} else if (std::holds_alternative<std::vector<uint8_t>>(value)) {
97-
return std::get<std::vector<uint8_t>>(value);
98-
} else {
99-
std::string actual_type = std::visit(
100-
[](auto&& arg) -> std::string { return typeid(arg).name(); }, value);
101-
return InvalidArgument("Invalid value type for Fixed literal, got type: {}",
102-
actual_type);
103-
}
104-
}
107+
DISPATCH_LITERAL_TO_BYTES(TypeId::kBoolean)
108+
DISPATCH_LITERAL_TO_BYTES(TypeId::kString)
109+
DISPATCH_LITERAL_TO_BYTES(TypeId::kBinary)
110+
DISPATCH_LITERAL_TO_BYTES(TypeId::kFixed)
105111
// TODO(Li Feiyang): Add support for UUID and Decimal
106112

107113
default:
@@ -129,34 +135,23 @@ Result<std::vector<uint8_t>> Conversions::ToBytes(const Literal& literal) {
129135
Result<Literal::Value> Conversions::FromBytes(const PrimitiveType& type,
130136
std::span<const uint8_t> data) {
131137
if (data.empty()) {
132-
return InvalidArgument("Data cannot be empty");
138+
return InvalidArgument("Cannot deserialize empty value");
133139
}
134140

135141
const auto type_id = type.type_id();
136142

137143
switch (type_id) {
138144
case TypeId::kBoolean: {
139-
if (data.size() != 1) {
140-
return InvalidArgument("Boolean requires 1 byte, got {}", data.size());
141-
}
142145
ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<uint8_t>(data));
143146
return Literal::Value{static_cast<bool>(value != 0x00)};
144147
}
145148

146149
case TypeId::kInt: {
147-
if (data.size() != sizeof(int32_t)) {
148-
return InvalidArgument("Int requires {} bytes, got {}", sizeof(int32_t),
149-
data.size());
150-
}
151150
ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<int32_t>(data));
152151
return Literal::Value{value};
153152
}
154153

155154
case TypeId::kDate: {
156-
if (data.size() != sizeof(int32_t)) {
157-
return InvalidArgument("Date requires {} bytes, got {}", sizeof(int32_t),
158-
data.size());
159-
}
160155
ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<int32_t>(data));
161156
return Literal::Value{value};
162157
}
@@ -166,40 +161,30 @@ Result<Literal::Value> Conversions::FromBytes(const PrimitiveType& type,
166161
case TypeId::kTimestamp:
167162
case TypeId::kTimestampTz: {
168163
int64_t value;
169-
if (data.size() == 8) {
170-
ICEBERG_ASSIGN_OR_RAISE(auto long_value, ReadLittleEndian<int64_t>(data));
171-
value = long_value;
172-
} else if (data.size() == 4) {
164+
if (data.size() < 8) {
173165
// Type was promoted from int to long
174166
ICEBERG_ASSIGN_OR_RAISE(auto int_value, ReadLittleEndian<int32_t>(data));
175167
value = static_cast<int64_t>(int_value);
176168
} else {
177-
return InvalidArgument("{} requires 4 or 8 bytes, got {}", ToString(type_id),
178-
data.size());
169+
ICEBERG_ASSIGN_OR_RAISE(auto long_value, ReadLittleEndian<int64_t>(data));
170+
value = long_value;
179171
}
180-
181172
return Literal::Value{value};
182173
}
183174

184175
case TypeId::kFloat: {
185-
if (data.size() != sizeof(float)) {
186-
return InvalidArgument("Float requires {} bytes, got {}", sizeof(float),
187-
data.size());
188-
}
189176
ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<float>(data));
190177
return Literal::Value{value};
191178
}
192179

193180
case TypeId::kDouble: {
194-
if (data.size() == 8) {
195-
ICEBERG_ASSIGN_OR_RAISE(auto double_value, ReadLittleEndian<double>(data));
196-
return Literal::Value{double_value};
197-
} else if (data.size() == 4) {
181+
if (data.size() < 8) {
198182
// Type was promoted from float to double
199183
ICEBERG_ASSIGN_OR_RAISE(auto float_value, ReadLittleEndian<float>(data));
200184
return Literal::Value{static_cast<double>(float_value)};
201185
} else {
202-
return InvalidArgument("Double requires 4 or 8 bytes, got {}", data.size());
186+
ICEBERG_ASSIGN_OR_RAISE(auto double_value, ReadLittleEndian<double>(data));
187+
return Literal::Value{double_value};
203188
}
204189
}
205190

‎src/iceberg/util/conversions.h‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,30 @@ namespace iceberg {
3434
/// \brief Conversion utilities for primitive types
3535
class ICEBERG_EXPORT Conversions {
3636
public:
37-
/// \brief Convert a literal value to bytes
37+
/// \brief Serializes a raw literal value into a byte vector according to its type.
38+
/// \param type The primitive type of the value.
39+
/// \param value The std::variant holding the raw literal value to serialize.
40+
/// \return A Result containing the serialized value.
3841
static Result<std::vector<uint8_t>> ToBytes(const PrimitiveType& type,
3942
const Literal::Value& value);
4043

44+
/// \brief Serializes a complete Literal object into a byte vector.
45+
/// \param literal The Literal object to serialize.
46+
/// \return A Result containing the serialized value.
4147
static Result<std::vector<uint8_t>> ToBytes(const Literal& literal);
4248

43-
/// \brief Convert bytes to a literal value
49+
/// \brief Deserializes a span of bytes into a raw literal value based on the given
50+
/// type.
51+
/// \param type The target primitive type to interpret the bytes as.
52+
/// \param data A std::span of bytes representing the serialized value.
53+
/// \return A Result containing the deserialized value.
4454
static Result<Literal::Value> FromBytes(const PrimitiveType& type,
4555
std::span<const uint8_t> data);
4656

57+
/// \brief Deserializes a span of bytes into a complete Literal object.
58+
/// \param type A shared pointer to the target primitive type.
59+
/// \param data A std::span of bytes representing the serialized value.
60+
/// \return A Result containing the deserialized value.
4761
static Result<Literal> FromBytes(std::shared_ptr<PrimitiveType> type,
4862
std::span<const uint8_t> data);
4963
};

‎test/literal_test.cc‎

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ TEST(LiteralTest, IntCastTo) {
8181
auto long_result = int_literal.CastTo(iceberg::int64());
8282
ASSERT_THAT(long_result, IsOk());
8383
EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
84-
EXPECT_EQ(long_result->ToString(), "42");
84+
EXPECT_EQ(std::get<int64_t>(long_result->value()), 42L);
8585

8686
// Cast to Float
8787
auto float_result = int_literal.CastTo(iceberg::float32());
@@ -136,8 +136,8 @@ TEST(LiteralTest, LongCastTo) {
136136
EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
137137
}
138138

139+
// Test overflow cases
139140
TEST(LiteralTest, LongCastToIntOverflow) {
140-
// Test overflow cases
141141
auto max_long =
142142
Literal::Long(static_cast<int64_t>(std::numeric_limits<int32_t>::max()) + 1);
143143
auto min_long =
@@ -383,42 +383,17 @@ TEST(LiteralTest, DoubleZeroComparison) {
383383
EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
384384
}
385385

386-
// Type promotion tests
387-
TEST(LiteralSerializationTest, TypePromotion) {
388-
// 4-byte int data can be deserialized as long
389-
std::vector<uint8_t> int_data = {32, 0, 0, 0};
390-
auto long_result = Literal::Deserialize(int_data, int64());
391-
ASSERT_TRUE(long_result.has_value());
392-
EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
393-
EXPECT_EQ(long_result->ToString(), "32");
394-
395-
auto long_bytes = long_result->Serialize();
396-
ASSERT_TRUE(long_bytes.has_value());
397-
EXPECT_EQ(long_bytes->size(), 8);
398-
399-
// 4-byte float data can be deserialized as double
400-
std::vector<uint8_t> float_data = {0, 0, 128, 63};
401-
auto double_result = Literal::Deserialize(float_data, float64());
402-
ASSERT_TRUE(double_result.has_value());
403-
EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
404-
EXPECT_EQ(double_result->ToString(), "1.000000");
405-
406-
auto double_bytes = double_result->Serialize();
407-
ASSERT_TRUE(double_bytes.has_value());
408-
EXPECT_EQ(double_bytes->size(), 8);
409-
}
410-
411386
struct LiteralRoundTripParam {
412387
std::string test_name;
413388
std::vector<uint8_t> input_bytes;
414389
Literal expected_literal;
415390
std::shared_ptr<PrimitiveType> type;
416391
};
417392

418-
class LiteralSerializationParamTest
419-
: public ::testing::TestWithParam<LiteralRoundTripParam> {};
393+
class LiteralSerializationParam : public ::testing::TestWithParam<LiteralRoundTripParam> {
394+
};
420395

421-
TEST_P(LiteralSerializationParamTest, RoundTrip) {
396+
TEST_P(LiteralSerializationParam, RoundTrip) {
422397
const auto& param = GetParam();
423398

424399
// Deserialize from bytes
@@ -427,8 +402,7 @@ TEST_P(LiteralSerializationParamTest, RoundTrip) {
427402
<< "Deserialization failed: " << literal_result.error().message;
428403

429404
// Check type and value
430-
EXPECT_EQ(literal_result->type()->type_id(), param.expected_literal.type()->type_id());
431-
EXPECT_EQ(literal_result->ToString(), param.expected_literal.ToString());
405+
EXPECT_EQ(*literal_result, param.expected_literal);
432406

433407
// Serialize back to bytes
434408
Result<std::vector<uint8_t>> bytes_result = literal_result->Serialize();
@@ -440,12 +414,11 @@ TEST_P(LiteralSerializationParamTest, RoundTrip) {
440414
Result<Literal> final_literal = Literal::Deserialize(*bytes_result, param.type);
441415
ASSERT_TRUE(final_literal.has_value())
442416
<< "Final deserialization failed: " << final_literal.error().message;
443-
EXPECT_EQ(final_literal->type()->type_id(), param.expected_literal.type()->type_id());
444-
EXPECT_EQ(final_literal->ToString(), param.expected_literal.ToString());
417+
EXPECT_EQ(*final_literal, param.expected_literal);
445418
}
446419

447420
INSTANTIATE_TEST_SUITE_P(
448-
BinarySerializationTests, LiteralSerializationParamTest,
421+
BinarySerialization, LiteralSerializationParam,
449422
::testing::Values(
450423
// Basic types
451424
LiteralRoundTripParam{"BooleanTrue", {1}, Literal::Boolean(true), boolean()},
@@ -483,11 +456,11 @@ INSTANTIATE_TEST_SUITE_P(
483456
// TODO(Li Feiyang): Add tests for Date, Time, Timestamp, TimestampTz
484457
),
485458

486-
[](const testing::TestParamInfo<LiteralSerializationParamTest::ParamType>& info) {
459+
[](const testing::TestParamInfo<LiteralSerializationParam::ParamType>& info) {
487460
return info.param.test_name;
488461
});
489462

490-
TEST(LiteralSerializationEdgeCaseTest, EmptyStringSerialization) {
463+
TEST(LiteralSerializationTest, EmptyString) {
491464
auto empty_string = Literal::String("");
492465
auto empty_bytes = empty_string.Serialize();
493466
ASSERT_TRUE(empty_bytes.has_value());
@@ -497,4 +470,29 @@ TEST(LiteralSerializationEdgeCaseTest, EmptyStringSerialization) {
497470
EXPECT_THAT(deserialize_result, IsError(ErrorKind::kInvalidArgument));
498471
}
499472

473+
// Type promotion tests
474+
TEST(LiteralSerializationTest, TypePromotion) {
475+
// 4-byte int data can be deserialized as long
476+
std::vector<uint8_t> int_data = {32, 0, 0, 0};
477+
auto long_result = Literal::Deserialize(int_data, int64());
478+
ASSERT_TRUE(long_result.has_value());
479+
EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
480+
EXPECT_EQ(std::get<int64_t>(long_result->value()), 32L);
481+
482+
auto long_bytes = long_result->Serialize();
483+
ASSERT_TRUE(long_bytes.has_value());
484+
EXPECT_EQ(long_bytes->size(), 8);
485+
486+
// 4-byte float data can be deserialized as double
487+
std::vector<uint8_t> float_data = {0, 0, 128, 63};
488+
auto double_result = Literal::Deserialize(float_data, float64());
489+
ASSERT_TRUE(double_result.has_value());
490+
EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
491+
EXPECT_EQ(std::get<double>(double_result->value()), 1.0);
492+
493+
auto double_bytes = double_result->Serialize();
494+
ASSERT_TRUE(double_bytes.has_value());
495+
EXPECT_EQ(double_bytes->size(), 8);
496+
}
497+
500498
} // namespace iceberg

0 commit comments

Comments
 (0)