Skip to content

Commit 694cb55

Browse files
mapleFUmapleFU
committed
Apply suggestions and create PrimitiveLiteralCaster
1 parent 88f67dc commit 694cb55

File tree

3 files changed

+161
-125
lines changed

3 files changed

+161
-125
lines changed

‎src/iceberg/expression/literal.cc‎

Lines changed: 141 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -27,102 +27,49 @@
2727

2828
namespace iceberg {
2929

30-
// Constructor
31-
PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value,
32-
std::shared_ptr<PrimitiveType> type)
33-
: value_(std::move(value)), type_(std::move(type)) {}
34-
35-
// Factory methods
36-
PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) {
37-
return {PrimitiveLiteralValue{value}, std::make_shared<BooleanType>()};
38-
}
39-
40-
PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) {
41-
return {PrimitiveLiteralValue{value}, std::make_shared<IntType>()};
42-
}
43-
44-
PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) {
45-
return {PrimitiveLiteralValue{value}, std::make_shared<LongType>()};
46-
}
47-
48-
PrimitiveLiteral PrimitiveLiteral::Float(float value) {
49-
return {PrimitiveLiteralValue{value}, std::make_shared<FloatType>()};
50-
}
51-
52-
PrimitiveLiteral PrimitiveLiteral::Double(double value) {
53-
return {PrimitiveLiteralValue{value}, std::make_shared<DoubleType>()};
30+
/// \brief PrimitiveLiteralCaster handles type casting operations for PrimitiveLiteral.
31+
/// This is an internal implementation class.
32+
class PrimitiveLiteralCaster {
33+
public:
34+
/// Cast a PrimitiveLiteral to the target type.
35+
static Result<PrimitiveLiteral> CastTo(
36+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type);
37+
38+
/// Create a literal representing a value below the minimum for the given type.
39+
static PrimitiveLiteral BelowMinLiteral(std::shared_ptr<PrimitiveType> type);
40+
41+
/// Create a literal representing a value above the maximum for the given type.
42+
static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr<PrimitiveType> type);
43+
44+
private:
45+
/// Cast from Int type to target type.
46+
static Result<PrimitiveLiteral> CastFromInt(
47+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type);
48+
49+
/// Cast from Long type to target type.
50+
static Result<PrimitiveLiteral> CastFromLong(
51+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type);
52+
53+
/// Cast from Float type to target type.
54+
static Result<PrimitiveLiteral> CastFromFloat(
55+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type);
56+
};
57+
58+
PrimitiveLiteral PrimitiveLiteralCaster::BelowMinLiteral(
59+
std::shared_ptr<PrimitiveType> type) {
60+
return PrimitiveLiteral(PrimitiveLiteral::BelowMin{}, std::move(type));
5461
}
5562

56-
PrimitiveLiteral PrimitiveLiteral::String(std::string value) {
57-
return {PrimitiveLiteralValue{std::move(value)}, std::make_shared<StringType>()};
63+
PrimitiveLiteral PrimitiveLiteralCaster::AboveMaxLiteral(
64+
std::shared_ptr<PrimitiveType> type) {
65+
return PrimitiveLiteral(PrimitiveLiteral::AboveMax{}, std::move(type));
5866
}
5967

60-
PrimitiveLiteral PrimitiveLiteral::Binary(std::vector<uint8_t> value) {
61-
return {PrimitiveLiteralValue{std::move(value)}, std::make_shared<BinaryType>()};
62-
}
63-
64-
PrimitiveLiteral PrimitiveLiteral::BelowMinLiteral(std::shared_ptr<PrimitiveType> type) {
65-
return {PrimitiveLiteralValue{BelowMin{}}, std::move(type)};
66-
}
67-
68-
PrimitiveLiteral PrimitiveLiteral::AboveMaxLiteral(std::shared_ptr<PrimitiveType> type) {
69-
return {PrimitiveLiteralValue{AboveMax{}}, std::move(type)};
70-
}
71-
72-
Result<PrimitiveLiteral> PrimitiveLiteral::Deserialize(std::span<const uint8_t> data) {
73-
return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet");
74-
}
75-
76-
Result<std::vector<uint8_t>> PrimitiveLiteral::Serialize() const {
77-
return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet");
78-
}
79-
80-
// Getters
81-
82-
const std::shared_ptr<PrimitiveType>& PrimitiveLiteral::type() const { return type_; }
83-
84-
// Cast method
85-
Result<PrimitiveLiteral> PrimitiveLiteral::CastTo(
86-
const std::shared_ptr<PrimitiveType>& target_type) const {
87-
if (*type_ == *target_type) {
88-
// If types are the same, return a copy of the current literal
89-
return PrimitiveLiteral(value_, target_type);
90-
}
91-
92-
// Handle special values
93-
if (std::holds_alternative<BelowMin>(value_) ||
94-
std::holds_alternative<AboveMax>(value_)) {
95-
// Cannot cast type for special values
96-
return NotSupported("Cannot cast type for {}", ToString());
97-
}
98-
99-
auto source_type_id = type_->type_id();
68+
Result<PrimitiveLiteral> PrimitiveLiteralCaster::CastFromInt(
69+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type) {
70+
auto int_val = std::get<int32_t>(literal.value_);
10071
auto target_type_id = target_type->type_id();
10172

102-
// Delegate to specific cast functions based on source type
103-
switch (source_type_id) {
104-
case TypeId::kInt:
105-
return CastFromInt(target_type_id);
106-
case TypeId::kLong:
107-
return CastFromLong(target_type_id);
108-
case TypeId::kFloat:
109-
return CastFromFloat(target_type_id);
110-
case TypeId::kDouble:
111-
case TypeId::kBoolean:
112-
case TypeId::kString:
113-
case TypeId::kBinary:
114-
break;
115-
default:
116-
break;
117-
}
118-
119-
return NotSupported("Cast from {} to {} is not implemented", type_->ToString(),
120-
target_type->ToString());
121-
}
122-
123-
Result<PrimitiveLiteral> PrimitiveLiteral::CastFromInt(TypeId target_type_id) const {
124-
auto int_val = std::get<int32_t>(value_);
125-
12673
switch (target_type_id) {
12774
case TypeId::kLong:
12875
return PrimitiveLiteral::Long(static_cast<int64_t>(int_val));
@@ -137,17 +84,19 @@ Result<PrimitiveLiteral> PrimitiveLiteral::CastFromInt(TypeId target_type_id) co
13784
}
13885
}
13986

140-
Result<PrimitiveLiteral> PrimitiveLiteral::CastFromLong(TypeId target_type_id) const {
141-
auto long_val = std::get<int64_t>(value_);
87+
Result<PrimitiveLiteral> PrimitiveLiteralCaster::CastFromLong(
88+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type) {
89+
auto long_val = std::get<int64_t>(literal.value_);
90+
auto target_type_id = target_type->type_id();
14291

14392
switch (target_type_id) {
14493
case TypeId::kInt: {
14594
// Check for overflow
14695
if (long_val >= std::numeric_limits<int32_t>::max()) {
147-
return PrimitiveLiteral::AboveMaxLiteral(type_);
96+
return AboveMaxLiteral(target_type);
14897
}
14998
if (long_val <= std::numeric_limits<int32_t>::min()) {
150-
return PrimitiveLiteral::BelowMinLiteral(type_);
99+
return BelowMinLiteral(target_type);
151100
}
152101
return PrimitiveLiteral::Int(static_cast<int32_t>(long_val));
153102
}
@@ -161,8 +110,10 @@ Result<PrimitiveLiteral> PrimitiveLiteral::CastFromLong(TypeId target_type_id) c
161110
}
162111
}
163112

164-
Result<PrimitiveLiteral> PrimitiveLiteral::CastFromFloat(TypeId target_type_id) const {
165-
auto float_val = std::get<float>(value_);
113+
Result<PrimitiveLiteral> PrimitiveLiteralCaster::CastFromFloat(
114+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type) {
115+
auto float_val = std::get<float>(literal.value_);
116+
auto target_type_id = target_type->type_id();
166117

167118
switch (target_type_id) {
168119
case TypeId::kDouble:
@@ -173,6 +124,58 @@ Result<PrimitiveLiteral> PrimitiveLiteral::CastFromFloat(TypeId target_type_id)
173124
}
174125
}
175126

127+
// Constructor
128+
PrimitiveLiteral::PrimitiveLiteral(Value value, std::shared_ptr<PrimitiveType> type)
129+
: value_(std::move(value)), type_(std::move(type)) {}
130+
131+
// Factory methods
132+
PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) {
133+
return {Value{value}, std::make_shared<BooleanType>()};
134+
}
135+
136+
PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) {
137+
return {Value{value}, std::make_shared<IntType>()};
138+
}
139+
140+
PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) {
141+
return {Value{value}, std::make_shared<LongType>()};
142+
}
143+
144+
PrimitiveLiteral PrimitiveLiteral::Float(float value) {
145+
return {Value{value}, std::make_shared<FloatType>()};
146+
}
147+
148+
PrimitiveLiteral PrimitiveLiteral::Double(double value) {
149+
return {Value{value}, std::make_shared<DoubleType>()};
150+
}
151+
152+
PrimitiveLiteral PrimitiveLiteral::String(std::string value) {
153+
return {Value{std::move(value)}, std::make_shared<StringType>()};
154+
}
155+
156+
PrimitiveLiteral PrimitiveLiteral::Binary(std::vector<uint8_t> value) {
157+
return {Value{std::move(value)}, std::make_shared<BinaryType>()};
158+
}
159+
160+
Result<PrimitiveLiteral> PrimitiveLiteral::Deserialize(
161+
std::span<const uint8_t> data, std::shared_ptr<PrimitiveType> type) {
162+
return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet");
163+
}
164+
165+
Result<std::vector<uint8_t>> PrimitiveLiteral::Serialize() const {
166+
return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet");
167+
}
168+
169+
// Getters
170+
171+
const std::shared_ptr<PrimitiveType>& PrimitiveLiteral::type() const { return type_; }
172+
173+
// Cast method
174+
Result<PrimitiveLiteral> PrimitiveLiteral::CastTo(
175+
const std::shared_ptr<PrimitiveType>& target_type) const {
176+
return PrimitiveLiteralCaster::CastTo(*this, target_type);
177+
}
178+
176179
// Template function for floating point comparison following Iceberg rules:
177180
// -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign
178181
template <std::floating_point T>
@@ -205,7 +208,7 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe
205208
}
206209

207210
// If either value is AboveMax or BelowMin, comparison is unordered
208-
if (isAboveMax() || isBelowMin() || other.isAboveMax() || other.isBelowMin()) {
211+
if (IsAboveMax() || IsBelowMin() || other.IsAboveMax() || other.IsBelowMin()) {
209212
return std::partial_ordering::unordered;
210213
}
211214

@@ -313,12 +316,51 @@ std::string PrimitiveLiteral::ToString() const {
313316
}
314317
}
315318

316-
bool PrimitiveLiteral::isBelowMin() const {
319+
bool PrimitiveLiteral::IsBelowMin() const {
317320
return std::holds_alternative<BelowMin>(value_);
318321
}
319322

320-
bool PrimitiveLiteral::isAboveMax() const {
323+
bool PrimitiveLiteral::IsAboveMax() const {
321324
return std::holds_alternative<AboveMax>(value_);
322325
}
323326

327+
// PrimitiveLiteralCaster implementation
328+
329+
Result<PrimitiveLiteral> PrimitiveLiteralCaster::CastTo(
330+
const PrimitiveLiteral& literal, const std::shared_ptr<PrimitiveType>& target_type) {
331+
if (*literal.type_ == *target_type) {
332+
// If types are the same, return a copy of the current literal
333+
return PrimitiveLiteral(literal.value_, target_type);
334+
}
335+
336+
// Handle special values
337+
if (std::holds_alternative<PrimitiveLiteral::BelowMin>(literal.value_) ||
338+
std::holds_alternative<PrimitiveLiteral::AboveMax>(literal.value_)) {
339+
// Cannot cast type for special values
340+
return NotSupported("Cannot cast type for {}", literal.ToString());
341+
}
342+
343+
auto source_type_id = literal.type_->type_id();
344+
345+
// Delegate to specific cast functions based on source type
346+
switch (source_type_id) {
347+
case TypeId::kInt:
348+
return CastFromInt(literal, target_type);
349+
case TypeId::kLong:
350+
return CastFromLong(literal, target_type);
351+
case TypeId::kFloat:
352+
return CastFromFloat(literal, target_type);
353+
case TypeId::kDouble:
354+
case TypeId::kBoolean:
355+
case TypeId::kString:
356+
case TypeId::kBinary:
357+
break;
358+
default:
359+
break;
360+
}
361+
362+
return NotSupported("Cast from {} to {} is not implemented", literal.type_->ToString(),
363+
target_type->ToString());
364+
}
365+
324366
} // namespace iceberg

‎src/iceberg/expression/literal.h‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,15 @@ class ICEBERG_EXPORT PrimitiveLiteral {
5353
std::strong_ordering operator<=>(const AboveMax&) const = default;
5454
};
5555

56-
using PrimitiveLiteralValue =
57-
std::variant<bool, // for boolean
58-
int32_t, // for int, date
59-
int64_t, // for long, timestamp, timestamp_tz, time
60-
float, // for float
61-
double, // for double
62-
std::string, // for string
63-
std::vector<uint8_t>, // for binary, fixed
64-
std::array<uint8_t, 16>, // for uuid and decimal
65-
BelowMin, AboveMax>;
56+
using Value = std::variant<bool, // for boolean
57+
int32_t, // for int, date
58+
int64_t, // for long, timestamp, timestamp_tz, time
59+
float, // for float
60+
double, // for double
61+
std::string, // for string
62+
std::vector<uint8_t>, // for binary, fixed
63+
std::array<uint8_t, 16>, // for uuid and decimal
64+
BelowMin, AboveMax>;
6665

6766
public:
6867
/// Factory methods for primitive types
@@ -78,7 +77,8 @@ class ICEBERG_EXPORT PrimitiveLiteral {
7877
///
7978
/// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization)
8079
/// for reference.
81-
static Result<PrimitiveLiteral> Deserialize(std::span<const uint8_t> data);
80+
static Result<PrimitiveLiteral> Deserialize(std::span<const uint8_t> data,
81+
std::shared_ptr<PrimitiveType> type);
8282

8383
/// Serialize iceberg literal to bytes.
8484
///
@@ -114,24 +114,18 @@ class ICEBERG_EXPORT PrimitiveLiteral {
114114
/// and should not be AboveMax or BelowMin.
115115
std::partial_ordering operator<=>(const PrimitiveLiteral& other) const;
116116

117-
bool isAboveMax() const;
118-
bool isBelowMin() const;
117+
bool IsAboveMax() const;
118+
bool IsBelowMin() const;
119119

120120
std::string ToString() const;
121121

122122
private:
123-
PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr<PrimitiveType> type);
123+
PrimitiveLiteral(Value value, std::shared_ptr<PrimitiveType> type);
124124

125-
static PrimitiveLiteral BelowMinLiteral(std::shared_ptr<PrimitiveType> type);
126-
static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr<PrimitiveType> type);
127-
128-
// Helper methods for type casting
129-
Result<PrimitiveLiteral> CastFromInt(TypeId target_type_id) const;
130-
Result<PrimitiveLiteral> CastFromLong(TypeId target_type_id) const;
131-
Result<PrimitiveLiteral> CastFromFloat(TypeId target_type_id) const;
125+
friend class PrimitiveLiteralCaster;
132126

133127
private:
134-
PrimitiveLiteralValue value_;
128+
Value value_;
135129
std::shared_ptr<PrimitiveType> type_;
136130
};
137131

‎test/literal_test.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,11 @@ TEST(PrimitiveLiteralTest, LongCastToIntOverflow) {
145145

146146
auto max_result = max_long.CastTo(std::make_shared<IntType>());
147147
ASSERT_THAT(max_result, IsOk());
148-
EXPECT_TRUE(max_result->isAboveMax());
148+
EXPECT_TRUE(max_result->IsAboveMax());
149149

150150
auto min_result = min_long.CastTo(std::make_shared<IntType>());
151151
ASSERT_THAT(min_result, IsOk());
152-
EXPECT_TRUE(min_result->isBelowMin());
152+
EXPECT_TRUE(min_result->IsBelowMin());
153153
}
154154

155155
// Float type tests
@@ -267,8 +267,8 @@ TEST(PrimitiveLiteralTest, CrossTypeComparison) {
267267
TEST(PrimitiveLiteralTest, SpecialValues) {
268268
auto int_literal = PrimitiveLiteral::Int(42);
269269

270-
EXPECT_FALSE(int_literal.isAboveMax());
271-
EXPECT_FALSE(int_literal.isBelowMin());
270+
EXPECT_FALSE(int_literal.IsAboveMax());
271+
EXPECT_FALSE(int_literal.IsBelowMin());
272272
}
273273

274274
// Same type cast test

0 commit comments

Comments
 (0)