wgtmac commented on code in PR #185:
URL: https://github.com/apache/iceberg-cpp/pull/185#discussion_r2375102363
##########
src/iceberg/expression/literal.cc:
##########
@@ -249,6 +255,20 @@ std::partial_ordering Literal::operator<=>(const Literal&
other) const {
return this_val <=> other_val;
}
+ case TypeId::kFixed: {
+ // Fixed types can only be compared if they have the same length
+ auto& this_fixed_type = static_cast<const FixedType&>(*type_);
+ auto& other_fixed_type = static_cast<const FixedType&>(*other.type_);
+
+ if (this_fixed_type.length() != other_fixed_type.length()) {
+ return std::partial_ordering::unordered;
+ }
Review Comment:
I think we can remove line 259 to line 256 if we modify line 197 to
```
// If types are different, comparison is unordered
if (*type_ != *other.type_) {
return std::partial_ordering::unordered;
}
```
##########
src/iceberg/expression/literal.h:
##########
@@ -71,6 +71,7 @@ class ICEBERG_EXPORT Literal {
static Literal Double(double value);
static Literal String(std::string value);
static Literal Binary(std::vector<uint8_t> value);
+ static Literal Fixed(std::vector<uint8_t> value, int32_t length);
Review Comment:
```suggestion
static Literal Fixed(std::vector<uint8_t> value);
```
I think a single vector is enough and the length can be deduced from
value.size(). Otherwise, it is annoying when `length != value.size()`, should
we throw then?
##########
src/iceberg/expression/literal.cc:
##########
@@ -249,6 +255,20 @@ std::partial_ordering Literal::operator<=>(const Literal&
other) const {
return this_val <=> other_val;
}
+ case TypeId::kFixed: {
+ // Fixed types can only be compared if they have the same length
+ auto& this_fixed_type = static_cast<const FixedType&>(*type_);
+ auto& other_fixed_type = static_cast<const FixedType&>(*other.type_);
+
+ if (this_fixed_type.length() != other_fixed_type.length()) {
+ return std::partial_ordering::unordered;
+ }
+
Review Comment:
```suggestion
```
I'd suggest to remove these lines and change the line 196 above as below:
```cpp
// If types are different, comparison is unordered
if (*type_ != *other.type_) {
return std::partial_ordering::unordered;
}
```
##########
src/iceberg/expression/literal.cc:
##########
@@ -149,13 +150,17 @@ Literal Literal::Binary(std::vector<uint8_t> value) {
return {Value{std::move(value)}, binary()};
}
+Literal Literal::Fixed(std::vector<uint8_t> value, int32_t length) {
+ return {Value{std::move(value)}, fixed(length)};
Review Comment:
nit: add `ICEBERG_DCHECK(value.size() == length)`
##########
test/literal_test.cc:
##########
@@ -383,4 +383,211 @@ TEST(LiteralTest, DoubleZeroComparison) {
EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
}
+struct LiteralRoundTripParam {
+ std::string test_name;
+ std::vector<uint8_t> input_bytes;
+ Literal expected_literal;
+ std::shared_ptr<PrimitiveType> type;
+};
+
+class LiteralSerializationParam : public
::testing::TestWithParam<LiteralRoundTripParam> {
+};
+
+TEST_P(LiteralSerializationParam, RoundTrip) {
+ const auto& param = GetParam();
+
+ // Deserialize from bytes
+ Result<Literal> literal_result = Literal::Deserialize(param.input_bytes,
param.type);
+ ASSERT_TRUE(literal_result.has_value())
+ << "Deserialization failed: " << literal_result.error().message;
+
+ // Check type and value
+ EXPECT_EQ(*literal_result, param.expected_literal);
+
+ // Serialize back to bytes
+ Result<std::vector<uint8_t>> bytes_result = literal_result->Serialize();
+ ASSERT_TRUE(bytes_result.has_value())
+ << "Serialization failed: " << bytes_result.error().message;
+ EXPECT_EQ(*bytes_result, param.input_bytes);
+
+ // Deserialize again to verify idempotency
+ Result<Literal> final_literal = Literal::Deserialize(*bytes_result,
param.type);
+ ASSERT_TRUE(final_literal.has_value())
+ << "Final deserialization failed: " << final_literal.error().message;
+ EXPECT_EQ(*final_literal, param.expected_literal);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ BinarySerialization, LiteralSerializationParam,
+ ::testing::Values(
+ // Basic types
+ LiteralRoundTripParam{"BooleanTrue", {1}, Literal::Boolean(true),
boolean()},
+ LiteralRoundTripParam{"BooleanFalse", {0}, Literal::Boolean(false),
boolean()},
+
+ LiteralRoundTripParam{"Int", {32, 0, 0, 0}, Literal::Int(32), int32()},
+ LiteralRoundTripParam{
+ "IntMaxValue", {255, 255, 255, 127}, Literal::Int(2147483647),
int32()},
+ LiteralRoundTripParam{
+ "IntMinValue", {0, 0, 0, 128}, Literal::Int(-2147483648), int32()},
+ LiteralRoundTripParam{
+ "NegativeInt", {224, 255, 255, 255}, Literal::Int(-32), int32()},
+
+ LiteralRoundTripParam{
+ "Long", {32, 0, 0, 0, 0, 0, 0, 0}, Literal::Long(32), int64()},
+ LiteralRoundTripParam{"LongMaxValue",
+ {255, 255, 255, 255, 255, 255, 255, 127},
+
Literal::Long(std::numeric_limits<int64_t>::max()),
+ int64()},
+ LiteralRoundTripParam{"LongMinValue",
+ {0, 0, 0, 0, 0, 0, 0, 128},
+
Literal::Long(std::numeric_limits<int64_t>::min()),
+ int64()},
+ LiteralRoundTripParam{"NegativeLong",
+ {224, 255, 255, 255, 255, 255, 255, 255},
+ Literal::Long(-32),
+ int64()},
+
+ LiteralRoundTripParam{"Float", {0, 0, 128, 63}, Literal::Float(1.0f),
float32()},
+ LiteralRoundTripParam{"FloatNegativeInfinity",
+ {0, 0, 128, 255},
+
Literal::Float(-std::numeric_limits<float>::infinity()),
+ float32()},
+ LiteralRoundTripParam{"FloatMaxValue",
+ {255, 255, 127, 127},
+
Literal::Float(std::numeric_limits<float>::max()),
+ float32()},
+ LiteralRoundTripParam{"FloatMinValue",
+ {255, 255, 127, 255},
+
Literal::Float(std::numeric_limits<float>::lowest()),
+ float32()},
+
+ LiteralRoundTripParam{
+ "Double", {0, 0, 0, 0, 0, 0, 240, 63}, Literal::Double(1.0),
float64()},
+ LiteralRoundTripParam{"DoubleNegativeInfinity",
+ {0, 0, 0, 0, 0, 0, 240, 255},
+
Literal::Double(-std::numeric_limits<double>::infinity()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMaxValue",
+ {255, 255, 255, 255, 255, 255, 239, 127},
+
Literal::Double(std::numeric_limits<double>::max()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMinValue",
+ {255, 255, 255, 255, 255, 255, 239, 255},
+
Literal::Double(std::numeric_limits<double>::lowest()),
+ float64()},
+
+ LiteralRoundTripParam{"String",
+ {105, 99, 101, 98, 101, 114, 103},
+ Literal::String("iceberg"),
+ string()},
+ LiteralRoundTripParam{
+ "StringLong",
+ {65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65},
+ Literal::String("AAAAAAAAAAAAAAAA"),
+ string()},
+
+ LiteralRoundTripParam{"BinaryData",
+ {0x01, 0x02, 0x03, 0xFF},
+ Literal::Binary({0x01, 0x02, 0x03, 0xFF}),
+ binary()},
+ LiteralRoundTripParam{"BinarySingleByte", {42}, Literal::Binary({42}),
binary()},
+
+ // Fixed type
+ LiteralRoundTripParam{"FixedLength4",
+ {0x01, 0x02, 0x03, 0x04},
+ Literal::Fixed({0x01, 0x02, 0x03, 0x04}, 4),
+ fixed(4)},
+ LiteralRoundTripParam{
+ "FixedLength8",
+ {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
+ Literal::Fixed({0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
8),
+ fixed(8)},
+ LiteralRoundTripParam{
+ "FixedLength16",
+ {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A,
0x0B, 0x0C,
+ 0x0D, 0x0E, 0x0F},
+ Literal::Fixed({0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09,
+ 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
+ 16),
+ fixed(16)},
+ LiteralRoundTripParam{
+ "FixedSingleByte", {0xFF}, Literal::Fixed({0xFF}, 1), fixed(1)},
+
+ // Temporal types
+ LiteralRoundTripParam{"DateEpoch", {0, 0, 0, 0}, Literal::Date(0),
date()},
+ LiteralRoundTripParam{"DateNextDay", {1, 0, 0, 0}, Literal::Date(1),
date()},
+ LiteralRoundTripParam{"DateY2K", {205, 42, 0, 0},
Literal::Date(10957), date()},
+ LiteralRoundTripParam{
+ "DateNegative", {255, 255, 255, 255}, Literal::Date(-1), date()},
+
+ LiteralRoundTripParam{
+ "TimeMidnight", {0, 0, 0, 0, 0, 0, 0, 0}, Literal::Time(0),
time()},
+ LiteralRoundTripParam{"TimeNoon",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Time(45045123456),
+ time()},
+ LiteralRoundTripParam{
+ "TimeOneSecond", {64, 66, 15, 0, 0, 0, 0, 0},
Literal::Time(1000000), time()},
+
+ LiteralRoundTripParam{"TimestampEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::Timestamp(0),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampOneSecond",
+ {64, 66, 15, 0, 0, 0, 0, 0},
+ Literal::Timestamp(1000000),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampNoon2024",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Timestamp(45045123456),
+ timestamp()},
+
+ LiteralRoundTripParam{"TimestampTzEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::TimestampTz(0),
+ timestamp_tz()},
+ LiteralRoundTripParam{"TimestampTzOneHour",
+ {0, 164, 147, 214, 0, 0, 0, 0},
+ Literal::TimestampTz(3600000000),
+ timestamp_tz()}),
+
+ [](const testing::TestParamInfo<LiteralSerializationParam::ParamType>&
info) {
+ return info.param.test_name;
+ });
+
+TEST(LiteralSerializationTest, EmptyString) {
+ auto empty_string = Literal::String("");
+ auto empty_bytes = empty_string.Serialize();
+ ASSERT_TRUE(empty_bytes.has_value());
+ EXPECT_TRUE(empty_bytes->empty());
+
+ auto deserialize_result = Literal::Deserialize(*empty_bytes, string());
+ EXPECT_THAT(deserialize_result, IsError(ErrorKind::kInvalidArgument));
+}
+
+// Type promotion tests
+TEST(LiteralSerializationTest, TypePromotion) {
+ // 4-byte int data can be deserialized as long
+ std::vector<uint8_t> int_data = {32, 0, 0, 0};
+ auto long_result = Literal::Deserialize(int_data, int64());
+ ASSERT_TRUE(long_result.has_value());
+ EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
+ EXPECT_EQ(std::get<int64_t>(long_result->value()), 32L);
+
+ auto long_bytes = long_result->Serialize();
+ ASSERT_TRUE(long_bytes.has_value());
+ EXPECT_EQ(long_bytes->size(), 8);
+
+ // 4-byte float data can be deserialized as double
+ std::vector<uint8_t> float_data = {0, 0, 128, 63};
+ auto double_result = Literal::Deserialize(float_data, float64());
+ ASSERT_TRUE(double_result.has_value());
+ EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
+ EXPECT_EQ(std::get<double>(double_result->value()), 1.0);
+
+ auto double_bytes = double_result->Serialize();
+ ASSERT_TRUE(double_bytes.has_value());
+ EXPECT_EQ(double_bytes->size(), 8);
Review Comment:
```suggestion
```
ditto
##########
test/literal_test.cc:
##########
@@ -383,4 +383,211 @@ TEST(LiteralTest, DoubleZeroComparison) {
EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
}
+struct LiteralRoundTripParam {
+ std::string test_name;
+ std::vector<uint8_t> input_bytes;
+ Literal expected_literal;
+ std::shared_ptr<PrimitiveType> type;
+};
+
+class LiteralSerializationParam : public
::testing::TestWithParam<LiteralRoundTripParam> {
+};
+
+TEST_P(LiteralSerializationParam, RoundTrip) {
+ const auto& param = GetParam();
+
+ // Deserialize from bytes
+ Result<Literal> literal_result = Literal::Deserialize(param.input_bytes,
param.type);
+ ASSERT_TRUE(literal_result.has_value())
+ << "Deserialization failed: " << literal_result.error().message;
+
+ // Check type and value
+ EXPECT_EQ(*literal_result, param.expected_literal);
+
+ // Serialize back to bytes
+ Result<std::vector<uint8_t>> bytes_result = literal_result->Serialize();
+ ASSERT_TRUE(bytes_result.has_value())
+ << "Serialization failed: " << bytes_result.error().message;
+ EXPECT_EQ(*bytes_result, param.input_bytes);
+
+ // Deserialize again to verify idempotency
+ Result<Literal> final_literal = Literal::Deserialize(*bytes_result,
param.type);
+ ASSERT_TRUE(final_literal.has_value())
+ << "Final deserialization failed: " << final_literal.error().message;
+ EXPECT_EQ(*final_literal, param.expected_literal);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ BinarySerialization, LiteralSerializationParam,
+ ::testing::Values(
+ // Basic types
+ LiteralRoundTripParam{"BooleanTrue", {1}, Literal::Boolean(true),
boolean()},
+ LiteralRoundTripParam{"BooleanFalse", {0}, Literal::Boolean(false),
boolean()},
+
+ LiteralRoundTripParam{"Int", {32, 0, 0, 0}, Literal::Int(32), int32()},
+ LiteralRoundTripParam{
+ "IntMaxValue", {255, 255, 255, 127}, Literal::Int(2147483647),
int32()},
+ LiteralRoundTripParam{
+ "IntMinValue", {0, 0, 0, 128}, Literal::Int(-2147483648), int32()},
+ LiteralRoundTripParam{
+ "NegativeInt", {224, 255, 255, 255}, Literal::Int(-32), int32()},
+
+ LiteralRoundTripParam{
+ "Long", {32, 0, 0, 0, 0, 0, 0, 0}, Literal::Long(32), int64()},
+ LiteralRoundTripParam{"LongMaxValue",
+ {255, 255, 255, 255, 255, 255, 255, 127},
+
Literal::Long(std::numeric_limits<int64_t>::max()),
+ int64()},
+ LiteralRoundTripParam{"LongMinValue",
+ {0, 0, 0, 0, 0, 0, 0, 128},
+
Literal::Long(std::numeric_limits<int64_t>::min()),
+ int64()},
+ LiteralRoundTripParam{"NegativeLong",
+ {224, 255, 255, 255, 255, 255, 255, 255},
+ Literal::Long(-32),
+ int64()},
+
+ LiteralRoundTripParam{"Float", {0, 0, 128, 63}, Literal::Float(1.0f),
float32()},
+ LiteralRoundTripParam{"FloatNegativeInfinity",
+ {0, 0, 128, 255},
+
Literal::Float(-std::numeric_limits<float>::infinity()),
+ float32()},
+ LiteralRoundTripParam{"FloatMaxValue",
+ {255, 255, 127, 127},
+
Literal::Float(std::numeric_limits<float>::max()),
+ float32()},
+ LiteralRoundTripParam{"FloatMinValue",
+ {255, 255, 127, 255},
+
Literal::Float(std::numeric_limits<float>::lowest()),
+ float32()},
+
+ LiteralRoundTripParam{
+ "Double", {0, 0, 0, 0, 0, 0, 240, 63}, Literal::Double(1.0),
float64()},
+ LiteralRoundTripParam{"DoubleNegativeInfinity",
+ {0, 0, 0, 0, 0, 0, 240, 255},
+
Literal::Double(-std::numeric_limits<double>::infinity()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMaxValue",
+ {255, 255, 255, 255, 255, 255, 239, 127},
+
Literal::Double(std::numeric_limits<double>::max()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMinValue",
+ {255, 255, 255, 255, 255, 255, 239, 255},
+
Literal::Double(std::numeric_limits<double>::lowest()),
+ float64()},
+
+ LiteralRoundTripParam{"String",
+ {105, 99, 101, 98, 101, 114, 103},
+ Literal::String("iceberg"),
+ string()},
+ LiteralRoundTripParam{
+ "StringLong",
+ {65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65},
+ Literal::String("AAAAAAAAAAAAAAAA"),
+ string()},
+
+ LiteralRoundTripParam{"BinaryData",
+ {0x01, 0x02, 0x03, 0xFF},
+ Literal::Binary({0x01, 0x02, 0x03, 0xFF}),
+ binary()},
+ LiteralRoundTripParam{"BinarySingleByte", {42}, Literal::Binary({42}),
binary()},
+
+ // Fixed type
+ LiteralRoundTripParam{"FixedLength4",
+ {0x01, 0x02, 0x03, 0x04},
+ Literal::Fixed({0x01, 0x02, 0x03, 0x04}, 4),
+ fixed(4)},
+ LiteralRoundTripParam{
+ "FixedLength8",
+ {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
+ Literal::Fixed({0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
8),
+ fixed(8)},
+ LiteralRoundTripParam{
+ "FixedLength16",
+ {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A,
0x0B, 0x0C,
+ 0x0D, 0x0E, 0x0F},
+ Literal::Fixed({0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09,
+ 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
+ 16),
+ fixed(16)},
+ LiteralRoundTripParam{
+ "FixedSingleByte", {0xFF}, Literal::Fixed({0xFF}, 1), fixed(1)},
+
+ // Temporal types
+ LiteralRoundTripParam{"DateEpoch", {0, 0, 0, 0}, Literal::Date(0),
date()},
+ LiteralRoundTripParam{"DateNextDay", {1, 0, 0, 0}, Literal::Date(1),
date()},
+ LiteralRoundTripParam{"DateY2K", {205, 42, 0, 0},
Literal::Date(10957), date()},
+ LiteralRoundTripParam{
+ "DateNegative", {255, 255, 255, 255}, Literal::Date(-1), date()},
+
+ LiteralRoundTripParam{
+ "TimeMidnight", {0, 0, 0, 0, 0, 0, 0, 0}, Literal::Time(0),
time()},
+ LiteralRoundTripParam{"TimeNoon",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Time(45045123456),
+ time()},
+ LiteralRoundTripParam{
+ "TimeOneSecond", {64, 66, 15, 0, 0, 0, 0, 0},
Literal::Time(1000000), time()},
+
+ LiteralRoundTripParam{"TimestampEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::Timestamp(0),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampOneSecond",
+ {64, 66, 15, 0, 0, 0, 0, 0},
+ Literal::Timestamp(1000000),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampNoon2024",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Timestamp(45045123456),
+ timestamp()},
+
+ LiteralRoundTripParam{"TimestampTzEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::TimestampTz(0),
+ timestamp_tz()},
+ LiteralRoundTripParam{"TimestampTzOneHour",
+ {0, 164, 147, 214, 0, 0, 0, 0},
+ Literal::TimestampTz(3600000000),
+ timestamp_tz()}),
+
+ [](const testing::TestParamInfo<LiteralSerializationParam::ParamType>&
info) {
+ return info.param.test_name;
+ });
+
+TEST(LiteralSerializationTest, EmptyString) {
+ auto empty_string = Literal::String("");
+ auto empty_bytes = empty_string.Serialize();
+ ASSERT_TRUE(empty_bytes.has_value());
+ EXPECT_TRUE(empty_bytes->empty());
+
+ auto deserialize_result = Literal::Deserialize(*empty_bytes, string());
+ EXPECT_THAT(deserialize_result, IsError(ErrorKind::kInvalidArgument));
Review Comment:
Could you double check the spec and java impl on how to deal with empty
values? I think this is a valid case which shouldn't return an error.
##########
src/iceberg/util/conversions.cc:
##########
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/util/conversions.h"
+
+#include <array>
+#include <cstring>
+#include <span>
+#include <string>
+
+#include "iceberg/util/endian.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+/// \brief Write a value in little-endian format and return as vector.
+template <EndianConvertible T>
+std::vector<uint8_t> WriteLittleEndian(T value) {
+ value = ToLittleEndian(value);
+ const auto* bytes = reinterpret_cast<const uint8_t*>(&value);
+ std::vector<uint8_t> result;
+ result.insert(result.end(), bytes, bytes + sizeof(T));
+ return result;
+}
+
+/// \brief Read a value in little-endian format from the data.
+template <EndianConvertible T>
+Result<T> ReadLittleEndian(std::span<const uint8_t> data) {
+ if (data.size() != sizeof(T)) [[unlikely]] {
+ return InvalidArgument("Insufficient data to read {} bytes, got {}",
sizeof(T),
+ data.size());
+ }
+
+ T value;
+ std::memcpy(&value, data.data(), sizeof(T));
+ return FromLittleEndian(value);
+}
+
+template <TypeId type_id>
+Result<std::vector<uint8_t>> ToBytesImpl(const Literal::Value& value) {
+ using CppType = typename LiteralTraits<type_id>::ValueType;
+ return WriteLittleEndian(std::get<CppType>(value));
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBoolean>(const
Literal::Value& value) {
+ return std::vector<uint8_t>{std::get<bool>(value) ?
static_cast<uint8_t>(0x01)
+ :
static_cast<uint8_t>(0x00)};
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kString>(const
Literal::Value& value) {
+ const auto& str = std::get<std::string>(value);
+ return std::vector<uint8_t>(str.begin(), str.end());
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBinary>(const
Literal::Value& value) {
+ return std::get<std::vector<uint8_t>>(value);
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kFixed>(const Literal::Value&
value) {
+ if (std::holds_alternative<std::vector<uint8_t>>(value)) {
+ return std::get<std::vector<uint8_t>>(value);
+ } else {
+ std::string actual_type =
+ std::visit([](auto&& arg) -> std::string { return typeid(arg).name();
}, value);
+ return InvalidArgument("Invalid value type for Fixed literal, got type:
{}",
+ actual_type);
+ }
Review Comment:
```suggestion
return std::get<std::vector<uint8_t>>(value);
```
##########
src/iceberg/util/conversions.cc:
##########
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/util/conversions.h"
+
+#include <array>
+#include <cstring>
+#include <span>
+#include <string>
+
+#include "iceberg/util/endian.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+/// \brief Write a value in little-endian format and return as vector.
+template <EndianConvertible T>
+std::vector<uint8_t> WriteLittleEndian(T value) {
+ value = ToLittleEndian(value);
+ const auto* bytes = reinterpret_cast<const uint8_t*>(&value);
+ std::vector<uint8_t> result;
+ result.insert(result.end(), bytes, bytes + sizeof(T));
+ return result;
+}
+
+/// \brief Read a value in little-endian format from the data.
+template <EndianConvertible T>
+Result<T> ReadLittleEndian(std::span<const uint8_t> data) {
+ if (data.size() != sizeof(T)) [[unlikely]] {
+ return InvalidArgument("Insufficient data to read {} bytes, got {}",
sizeof(T),
+ data.size());
+ }
+
+ T value;
+ std::memcpy(&value, data.data(), sizeof(T));
+ return FromLittleEndian(value);
+}
+
+template <TypeId type_id>
+Result<std::vector<uint8_t>> ToBytesImpl(const Literal::Value& value) {
+ using CppType = typename LiteralTraits<type_id>::ValueType;
+ return WriteLittleEndian(std::get<CppType>(value));
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBoolean>(const
Literal::Value& value) {
+ return std::vector<uint8_t>{std::get<bool>(value) ?
static_cast<uint8_t>(0x01)
+ :
static_cast<uint8_t>(0x00)};
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kString>(const
Literal::Value& value) {
+ const auto& str = std::get<std::string>(value);
+ return std::vector<uint8_t>(str.begin(), str.end());
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBinary>(const
Literal::Value& value) {
+ return std::get<std::vector<uint8_t>>(value);
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kFixed>(const Literal::Value&
value) {
+ if (std::holds_alternative<std::vector<uint8_t>>(value)) {
+ return std::get<std::vector<uint8_t>>(value);
+ } else {
+ std::string actual_type =
+ std::visit([](auto&& arg) -> std::string { return typeid(arg).name();
}, value);
+ return InvalidArgument("Invalid value type for Fixed literal, got type:
{}",
+ actual_type);
+ }
+}
+
+#define DISPATCH_LITERAL_TO_BYTES(type_id) \
+ case type_id: \
+ return ToBytesImpl<type_id>(value);
+
+Result<std::vector<uint8_t>> Conversions::ToBytes(const PrimitiveType& type,
+ const Literal::Value& value)
{
+ const auto type_id = type.type_id();
+
+ switch (type_id) {
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kInt)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kDate)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kLong)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTime)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTimestamp)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTimestampTz)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kFloat)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kDouble)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kBoolean)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kString)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kBinary)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kFixed)
+ // TODO(Li Feiyang): Add support for UUID and Decimal
+
+ default:
+ return NotSupported("Serialization for type {} is not supported",
type.ToString());
+ }
+}
+
+#undef DISPATCH_LITERAL_TO_BYTES
+
+Result<std::vector<uint8_t>> Conversions::ToBytes(const Literal& literal) {
+ // Cannot serialize special values
+ if (literal.IsAboveMax()) {
+ return NotSupported("Cannot serialize AboveMax");
+ }
+ if (literal.IsBelowMin()) {
+ return NotSupported("Cannot serialize BelowMin");
+ }
+ if (literal.IsNull()) {
+ return NotSupported("Cannot serialize null");
+ }
+
+ return ToBytes(*literal.type(), literal.value());
+}
+
+Result<Literal::Value> Conversions::FromBytes(const PrimitiveType& type,
+ std::span<const uint8_t> data) {
+ if (data.empty()) {
+ return InvalidArgument("Cannot deserialize empty value");
+ }
+
+ const auto type_id = type.type_id();
+
+ switch (type_id) {
+ case TypeId::kBoolean: {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<uint8_t>(data));
+ return Literal::Value{static_cast<bool>(value != 0x00)};
+ }
+
Review Comment:
Generally I don't think it is a good practice to add blank lines between
cases which makes the switch block lengthy.
##########
test/literal_test.cc:
##########
@@ -136,8 +136,8 @@ TEST(LiteralTest, LongCastTo) {
EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
}
+// Test overflow cases
Review Comment:
```suggestion
```
The comment is redundant.
##########
src/iceberg/util/conversions.cc:
##########
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/util/conversions.h"
+
+#include <array>
+#include <cstring>
+#include <span>
+#include <string>
+
+#include "iceberg/util/endian.h"
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+/// \brief Write a value in little-endian format and return as vector.
+template <EndianConvertible T>
+std::vector<uint8_t> WriteLittleEndian(T value) {
+ value = ToLittleEndian(value);
+ const auto* bytes = reinterpret_cast<const uint8_t*>(&value);
+ std::vector<uint8_t> result;
+ result.insert(result.end(), bytes, bytes + sizeof(T));
+ return result;
+}
+
+/// \brief Read a value in little-endian format from the data.
+template <EndianConvertible T>
+Result<T> ReadLittleEndian(std::span<const uint8_t> data) {
+ if (data.size() != sizeof(T)) [[unlikely]] {
+ return InvalidArgument("Insufficient data to read {} bytes, got {}",
sizeof(T),
+ data.size());
+ }
+
+ T value;
+ std::memcpy(&value, data.data(), sizeof(T));
+ return FromLittleEndian(value);
+}
+
+template <TypeId type_id>
+Result<std::vector<uint8_t>> ToBytesImpl(const Literal::Value& value) {
+ using CppType = typename LiteralTraits<type_id>::ValueType;
+ return WriteLittleEndian(std::get<CppType>(value));
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBoolean>(const
Literal::Value& value) {
+ return std::vector<uint8_t>{std::get<bool>(value) ?
static_cast<uint8_t>(0x01)
+ :
static_cast<uint8_t>(0x00)};
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kString>(const
Literal::Value& value) {
+ const auto& str = std::get<std::string>(value);
+ return std::vector<uint8_t>(str.begin(), str.end());
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kBinary>(const
Literal::Value& value) {
+ return std::get<std::vector<uint8_t>>(value);
+}
+
+template <>
+Result<std::vector<uint8_t>> ToBytesImpl<TypeId::kFixed>(const Literal::Value&
value) {
+ if (std::holds_alternative<std::vector<uint8_t>>(value)) {
+ return std::get<std::vector<uint8_t>>(value);
+ } else {
+ std::string actual_type =
+ std::visit([](auto&& arg) -> std::string { return typeid(arg).name();
}, value);
+ return InvalidArgument("Invalid value type for Fixed literal, got type:
{}",
+ actual_type);
+ }
+}
+
+#define DISPATCH_LITERAL_TO_BYTES(type_id) \
+ case type_id: \
+ return ToBytesImpl<type_id>(value);
+
+Result<std::vector<uint8_t>> Conversions::ToBytes(const PrimitiveType& type,
+ const Literal::Value& value)
{
+ const auto type_id = type.type_id();
+
+ switch (type_id) {
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kInt)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kDate)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kLong)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTime)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTimestamp)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kTimestampTz)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kFloat)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kDouble)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kBoolean)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kString)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kBinary)
+ DISPATCH_LITERAL_TO_BYTES(TypeId::kFixed)
+ // TODO(Li Feiyang): Add support for UUID and Decimal
+
+ default:
+ return NotSupported("Serialization for type {} is not supported",
type.ToString());
+ }
+}
+
+#undef DISPATCH_LITERAL_TO_BYTES
+
+Result<std::vector<uint8_t>> Conversions::ToBytes(const Literal& literal) {
+ // Cannot serialize special values
+ if (literal.IsAboveMax()) {
+ return NotSupported("Cannot serialize AboveMax");
+ }
+ if (literal.IsBelowMin()) {
+ return NotSupported("Cannot serialize BelowMin");
+ }
+ if (literal.IsNull()) {
+ return NotSupported("Cannot serialize null");
+ }
+
+ return ToBytes(*literal.type(), literal.value());
+}
+
+Result<Literal::Value> Conversions::FromBytes(const PrimitiveType& type,
+ std::span<const uint8_t> data) {
+ if (data.empty()) {
+ return InvalidArgument("Cannot deserialize empty value");
+ }
+
+ const auto type_id = type.type_id();
+
+ switch (type_id) {
+ case TypeId::kBoolean: {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<uint8_t>(data));
+ return Literal::Value{static_cast<bool>(value != 0x00)};
+ }
+
+ case TypeId::kInt: {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<int32_t>(data));
+ return Literal::Value{value};
+ }
+
+ case TypeId::kDate: {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<int32_t>(data));
+ return Literal::Value{value};
+ }
+
+ case TypeId::kLong:
+ case TypeId::kTime:
+ case TypeId::kTimestamp:
+ case TypeId::kTimestampTz: {
+ int64_t value;
+ if (data.size() < 8) {
+ // Type was promoted from int to long
+ ICEBERG_ASSIGN_OR_RAISE(auto int_value,
ReadLittleEndian<int32_t>(data));
+ value = static_cast<int64_t>(int_value);
+ } else {
+ ICEBERG_ASSIGN_OR_RAISE(auto long_value,
ReadLittleEndian<int64_t>(data));
+ value = long_value;
+ }
+ return Literal::Value{value};
+ }
+
+ case TypeId::kFloat: {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, ReadLittleEndian<float>(data));
+ return Literal::Value{value};
+ }
+
+ case TypeId::kDouble: {
+ if (data.size() < 8) {
+ // Type was promoted from float to double
+ ICEBERG_ASSIGN_OR_RAISE(auto float_value,
ReadLittleEndian<float>(data));
+ return Literal::Value{static_cast<double>(float_value)};
+ } else {
+ ICEBERG_ASSIGN_OR_RAISE(auto double_value,
ReadLittleEndian<double>(data));
+ return Literal::Value{double_value};
+ }
+ }
+
+ case TypeId::kString: {
+ return Literal::Value{
+ std::string(reinterpret_cast<const char*>(data.data()),
data.size())};
+ }
Review Comment:
```suggestion
case TypeId::kString:
return Literal::Value{
std::string(reinterpret_cast<const char*>(data.data()),
data.size())};
```
Usually we don't add curly braces to a single-line case.
##########
test/literal_test.cc:
##########
@@ -383,4 +383,211 @@ TEST(LiteralTest, DoubleZeroComparison) {
EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
}
+struct LiteralRoundTripParam {
+ std::string test_name;
+ std::vector<uint8_t> input_bytes;
+ Literal expected_literal;
+ std::shared_ptr<PrimitiveType> type;
+};
+
+class LiteralSerializationParam : public
::testing::TestWithParam<LiteralRoundTripParam> {
+};
+
+TEST_P(LiteralSerializationParam, RoundTrip) {
+ const auto& param = GetParam();
+
+ // Deserialize from bytes
+ Result<Literal> literal_result = Literal::Deserialize(param.input_bytes,
param.type);
+ ASSERT_TRUE(literal_result.has_value())
+ << "Deserialization failed: " << literal_result.error().message;
+
+ // Check type and value
+ EXPECT_EQ(*literal_result, param.expected_literal);
+
+ // Serialize back to bytes
+ Result<std::vector<uint8_t>> bytes_result = literal_result->Serialize();
+ ASSERT_TRUE(bytes_result.has_value())
+ << "Serialization failed: " << bytes_result.error().message;
+ EXPECT_EQ(*bytes_result, param.input_bytes);
+
+ // Deserialize again to verify idempotency
+ Result<Literal> final_literal = Literal::Deserialize(*bytes_result,
param.type);
+ ASSERT_TRUE(final_literal.has_value())
+ << "Final deserialization failed: " << final_literal.error().message;
+ EXPECT_EQ(*final_literal, param.expected_literal);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ BinarySerialization, LiteralSerializationParam,
+ ::testing::Values(
+ // Basic types
+ LiteralRoundTripParam{"BooleanTrue", {1}, Literal::Boolean(true),
boolean()},
+ LiteralRoundTripParam{"BooleanFalse", {0}, Literal::Boolean(false),
boolean()},
+
+ LiteralRoundTripParam{"Int", {32, 0, 0, 0}, Literal::Int(32), int32()},
+ LiteralRoundTripParam{
+ "IntMaxValue", {255, 255, 255, 127}, Literal::Int(2147483647),
int32()},
+ LiteralRoundTripParam{
+ "IntMinValue", {0, 0, 0, 128}, Literal::Int(-2147483648), int32()},
+ LiteralRoundTripParam{
+ "NegativeInt", {224, 255, 255, 255}, Literal::Int(-32), int32()},
+
+ LiteralRoundTripParam{
+ "Long", {32, 0, 0, 0, 0, 0, 0, 0}, Literal::Long(32), int64()},
+ LiteralRoundTripParam{"LongMaxValue",
+ {255, 255, 255, 255, 255, 255, 255, 127},
+
Literal::Long(std::numeric_limits<int64_t>::max()),
+ int64()},
+ LiteralRoundTripParam{"LongMinValue",
+ {0, 0, 0, 0, 0, 0, 0, 128},
+
Literal::Long(std::numeric_limits<int64_t>::min()),
+ int64()},
+ LiteralRoundTripParam{"NegativeLong",
+ {224, 255, 255, 255, 255, 255, 255, 255},
+ Literal::Long(-32),
+ int64()},
+
+ LiteralRoundTripParam{"Float", {0, 0, 128, 63}, Literal::Float(1.0f),
float32()},
+ LiteralRoundTripParam{"FloatNegativeInfinity",
+ {0, 0, 128, 255},
+
Literal::Float(-std::numeric_limits<float>::infinity()),
+ float32()},
+ LiteralRoundTripParam{"FloatMaxValue",
+ {255, 255, 127, 127},
+
Literal::Float(std::numeric_limits<float>::max()),
+ float32()},
+ LiteralRoundTripParam{"FloatMinValue",
+ {255, 255, 127, 255},
+
Literal::Float(std::numeric_limits<float>::lowest()),
+ float32()},
+
+ LiteralRoundTripParam{
+ "Double", {0, 0, 0, 0, 0, 0, 240, 63}, Literal::Double(1.0),
float64()},
+ LiteralRoundTripParam{"DoubleNegativeInfinity",
+ {0, 0, 0, 0, 0, 0, 240, 255},
+
Literal::Double(-std::numeric_limits<double>::infinity()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMaxValue",
+ {255, 255, 255, 255, 255, 255, 239, 127},
+
Literal::Double(std::numeric_limits<double>::max()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMinValue",
+ {255, 255, 255, 255, 255, 255, 239, 255},
+
Literal::Double(std::numeric_limits<double>::lowest()),
+ float64()},
+
+ LiteralRoundTripParam{"String",
+ {105, 99, 101, 98, 101, 114, 103},
+ Literal::String("iceberg"),
+ string()},
+ LiteralRoundTripParam{
+ "StringLong",
+ {65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65},
+ Literal::String("AAAAAAAAAAAAAAAA"),
+ string()},
+
+ LiteralRoundTripParam{"BinaryData",
+ {0x01, 0x02, 0x03, 0xFF},
+ Literal::Binary({0x01, 0x02, 0x03, 0xFF}),
+ binary()},
+ LiteralRoundTripParam{"BinarySingleByte", {42}, Literal::Binary({42}),
binary()},
+
+ // Fixed type
+ LiteralRoundTripParam{"FixedLength4",
+ {0x01, 0x02, 0x03, 0x04},
+ Literal::Fixed({0x01, 0x02, 0x03, 0x04}, 4),
+ fixed(4)},
+ LiteralRoundTripParam{
+ "FixedLength8",
+ {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
+ Literal::Fixed({0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
8),
+ fixed(8)},
+ LiteralRoundTripParam{
+ "FixedLength16",
+ {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A,
0x0B, 0x0C,
+ 0x0D, 0x0E, 0x0F},
+ Literal::Fixed({0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09,
+ 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
+ 16),
+ fixed(16)},
+ LiteralRoundTripParam{
+ "FixedSingleByte", {0xFF}, Literal::Fixed({0xFF}, 1), fixed(1)},
+
+ // Temporal types
+ LiteralRoundTripParam{"DateEpoch", {0, 0, 0, 0}, Literal::Date(0),
date()},
+ LiteralRoundTripParam{"DateNextDay", {1, 0, 0, 0}, Literal::Date(1),
date()},
+ LiteralRoundTripParam{"DateY2K", {205, 42, 0, 0},
Literal::Date(10957), date()},
+ LiteralRoundTripParam{
+ "DateNegative", {255, 255, 255, 255}, Literal::Date(-1), date()},
+
+ LiteralRoundTripParam{
+ "TimeMidnight", {0, 0, 0, 0, 0, 0, 0, 0}, Literal::Time(0),
time()},
+ LiteralRoundTripParam{"TimeNoon",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Time(45045123456),
+ time()},
+ LiteralRoundTripParam{
+ "TimeOneSecond", {64, 66, 15, 0, 0, 0, 0, 0},
Literal::Time(1000000), time()},
+
+ LiteralRoundTripParam{"TimestampEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::Timestamp(0),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampOneSecond",
+ {64, 66, 15, 0, 0, 0, 0, 0},
+ Literal::Timestamp(1000000),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampNoon2024",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Timestamp(45045123456),
+ timestamp()},
+
+ LiteralRoundTripParam{"TimestampTzEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::TimestampTz(0),
+ timestamp_tz()},
+ LiteralRoundTripParam{"TimestampTzOneHour",
+ {0, 164, 147, 214, 0, 0, 0, 0},
+ Literal::TimestampTz(3600000000),
+ timestamp_tz()}),
+
+ [](const testing::TestParamInfo<LiteralSerializationParam::ParamType>&
info) {
+ return info.param.test_name;
+ });
+
+TEST(LiteralSerializationTest, EmptyString) {
+ auto empty_string = Literal::String("");
+ auto empty_bytes = empty_string.Serialize();
+ ASSERT_TRUE(empty_bytes.has_value());
+ EXPECT_TRUE(empty_bytes->empty());
+
+ auto deserialize_result = Literal::Deserialize(*empty_bytes, string());
+ EXPECT_THAT(deserialize_result, IsError(ErrorKind::kInvalidArgument));
+}
+
+// Type promotion tests
+TEST(LiteralSerializationTest, TypePromotion) {
+ // 4-byte int data can be deserialized as long
+ std::vector<uint8_t> int_data = {32, 0, 0, 0};
+ auto long_result = Literal::Deserialize(int_data, int64());
+ ASSERT_TRUE(long_result.has_value());
+ EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
+ EXPECT_EQ(std::get<int64_t>(long_result->value()), 32L);
+
+ auto long_bytes = long_result->Serialize();
+ ASSERT_TRUE(long_bytes.has_value());
+ EXPECT_EQ(long_bytes->size(), 8);
Review Comment:
```suggestion
```
These lines are meaningless to this case.
##########
test/literal_test.cc:
##########
@@ -383,4 +383,211 @@ TEST(LiteralTest, DoubleZeroComparison) {
EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
}
+struct LiteralRoundTripParam {
+ std::string test_name;
+ std::vector<uint8_t> input_bytes;
+ Literal expected_literal;
+ std::shared_ptr<PrimitiveType> type;
+};
+
+class LiteralSerializationParam : public
::testing::TestWithParam<LiteralRoundTripParam> {
+};
+
+TEST_P(LiteralSerializationParam, RoundTrip) {
+ const auto& param = GetParam();
+
+ // Deserialize from bytes
+ Result<Literal> literal_result = Literal::Deserialize(param.input_bytes,
param.type);
+ ASSERT_TRUE(literal_result.has_value())
+ << "Deserialization failed: " << literal_result.error().message;
+
+ // Check type and value
+ EXPECT_EQ(*literal_result, param.expected_literal);
+
+ // Serialize back to bytes
+ Result<std::vector<uint8_t>> bytes_result = literal_result->Serialize();
+ ASSERT_TRUE(bytes_result.has_value())
+ << "Serialization failed: " << bytes_result.error().message;
+ EXPECT_EQ(*bytes_result, param.input_bytes);
+
+ // Deserialize again to verify idempotency
+ Result<Literal> final_literal = Literal::Deserialize(*bytes_result,
param.type);
+ ASSERT_TRUE(final_literal.has_value())
+ << "Final deserialization failed: " << final_literal.error().message;
+ EXPECT_EQ(*final_literal, param.expected_literal);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ BinarySerialization, LiteralSerializationParam,
+ ::testing::Values(
+ // Basic types
+ LiteralRoundTripParam{"BooleanTrue", {1}, Literal::Boolean(true),
boolean()},
+ LiteralRoundTripParam{"BooleanFalse", {0}, Literal::Boolean(false),
boolean()},
+
+ LiteralRoundTripParam{"Int", {32, 0, 0, 0}, Literal::Int(32), int32()},
+ LiteralRoundTripParam{
+ "IntMaxValue", {255, 255, 255, 127}, Literal::Int(2147483647),
int32()},
+ LiteralRoundTripParam{
+ "IntMinValue", {0, 0, 0, 128}, Literal::Int(-2147483648), int32()},
+ LiteralRoundTripParam{
+ "NegativeInt", {224, 255, 255, 255}, Literal::Int(-32), int32()},
+
+ LiteralRoundTripParam{
+ "Long", {32, 0, 0, 0, 0, 0, 0, 0}, Literal::Long(32), int64()},
+ LiteralRoundTripParam{"LongMaxValue",
+ {255, 255, 255, 255, 255, 255, 255, 127},
+
Literal::Long(std::numeric_limits<int64_t>::max()),
+ int64()},
+ LiteralRoundTripParam{"LongMinValue",
+ {0, 0, 0, 0, 0, 0, 0, 128},
+
Literal::Long(std::numeric_limits<int64_t>::min()),
+ int64()},
+ LiteralRoundTripParam{"NegativeLong",
+ {224, 255, 255, 255, 255, 255, 255, 255},
+ Literal::Long(-32),
+ int64()},
+
+ LiteralRoundTripParam{"Float", {0, 0, 128, 63}, Literal::Float(1.0f),
float32()},
+ LiteralRoundTripParam{"FloatNegativeInfinity",
+ {0, 0, 128, 255},
+
Literal::Float(-std::numeric_limits<float>::infinity()),
+ float32()},
+ LiteralRoundTripParam{"FloatMaxValue",
+ {255, 255, 127, 127},
+
Literal::Float(std::numeric_limits<float>::max()),
+ float32()},
+ LiteralRoundTripParam{"FloatMinValue",
+ {255, 255, 127, 255},
+
Literal::Float(std::numeric_limits<float>::lowest()),
+ float32()},
+
+ LiteralRoundTripParam{
+ "Double", {0, 0, 0, 0, 0, 0, 240, 63}, Literal::Double(1.0),
float64()},
+ LiteralRoundTripParam{"DoubleNegativeInfinity",
+ {0, 0, 0, 0, 0, 0, 240, 255},
+
Literal::Double(-std::numeric_limits<double>::infinity()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMaxValue",
+ {255, 255, 255, 255, 255, 255, 239, 127},
+
Literal::Double(std::numeric_limits<double>::max()),
+ float64()},
+ LiteralRoundTripParam{"DoubleMinValue",
+ {255, 255, 255, 255, 255, 255, 239, 255},
+
Literal::Double(std::numeric_limits<double>::lowest()),
+ float64()},
+
+ LiteralRoundTripParam{"String",
+ {105, 99, 101, 98, 101, 114, 103},
+ Literal::String("iceberg"),
+ string()},
+ LiteralRoundTripParam{
+ "StringLong",
+ {65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65, 65},
+ Literal::String("AAAAAAAAAAAAAAAA"),
+ string()},
+
+ LiteralRoundTripParam{"BinaryData",
+ {0x01, 0x02, 0x03, 0xFF},
+ Literal::Binary({0x01, 0x02, 0x03, 0xFF}),
+ binary()},
+ LiteralRoundTripParam{"BinarySingleByte", {42}, Literal::Binary({42}),
binary()},
+
+ // Fixed type
+ LiteralRoundTripParam{"FixedLength4",
+ {0x01, 0x02, 0x03, 0x04},
+ Literal::Fixed({0x01, 0x02, 0x03, 0x04}, 4),
+ fixed(4)},
+ LiteralRoundTripParam{
+ "FixedLength8",
+ {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
+ Literal::Fixed({0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11},
8),
+ fixed(8)},
+ LiteralRoundTripParam{
+ "FixedLength16",
+ {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A,
0x0B, 0x0C,
+ 0x0D, 0x0E, 0x0F},
+ Literal::Fixed({0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09,
+ 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
+ 16),
+ fixed(16)},
+ LiteralRoundTripParam{
+ "FixedSingleByte", {0xFF}, Literal::Fixed({0xFF}, 1), fixed(1)},
+
+ // Temporal types
+ LiteralRoundTripParam{"DateEpoch", {0, 0, 0, 0}, Literal::Date(0),
date()},
+ LiteralRoundTripParam{"DateNextDay", {1, 0, 0, 0}, Literal::Date(1),
date()},
+ LiteralRoundTripParam{"DateY2K", {205, 42, 0, 0},
Literal::Date(10957), date()},
+ LiteralRoundTripParam{
+ "DateNegative", {255, 255, 255, 255}, Literal::Date(-1), date()},
+
+ LiteralRoundTripParam{
+ "TimeMidnight", {0, 0, 0, 0, 0, 0, 0, 0}, Literal::Time(0),
time()},
+ LiteralRoundTripParam{"TimeNoon",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Time(45045123456),
+ time()},
+ LiteralRoundTripParam{
+ "TimeOneSecond", {64, 66, 15, 0, 0, 0, 0, 0},
Literal::Time(1000000), time()},
+
+ LiteralRoundTripParam{"TimestampEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::Timestamp(0),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampOneSecond",
+ {64, 66, 15, 0, 0, 0, 0, 0},
+ Literal::Timestamp(1000000),
+ timestamp()},
+ LiteralRoundTripParam{"TimestampNoon2024",
+ {128, 9, 230, 124, 10, 0, 0, 0},
+ Literal::Timestamp(45045123456),
+ timestamp()},
+
+ LiteralRoundTripParam{"TimestampTzEpoch",
+ {0, 0, 0, 0, 0, 0, 0, 0},
+ Literal::TimestampTz(0),
+ timestamp_tz()},
+ LiteralRoundTripParam{"TimestampTzOneHour",
+ {0, 164, 147, 214, 0, 0, 0, 0},
+ Literal::TimestampTz(3600000000),
+ timestamp_tz()}),
+
+ [](const testing::TestParamInfo<LiteralSerializationParam::ParamType>&
info) {
+ return info.param.test_name;
+ });
+
+TEST(LiteralSerializationTest, EmptyString) {
+ auto empty_string = Literal::String("");
+ auto empty_bytes = empty_string.Serialize();
+ ASSERT_TRUE(empty_bytes.has_value());
+ EXPECT_TRUE(empty_bytes->empty());
+
+ auto deserialize_result = Literal::Deserialize(*empty_bytes, string());
+ EXPECT_THAT(deserialize_result, IsError(ErrorKind::kInvalidArgument));
+}
+
+// Type promotion tests
+TEST(LiteralSerializationTest, TypePromotion) {
+ // 4-byte int data can be deserialized as long
+ std::vector<uint8_t> int_data = {32, 0, 0, 0};
+ auto long_result = Literal::Deserialize(int_data, int64());
+ ASSERT_TRUE(long_result.has_value());
+ EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong);
+ EXPECT_EQ(std::get<int64_t>(long_result->value()), 32L);
+
+ auto long_bytes = long_result->Serialize();
+ ASSERT_TRUE(long_bytes.has_value());
+ EXPECT_EQ(long_bytes->size(), 8);
+
+ // 4-byte float data can be deserialized as double
+ std::vector<uint8_t> float_data = {0, 0, 128, 63};
+ auto double_result = Literal::Deserialize(float_data, float64());
+ ASSERT_TRUE(double_result.has_value());
+ EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble);
+ EXPECT_EQ(std::get<double>(double_result->value()), 1.0);
Review Comment:
```suggestion
EXPECT_DOUBLE_EQ(std::get<double>(double_result->value()), 1.0);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
