From 59f60dab21cb05ea903f40e8c3149299b04b35fc Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 4 Jun 2025 23:10:15 +0800 Subject: [PATCH 01/19] init templates --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/datum.cc | 101 +++++++++++++++++++++++++++++++++++++ src/iceberg/datum.h | 95 ++++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+) create mode 100644 src/iceberg/datum.cc create mode 100644 src/iceberg/datum.h diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 09b419e7..3ff9d096 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -21,6 +21,7 @@ set(ICEBERG_SOURCES arrow_c_data_internal.cc catalog/in_memory_catalog.cc demo.cc + datum.cc expression/expression.cc file_reader.cc json_internal.cc diff --git a/src/iceberg/datum.cc b/src/iceberg/datum.cc new file mode 100644 index 00000000..15bf498d --- /dev/null +++ b/src/iceberg/datum.cc @@ -0,0 +1,101 @@ +/* + * 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/datum.h" +#include "iceberg/exception.h" + +#include + +namespace iceberg { + +// Constructor +PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type) + : value_(std::move(value)), type_(std::move(type)) {} + +// Factory methods +PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) { + return PrimitiveLiteral(value, std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::Integer(int32_t value) { + return PrimitiveLiteral(value, std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) { + return PrimitiveLiteral(value, std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::Float(float value) { + return PrimitiveLiteral(value, std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::Double(double value) { + return PrimitiveLiteral(value, std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::String(std::string value) { + return PrimitiveLiteral(std::move(value), std::make_shared()); +} + +PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { + return PrimitiveLiteral(std::move(value), std::make_shared()); +} + +Result PrimitiveLiteral::Deserialize(std::span data) { + return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet"); +} + +Result> PrimitiveLiteral::Serialize() const { + return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet"); +} + +// Getters +const PrimitiveLiteralValue& PrimitiveLiteral::value() const { + return value_; +} + +const std::shared_ptr& PrimitiveLiteral::type() const { + return type_; +} + +// Cast method +Result PrimitiveLiteral::CastTo(const std::shared_ptr& target_type) const { + if (*type_ == *target_type) { + // If types are the same, return a copy of the current literal + return PrimitiveLiteral(value_, target_type); + } + + return NotImplemented("Cast from {} to {} is not implemented", + type_->ToString(), target_type->ToString()); + +} + +// Three-way comparison operator +std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& other) const { + // If types are different, comparison is unordered + if (type_->type_id() != other.type_->type_id()) { + return std::partial_ordering::unordered; + } + if (value_ == other.value_) { + return std::partial_ordering::equivalent; + } + throw IcebergError("Not implemented: comparison between different primitive types"); +} + +} // namespace iceberg \ No newline at end of file diff --git a/src/iceberg/datum.h b/src/iceberg/datum.h new file mode 100644 index 00000000..d32055b0 --- /dev/null +++ b/src/iceberg/datum.h @@ -0,0 +1,95 @@ +/* +* 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. + */ + +#pragma once + +#include +#include +#include +#include +#include + +#include "iceberg/type.h" +#include "iceberg/result.h" + +namespace iceberg { + +/// \brief Exception type for values that are below the minimum allowed value for a primitive type. +/// +/// When casting a value to a narrow primitive type, if the value exceeds the maximum of dest type, +/// it might be above the maximum allowed value for that type. +struct BelowMin { + bool operator==(const BelowMin&) const = default; + std::strong_ordering operator<=>(const BelowMin&) const = default; +}; + +/// \brief Exception type for values that are above the maximum allowed value for a primitive type. +/// +/// When casting a value to a narrow primitive type, if the value exceeds the maximum of dest type, +/// it might be above the maximum allowed value for that type. +struct AboveMax { + bool operator==(const AboveMax&) const = default; + std::strong_ordering operator<=>(const AboveMax&) const = default; +}; + +// TODO(mwish): Supports More types +using PrimitiveLiteralValue = + std::variant, BelowMin, AboveMax>; + +/// \brief PrimitiveLiteral is owned literal of a primitive type. +class PrimitiveLiteral { + public: + explicit PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type); + + // Factory methods for primitive types + static PrimitiveLiteral Boolean(bool value); + static PrimitiveLiteral Integer(int32_t value); + static PrimitiveLiteral Long(int64_t value); + static PrimitiveLiteral Float(float value); + static PrimitiveLiteral Double(double value); + static PrimitiveLiteral String(std::string value); + static PrimitiveLiteral Binary(std::vector value); + + /// Create iceberg value from bytes. + /// + /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) for reference. + static Result Deserialize(std::span data); + /// Serialize iceberg value to bytes. + /// + /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) for reference. + Result> Serialize() const; + + // Get the value as a variant + const PrimitiveLiteralValue& value() const; + + // Get the Iceberg Type of the literal + const std::shared_ptr& type() const; + + // Cast the literal to a specific type + Result CastTo(const std::shared_ptr& target_type) const; + + std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; + + private: + PrimitiveLiteralValue value_; + std::shared_ptr type_; +}; + +} // namespace iceberg + From 9355670dd6ed72c454efa9550115f4ec1a299bfc Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 4 Jun 2025 23:37:42 +0800 Subject: [PATCH 02/19] some minor enhancement --- src/iceberg/datum.cc | 6 +++++- src/iceberg/datum.h | 18 +++++++++++++----- src/iceberg/expression/expression.h | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/iceberg/datum.cc b/src/iceberg/datum.cc index 15bf498d..4f788b1c 100644 --- a/src/iceberg/datum.cc +++ b/src/iceberg/datum.cc @@ -98,4 +98,8 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe throw IcebergError("Not implemented: comparison between different primitive types"); } -} // namespace iceberg \ No newline at end of file +std::string PrimitiveLiteral::ToString() const { + throw NotImplemented("ToString for PrimitiveLiteral is not implemented yet"); +} + +} // namespace iceberg diff --git a/src/iceberg/datum.h b/src/iceberg/datum.h index d32055b0..7230c42c 100644 --- a/src/iceberg/datum.h +++ b/src/iceberg/datum.h @@ -48,9 +48,15 @@ struct AboveMax { std::strong_ordering operator<=>(const AboveMax&) const = default; }; -// TODO(mwish): Supports More types using PrimitiveLiteralValue = - std::variant, BelowMin, AboveMax>; + std::variant, // for binary, fixed, decimal and uuid + BelowMin, AboveMax>; /// \brief PrimitiveLiteral is owned literal of a primitive type. class PrimitiveLiteral { @@ -75,17 +81,19 @@ class PrimitiveLiteral { /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) for reference. Result> Serialize() const; - // Get the value as a variant + /// Get the value as a variant const PrimitiveLiteralValue& value() const; - // Get the Iceberg Type of the literal + /// Get the Iceberg Type of the literal const std::shared_ptr& type() const; - // Cast the literal to a specific type + /// Cast the literal to a specific type Result CastTo(const std::shared_ptr& target_type) const; std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; + std::string ToString() const; + private: PrimitiveLiteralValue value_; std::shared_ptr type_; diff --git a/src/iceberg/expression/expression.h b/src/iceberg/expression/expression.h index 258c9ee2..342122f4 100644 --- a/src/iceberg/expression/expression.h +++ b/src/iceberg/expression/expression.h @@ -19,7 +19,7 @@ #pragma once -/// \file iceberg/expression.h +/// \file iceberg/expression/expression.h /// Expression interface for Iceberg table operations. #include From 082c7e15074913bf052a4efe747391e8e69fb3d6 Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 4 Jun 2025 23:40:17 +0800 Subject: [PATCH 03/19] try to fix lint --- src/iceberg/datum.cc | 22 ++++++++++------------ src/iceberg/datum.h | 31 ++++++++++++++++++------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/iceberg/datum.cc b/src/iceberg/datum.cc index 4f788b1c..3a42a3fe 100644 --- a/src/iceberg/datum.cc +++ b/src/iceberg/datum.cc @@ -18,14 +18,16 @@ */ #include "iceberg/datum.h" -#include "iceberg/exception.h" #include +#include "iceberg/exception.h" + namespace iceberg { // Constructor -PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type) +PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value, + std::shared_ptr type) : value_(std::move(value)), type_(std::move(type)) {} // Factory methods @@ -66,24 +68,20 @@ Result> PrimitiveLiteral::Serialize() const { } // Getters -const PrimitiveLiteralValue& PrimitiveLiteral::value() const { - return value_; -} +const PrimitiveLiteralValue& PrimitiveLiteral::value() const { return value_; } -const std::shared_ptr& PrimitiveLiteral::type() const { - return type_; -} +const std::shared_ptr& PrimitiveLiteral::type() const { return type_; } // Cast method -Result PrimitiveLiteral::CastTo(const std::shared_ptr& target_type) const { +Result PrimitiveLiteral::CastTo( + const std::shared_ptr& target_type) const { if (*type_ == *target_type) { // If types are the same, return a copy of the current literal return PrimitiveLiteral(value_, target_type); } - return NotImplemented("Cast from {} to {} is not implemented", - type_->ToString(), target_type->ToString()); - + return NotImplemented("Cast from {} to {} is not implemented", type_->ToString(), + target_type->ToString()); } // Three-way comparison operator diff --git a/src/iceberg/datum.h b/src/iceberg/datum.h index 7230c42c..c7351b90 100644 --- a/src/iceberg/datum.h +++ b/src/iceberg/datum.h @@ -1,5 +1,5 @@ /* -* Licensed to the Apache Software Foundation (ASF) under one + * 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 @@ -25,24 +25,26 @@ #include #include -#include "iceberg/type.h" #include "iceberg/result.h" +#include "iceberg/type.h" namespace iceberg { -/// \brief Exception type for values that are below the minimum allowed value for a primitive type. +/// \brief Exception type for values that are below the minimum allowed value for a +/// primitive type. /// -/// When casting a value to a narrow primitive type, if the value exceeds the maximum of dest type, -/// it might be above the maximum allowed value for that type. +/// When casting a value to a narrow primitive type, if the value exceeds the maximum of +/// dest type, it might be above the maximum allowed value for that type. struct BelowMin { bool operator==(const BelowMin&) const = default; std::strong_ordering operator<=>(const BelowMin&) const = default; }; -/// \brief Exception type for values that are above the maximum allowed value for a primitive type. +/// \brief Exception type for values that are above the maximum allowed value for a +/// primitive type. /// -/// When casting a value to a narrow primitive type, if the value exceeds the maximum of dest type, -/// it might be above the maximum allowed value for that type. +/// When casting a value to a narrow primitive type, if the value exceeds the maximum of +/// dest type, it might be above the maximum allowed value for that type. struct AboveMax { bool operator==(const AboveMax&) const = default; std::strong_ordering operator<=>(const AboveMax&) const = default; @@ -61,7 +63,8 @@ using PrimitiveLiteralValue = /// \brief PrimitiveLiteral is owned literal of a primitive type. class PrimitiveLiteral { public: - explicit PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type); + explicit PrimitiveLiteral(PrimitiveLiteralValue value, + std::shared_ptr type); // Factory methods for primitive types static PrimitiveLiteral Boolean(bool value); @@ -74,11 +77,13 @@ class PrimitiveLiteral { /// Create iceberg value from bytes. /// - /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) for reference. + /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) + /// for reference. static Result Deserialize(std::span data); /// Serialize iceberg value to bytes. /// - /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) for reference. + /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) + /// for reference. Result> Serialize() const; /// Get the value as a variant @@ -88,7 +93,8 @@ class PrimitiveLiteral { const std::shared_ptr& type() const; /// Cast the literal to a specific type - Result CastTo(const std::shared_ptr& target_type) const; + Result CastTo( + const std::shared_ptr& target_type) const; std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; @@ -100,4 +106,3 @@ class PrimitiveLiteral { }; } // namespace iceberg - From e1dd11d2753ccbe9be95447d5b241c3c25e2697f Mon Sep 17 00:00:00 2001 From: mwish Date: Thu, 5 Jun 2025 22:56:04 +0800 Subject: [PATCH 04/19] Resolve comments and add impl for compare / cast --- src/iceberg/datum.cc | 233 ++++++++++++++++++++++++++++++++++++++++--- src/iceberg/datum.h | 86 ++++++++-------- 2 files changed, 265 insertions(+), 54 deletions(-) diff --git a/src/iceberg/datum.cc b/src/iceberg/datum.cc index 3a42a3fe..120dba0e 100644 --- a/src/iceberg/datum.cc +++ b/src/iceberg/datum.cc @@ -32,31 +32,39 @@ PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value, // Factory methods PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) { - return PrimitiveLiteral(value, std::make_shared()); + return {PrimitiveLiteralValue{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Integer(int32_t value) { - return PrimitiveLiteral(value, std::make_shared()); +PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) { + return {PrimitiveLiteralValue{value}, std::make_shared()}; } PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) { - return PrimitiveLiteral(value, std::make_shared()); + return {PrimitiveLiteralValue{value}, std::make_shared()}; } PrimitiveLiteral PrimitiveLiteral::Float(float value) { - return PrimitiveLiteral(value, std::make_shared()); + return {PrimitiveLiteralValue{value}, std::make_shared()}; } PrimitiveLiteral PrimitiveLiteral::Double(double value) { - return PrimitiveLiteral(value, std::make_shared()); + return {PrimitiveLiteralValue{value}, std::make_shared()}; } PrimitiveLiteral PrimitiveLiteral::String(std::string value) { - return PrimitiveLiteral(std::move(value), std::make_shared()); + return {PrimitiveLiteralValue{std::move(value)}, std::make_shared()}; } PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { - return PrimitiveLiteral(std::move(value), std::make_shared()); + return {PrimitiveLiteralValue{std::move(value)}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::BelowMinLiteral(std::shared_ptr type) { + return PrimitiveLiteral(PrimitiveLiteralValue{BelowMin{}}, std::move(type)); +} + +PrimitiveLiteral PrimitiveLiteral::AboveMaxLiteral(std::shared_ptr type) { + return PrimitiveLiteral(PrimitiveLiteralValue{AboveMax{}}, std::move(type)); } Result PrimitiveLiteral::Deserialize(std::span data) { @@ -68,7 +76,6 @@ Result> PrimitiveLiteral::Serialize() const { } // Getters -const PrimitiveLiteralValue& PrimitiveLiteral::value() const { return value_; } const std::shared_ptr& PrimitiveLiteral::type() const { return type_; } @@ -80,8 +87,109 @@ Result PrimitiveLiteral::CastTo( return PrimitiveLiteral(value_, target_type); } - return NotImplemented("Cast from {} to {} is not implemented", type_->ToString(), - target_type->ToString()); + // Handle special values + if (std::holds_alternative(value_) || + std::holds_alternative(value_)) { + // Cannot cast type for special values + return NotSupported("Cannot cast type for {}", ToString()); + } + + auto source_type_id = type_->type_id(); + auto target_type_id = target_type->type_id(); + + // Delegate to specific cast functions based on source type + switch (source_type_id) { + case TypeId::kInt: + return CastFromInt(target_type_id); + case TypeId::kLong: + return CastFromLong(target_type_id); + case TypeId::kFloat: + return CastFromFloat(target_type_id); + case TypeId::kDouble: + return CastFromDouble(target_type_id); + case TypeId::kBoolean: + case TypeId::kString: + case TypeId::kBinary: + // These types only support conversion to string (handled above) + break; + default: + break; + } + + return NotSupported("Cast from {} to {} is not implemented", type_->ToString(), + target_type->ToString()); +} + +Result PrimitiveLiteral::CastFromInt(TypeId target_type_id) const { + auto int_val = std::get(value_); + + switch (target_type_id) { + case TypeId::kLong: + return PrimitiveLiteral::Long(static_cast(int_val)); + case TypeId::kFloat: + return PrimitiveLiteral::Float(static_cast(int_val)); + case TypeId::kDouble: + return PrimitiveLiteral::Double(static_cast(int_val)); + // TODO(mwish): Supports casts to date and literal + default: + return NotSupported("Cast from Int to {} is not implemented", + static_cast(target_type_id)); + } +} + +Result PrimitiveLiteral::CastFromLong(TypeId target_type_id) const { + auto long_val = std::get(value_); + + switch (target_type_id) { + case TypeId::kInt: { + // Check for overflow + if (long_val >= std::numeric_limits::max()) { + return PrimitiveLiteral::AboveMaxLiteral(type_); + } + if (long_val <= std::numeric_limits::min()) { + return PrimitiveLiteral::BelowMinLiteral(type_); + } + return PrimitiveLiteral::Int(static_cast(long_val)); + } + case TypeId::kFloat: + return PrimitiveLiteral::Float(static_cast(long_val)); + case TypeId::kDouble: + return PrimitiveLiteral::Double(static_cast(long_val)); + default: + return NotImplemented("Cast from Long to {} is not implemented", + static_cast(target_type_id)); + } +} + +Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) const { + auto float_val = std::get(value_); + + switch (target_type_id) { + case TypeId::kDouble: + return PrimitiveLiteral::Double(static_cast(float_val)); + default: + return NotImplemented("Cast from Float to {} is not implemented", + static_cast(target_type_id)); + } +} + +Result PrimitiveLiteral::CastFromDouble(TypeId target_type_id) const { + auto double_val = std::get(value_); + + switch (target_type_id) { + case TypeId::kFloat: { + if (double_val > std::numeric_limits::max()) { + return PrimitiveLiteral::AboveMaxLiteral(type_); + } + if (double_val < std::numeric_limits::lowest()) { + return PrimitiveLiteral::BelowMinLiteral(type_); + } + return PrimitiveLiteral::Float(static_cast(double_val)); + } + default: + return NotImplemented("Cast from Double to {} is not implemented", + static_cast(target_type_id)); + } } // Three-way comparison operator @@ -90,14 +198,109 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe if (type_->type_id() != other.type_->type_id()) { return std::partial_ordering::unordered; } - if (value_ == other.value_) { - return std::partial_ordering::equivalent; + + // Same type comparison + switch (type_->type_id()) { + case TypeId::kBoolean: { + auto this_val = std::get(value_); + auto other_val = std::get(other.value_); + if (this_val == other_val) return std::partial_ordering::equivalent; + return this_val ? std::partial_ordering::greater : std::partial_ordering::less; + } + + case TypeId::kInt: { + auto this_val = std::get(value_); + auto other_val = std::get(other.value_); + return this_val <=> other_val; + } + + case TypeId::kLong: { + auto this_val = std::get(value_); + auto other_val = std::get(other.value_); + return this_val <=> other_val; + } + + case TypeId::kFloat: { + auto this_val = std::get(value_); + auto other_val = std::get(other.value_); + // Use strong_ordering for floating point as spec requests + return std::strong_order(this_val, other_val); + } + + case TypeId::kDouble: { + auto this_val = std::get(value_); + auto other_val = std::get(other.value_); + // Use strong_ordering for floating point as spec requests + return std::strong_order(this_val, other_val); + } + + case TypeId::kString: { + auto& this_val = std::get(value_); + auto& other_val = std::get(other.value_); + return this_val <=> other_val; + } + + case TypeId::kBinary: { + auto& this_val = std::get>(value_); + auto& other_val = std::get>(other.value_); + return this_val <=> other_val; + } + + default: + // For unsupported types, return unordered + return std::partial_ordering::unordered; } - throw IcebergError("Not implemented: comparison between different primitive types"); } std::string PrimitiveLiteral::ToString() const { - throw NotImplemented("ToString for PrimitiveLiteral is not implemented yet"); + if (std::holds_alternative(value_)) { + return "BelowMin"; + } + if (std::holds_alternative(value_)) { + return "AboveMax"; + } + + switch (type_->type_id()) { + case TypeId::kBoolean: { + return std::get(value_) ? "true" : "false"; + } + case TypeId::kInt: { + return std::to_string(std::get(value_)); + } + case TypeId::kLong: { + return std::to_string(std::get(value_)); + } + case TypeId::kFloat: { + return std::to_string(std::get(value_)); + } + case TypeId::kDouble: { + return std::to_string(std::get(value_)); + } + case TypeId::kString: { + return std::get(value_); + } + case TypeId::kBinary: { + const auto& binary_data = std::get>(value_); + std::string result; + result.reserve(binary_data.size() * 2); // 2 chars per byte + for (const auto& byte : binary_data) { + result += std::format("{:02X}", byte); + } + return result; + } + case TypeId::kDecimal: + case TypeId::kUuid: + case TypeId::kFixed: + case TypeId::kDate: + case TypeId::kTime: + case TypeId::kTimestamp: + case TypeId::kTimestampTz: { + throw IcebergError("Not implemented: ToString for " + type_->ToString()); + } + default: { + throw IcebergError("Unknown type: " + type_->ToString()); + } + } } } // namespace iceberg diff --git a/src/iceberg/datum.h b/src/iceberg/datum.h index c7351b90..5922bfbc 100644 --- a/src/iceberg/datum.h +++ b/src/iceberg/datum.h @@ -30,45 +30,44 @@ namespace iceberg { -/// \brief Exception type for values that are below the minimum allowed value for a -/// primitive type. -/// -/// When casting a value to a narrow primitive type, if the value exceeds the maximum of -/// dest type, it might be above the maximum allowed value for that type. -struct BelowMin { - bool operator==(const BelowMin&) const = default; - std::strong_ordering operator<=>(const BelowMin&) const = default; -}; - -/// \brief Exception type for values that are above the maximum allowed value for a -/// primitive type. -/// -/// When casting a value to a narrow primitive type, if the value exceeds the maximum of -/// dest type, it might be above the maximum allowed value for that type. -struct AboveMax { - bool operator==(const AboveMax&) const = default; - std::strong_ordering operator<=>(const AboveMax&) const = default; -}; - -using PrimitiveLiteralValue = - std::variant, // for binary, fixed, decimal and uuid - BelowMin, AboveMax>; - /// \brief PrimitiveLiteral is owned literal of a primitive type. -class PrimitiveLiteral { - public: - explicit PrimitiveLiteral(PrimitiveLiteralValue value, - std::shared_ptr type); +class ICEBERG_EXPORT PrimitiveLiteral { + private: + /// \brief Exception type for values that are below the minimum allowed value for a + /// primitive type. + /// + /// When casting a value to a narrow primitive type, if the value exceeds the maximum of + /// dest type, it might be above the maximum allowed value for that type. + struct BelowMin { + bool operator==(const BelowMin&) const = default; + std::strong_ordering operator<=>(const BelowMin&) const = default; + }; + + /// \brief Exception type for values that are above the maximum allowed value for a + /// primitive type. + /// + /// When casting a value to a narrow primitive type, if the value exceeds the maximum of + /// dest type, it might be above the maximum allowed value for that type. + struct AboveMax { + bool operator==(const AboveMax&) const = default; + std::strong_ordering operator<=>(const AboveMax&) const = default; + }; + + using PrimitiveLiteralValue = + std::variant, // for binary, fixed + std::array, // for uuid and decimal + BelowMin, AboveMax>; - // Factory methods for primitive types + public: + /// Factory methods for primitive types static PrimitiveLiteral Boolean(bool value); - static PrimitiveLiteral Integer(int32_t value); + static PrimitiveLiteral Int(int32_t value); static PrimitiveLiteral Long(int64_t value); static PrimitiveLiteral Float(float value); static PrimitiveLiteral Double(double value); @@ -86,9 +85,6 @@ class PrimitiveLiteral { /// for reference. Result> Serialize() const; - /// Get the value as a variant - const PrimitiveLiteralValue& value() const; - /// Get the Iceberg Type of the literal const std::shared_ptr& type() const; @@ -100,6 +96,18 @@ class PrimitiveLiteral { std::string ToString() const; + private: + PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type); + + static PrimitiveLiteral BelowMinLiteral(std::shared_ptr type); + static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr type); + + // Helper methods for type casting + Result CastFromInt(TypeId target_type_id) const; + Result CastFromLong(TypeId target_type_id) const; + Result CastFromFloat(TypeId target_type_id) const; + Result CastFromDouble(TypeId target_type_id) const; + private: PrimitiveLiteralValue value_; std::shared_ptr type_; From 1132ef5ab3a150d1426be732734cd5e5d3b99d0c Mon Sep 17 00:00:00 2001 From: mwish Date: Thu, 5 Jun 2025 23:13:26 +0800 Subject: [PATCH 05/19] Fix some logic for using datum --- src/iceberg/datum.cc | 14 +++++++++++++- src/iceberg/datum.h | 23 ++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/iceberg/datum.cc b/src/iceberg/datum.cc index 120dba0e..3df52624 100644 --- a/src/iceberg/datum.cc +++ b/src/iceberg/datum.cc @@ -199,7 +199,12 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe return std::partial_ordering::unordered; } - // Same type comparison + // If either value is AboveMax or BelowMin, comparison is unordered + if (isAboveMax() || isBelowMin() || other.isAboveMax() || other.isBelowMin()) { + return std::partial_ordering::unordered; + } + + // Same type comparison for normal values switch (type_->type_id()) { case TypeId::kBoolean: { auto this_val = std::get(value_); @@ -303,4 +308,11 @@ std::string PrimitiveLiteral::ToString() const { } } +bool PrimitiveLiteral::isBelowMin() const { + return std::holds_alternative(value_); +} + +bool PrimitiveLiteral::isAboveMax() const { + return std::holds_alternative(value_); +} } // namespace iceberg diff --git a/src/iceberg/datum.h b/src/iceberg/datum.h index 5922bfbc..74937c96 100644 --- a/src/iceberg/datum.h +++ b/src/iceberg/datum.h @@ -88,12 +88,33 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// Get the Iceberg Type of the literal const std::shared_ptr& type() const; - /// Cast the literal to a specific type + /// Converts this literal to a literal of the given type. + /// + /// When a predicate is bound to a concrete data column, literals are converted to match + /// the bound column's type. This conversion process is more narrow than a cast and is + /// only intended for cases where substituting one type is a common mistake (e.g. 34 + /// instead of 34L) or where this API avoids requiring a concrete class (e.g., dates). + /// + /// If conversion to a target type is not supported, this method returns an error. + /// + /// This method may return BelowMin or AboveMax when the target type is not as wide as + /// the original type. These values indicate that the containing predicate can be + /// simplified. For example, Integer.MAX_VALUE+1 converted to an int will result in + /// AboveMax and can simplify a < Integer.MAX_VALUE+1 to always true. + /// + /// @param target_type A primitive PrimitiveType + /// @return A Result containing a literal of the given type or an error if conversion + /// was not valid Result CastTo( const std::shared_ptr& target_type) const; + /// Compare two PrimitiveLiterals. Both literals must have the same type + /// and should not be AboveMax or BelowMin. std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; + bool isAboveMax() const; + bool isBelowMin() const; + std::string ToString() const; private: From 1eb9a9ba7201a0014c92265d61022c984bd7eaf8 Mon Sep 17 00:00:00 2001 From: mwish Date: Thu, 5 Jun 2025 23:16:22 +0800 Subject: [PATCH 06/19] Rename files --- src/iceberg/CMakeLists.txt | 2 +- src/iceberg/{datum.cc => literal.cc} | 6 +++--- src/iceberg/{datum.h => literal.h} | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename src/iceberg/{datum.cc => literal.cc} (98%) rename src/iceberg/{datum.h => literal.h} (100%) diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 3ff9d096..73c5a9bc 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -21,10 +21,10 @@ set(ICEBERG_SOURCES arrow_c_data_internal.cc catalog/in_memory_catalog.cc demo.cc - datum.cc expression/expression.cc file_reader.cc json_internal.cc + literal.cc manifest_entry.cc manifest_list.cc metadata_columns.cc diff --git a/src/iceberg/datum.cc b/src/iceberg/literal.cc similarity index 98% rename from src/iceberg/datum.cc rename to src/iceberg/literal.cc index 3df52624..da3d2317 100644 --- a/src/iceberg/datum.cc +++ b/src/iceberg/literal.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/datum.h" +#include "iceberg/literal.h" #include @@ -187,8 +187,8 @@ Result PrimitiveLiteral::CastFromDouble(TypeId target_type_id) return PrimitiveLiteral::Float(static_cast(double_val)); } default: - return NotImplemented("Cast from Double to {} is not implemented", - static_cast(target_type_id)); + return NotSupported("Cast from Double to {} is not implemented", + static_cast(target_type_id)); } } diff --git a/src/iceberg/datum.h b/src/iceberg/literal.h similarity index 100% rename from src/iceberg/datum.h rename to src/iceberg/literal.h From 07dd25772aafc14a2d171af4c6ee5a763672a0cb Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 6 Jun 2025 20:16:52 +0800 Subject: [PATCH 07/19] Resolve some comments, and remove castFromDouble --- src/iceberg/literal.cc | 21 --------------------- src/iceberg/literal.h | 23 ++++++++++++----------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/iceberg/literal.cc b/src/iceberg/literal.cc index da3d2317..ce516c89 100644 --- a/src/iceberg/literal.cc +++ b/src/iceberg/literal.cc @@ -106,11 +106,9 @@ Result PrimitiveLiteral::CastTo( case TypeId::kFloat: return CastFromFloat(target_type_id); case TypeId::kDouble: - return CastFromDouble(target_type_id); case TypeId::kBoolean: case TypeId::kString: case TypeId::kBinary: - // These types only support conversion to string (handled above) break; default: break; @@ -173,25 +171,6 @@ Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) } } -Result PrimitiveLiteral::CastFromDouble(TypeId target_type_id) const { - auto double_val = std::get(value_); - - switch (target_type_id) { - case TypeId::kFloat: { - if (double_val > std::numeric_limits::max()) { - return PrimitiveLiteral::AboveMaxLiteral(type_); - } - if (double_val < std::numeric_limits::lowest()) { - return PrimitiveLiteral::BelowMinLiteral(type_); - } - return PrimitiveLiteral::Float(static_cast(double_val)); - } - default: - return NotSupported("Cast from Double to {} is not implemented", - static_cast(target_type_id)); - } -} - // Three-way comparison operator std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& other) const { // If types are different, comparison is unordered diff --git a/src/iceberg/literal.h b/src/iceberg/literal.h index 74937c96..6397f223 100644 --- a/src/iceberg/literal.h +++ b/src/iceberg/literal.h @@ -30,14 +30,14 @@ namespace iceberg { -/// \brief PrimitiveLiteral is owned literal of a primitive type. +/// \brief PrimitiveLiteral is a literal value that is associated with a primitive type. class ICEBERG_EXPORT PrimitiveLiteral { private: /// \brief Exception type for values that are below the minimum allowed value for a /// primitive type. /// /// When casting a value to a narrow primitive type, if the value exceeds the maximum of - /// dest type, it might be above the maximum allowed value for that type. + /// target type, it might be above the maximum allowed value for that type. struct BelowMin { bool operator==(const BelowMin&) const = default; std::strong_ordering operator<=>(const BelowMin&) const = default; @@ -47,7 +47,7 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// primitive type. /// /// When casting a value to a narrow primitive type, if the value exceeds the maximum of - /// dest type, it might be above the maximum allowed value for that type. + /// target type, it might be above the maximum allowed value for that type. struct AboveMax { bool operator==(const AboveMax&) const = default; std::strong_ordering operator<=>(const AboveMax&) const = default; @@ -74,18 +74,19 @@ class ICEBERG_EXPORT PrimitiveLiteral { static PrimitiveLiteral String(std::string value); static PrimitiveLiteral Binary(std::vector value); - /// Create iceberg value from bytes. + /// Create iceberg literal from bytes. /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. static Result Deserialize(std::span data); - /// Serialize iceberg value to bytes. + + /// Serialize iceberg literal to bytes. /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. Result> Serialize() const; - /// Get the Iceberg Type of the literal + /// Get the Iceberg Type of the literal. const std::shared_ptr& type() const; /// Converts this literal to a literal of the given type. @@ -99,11 +100,12 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// /// This method may return BelowMin or AboveMax when the target type is not as wide as /// the original type. These values indicate that the containing predicate can be - /// simplified. For example, Integer.MAX_VALUE+1 converted to an int will result in - /// AboveMax and can simplify a < Integer.MAX_VALUE+1 to always true. + /// simplified. For example, std::numeric_limits::max()+1 converted to an int will + /// result in AboveMax and can simplify a < std::numeric_limits::max()+1 to always + /// true. /// - /// @param target_type A primitive PrimitiveType - /// @return A Result containing a literal of the given type or an error if conversion + /// \param target_type A primitive PrimitiveType + /// \return A Result containing a literal of the given type or an error if conversion /// was not valid Result CastTo( const std::shared_ptr& target_type) const; @@ -127,7 +129,6 @@ class ICEBERG_EXPORT PrimitiveLiteral { Result CastFromInt(TypeId target_type_id) const; Result CastFromLong(TypeId target_type_id) const; Result CastFromFloat(TypeId target_type_id) const; - Result CastFromDouble(TypeId target_type_id) const; private: PrimitiveLiteralValue value_; From 146a86e420933a6c46c17d620db284d20127daea Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 6 Jun 2025 21:30:25 +0800 Subject: [PATCH 08/19] Add basic tests, and fix float compare bug(qNan != sNan) --- src/iceberg/literal.cc | 34 +++- test/CMakeLists.txt | 5 + test/literal_test.cc | 387 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 422 insertions(+), 4 deletions(-) create mode 100644 test/literal_test.cc diff --git a/src/iceberg/literal.cc b/src/iceberg/literal.cc index ce516c89..a20082c7 100644 --- a/src/iceberg/literal.cc +++ b/src/iceberg/literal.cc @@ -19,6 +19,8 @@ #include "iceberg/literal.h" +#include +#include #include #include "iceberg/exception.h" @@ -60,11 +62,11 @@ PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { } PrimitiveLiteral PrimitiveLiteral::BelowMinLiteral(std::shared_ptr type) { - return PrimitiveLiteral(PrimitiveLiteralValue{BelowMin{}}, std::move(type)); + return {PrimitiveLiteralValue{BelowMin{}}, std::move(type)}; } PrimitiveLiteral PrimitiveLiteral::AboveMaxLiteral(std::shared_ptr type) { - return PrimitiveLiteral(PrimitiveLiteralValue{AboveMax{}}, std::move(type)); + return {PrimitiveLiteralValue{AboveMax{}}, std::move(type)}; } Result PrimitiveLiteral::Deserialize(std::span data) { @@ -171,6 +173,30 @@ Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) } } +// Template function for floating point comparison following Iceberg rules: +// -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign +template +std::partial_ordering iceberg_float_compare(T lhs, T rhs) { + bool lhs_is_nan = std::isnan(lhs); + bool rhs_is_nan = std::isnan(rhs); + + // If both are NaN, check their signs + if (lhs_is_nan && rhs_is_nan) { + bool lhs_is_negative = std::signbit(lhs); + bool rhs_is_negative = std::signbit(rhs); + + if (lhs_is_negative == rhs_is_negative) { + // Same sign NaN values are equivalent (no qNaN vs sNaN distinction) + return std::partial_ordering::equivalent; + } + // -NaN < NaN + return lhs_is_negative ? std::partial_ordering::less : std::partial_ordering::greater; + } + + // For non-NaN values, use standard strong ordering + return std::strong_order(lhs, rhs); +} + // Three-way comparison operator std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& other) const { // If types are different, comparison is unordered @@ -208,14 +234,14 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe auto this_val = std::get(value_); auto other_val = std::get(other.value_); // Use strong_ordering for floating point as spec requests - return std::strong_order(this_val, other_val); + return iceberg_float_compare(this_val, other_val); } case TypeId::kDouble: { auto this_val = std::get(value_); auto other_val = std::get(other.value_); // Use strong_ordering for floating point as spec requests - return std::strong_order(this_val, other_val); + return iceberg_float_compare(this_val, other_val); } case TypeId::kString: { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 863bb09e..6ba48f30 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -69,6 +69,11 @@ target_sources(util_test PRIVATE expected_test.cc formatter_test.cc config_test. target_link_libraries(util_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) add_test(NAME util_test COMMAND util_test) +add_executable(literal_test) +target_sources(literal_test PRIVATE literal_test.cc) +target_link_libraries(literal_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) +add_test(NAME literal_test COMMAND literal_test) + if(ICEBERG_BUILD_BUNDLE) add_executable(avro_test) target_sources(avro_test PRIVATE avro_test.cc avro_schema_test.cc avro_stream_test.cc) diff --git a/test/literal_test.cc b/test/literal_test.cc new file mode 100644 index 00000000..8b6d728f --- /dev/null +++ b/test/literal_test.cc @@ -0,0 +1,387 @@ +/* + * 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/literal.h" + +#include +#include + +#include + +#include "iceberg/type.h" +#include "matchers.h" + +namespace iceberg { + +// Boolean type tests +TEST(PrimitiveLiteralTest, BooleanBasics) { + auto true_literal = PrimitiveLiteral::Boolean(true); + auto false_literal = PrimitiveLiteral::Boolean(false); + + EXPECT_EQ(true_literal.type()->type_id(), TypeId::kBoolean); + EXPECT_EQ(false_literal.type()->type_id(), TypeId::kBoolean); + + EXPECT_EQ(true_literal.ToString(), "true"); + EXPECT_EQ(false_literal.ToString(), "false"); +} + +TEST(PrimitiveLiteralTest, BooleanComparison) { + auto true_literal = PrimitiveLiteral::Boolean(true); + auto false_literal = PrimitiveLiteral::Boolean(false); + auto another_true = PrimitiveLiteral::Boolean(true); + + EXPECT_EQ(true_literal <=> another_true, std::partial_ordering::equivalent); + EXPECT_EQ(true_literal <=> false_literal, std::partial_ordering::greater); + EXPECT_EQ(false_literal <=> true_literal, std::partial_ordering::less); +} + +// Int type tests +TEST(PrimitiveLiteralTest, IntBasics) { + auto int_literal = PrimitiveLiteral::Int(42); + auto negative_int = PrimitiveLiteral::Int(-123); + + EXPECT_EQ(int_literal.type()->type_id(), TypeId::kInt); + EXPECT_EQ(negative_int.type()->type_id(), TypeId::kInt); + + EXPECT_EQ(int_literal.ToString(), "42"); + EXPECT_EQ(negative_int.ToString(), "-123"); +} + +TEST(PrimitiveLiteralTest, IntComparison) { + auto int1 = PrimitiveLiteral::Int(10); + auto int2 = PrimitiveLiteral::Int(20); + auto int3 = PrimitiveLiteral::Int(10); + + EXPECT_EQ(int1 <=> int3, std::partial_ordering::equivalent); + EXPECT_EQ(int1 <=> int2, std::partial_ordering::less); + EXPECT_EQ(int2 <=> int1, std::partial_ordering::greater); +} + +TEST(PrimitiveLiteralTest, IntCastTo) { + auto int_literal = PrimitiveLiteral::Int(42); + + // Cast to Long + auto long_result = int_literal.CastTo(std::make_shared()); + ASSERT_THAT(long_result, IsOk()); + EXPECT_EQ(long_result->type()->type_id(), TypeId::kLong); + EXPECT_EQ(long_result->ToString(), "42"); + + // Cast to Float + auto float_result = int_literal.CastTo(std::make_shared()); + ASSERT_THAT(float_result, IsOk()); + EXPECT_EQ(float_result->type()->type_id(), TypeId::kFloat); + + // Cast to Double + auto double_result = int_literal.CastTo(std::make_shared()); + ASSERT_THAT(double_result, IsOk()); + EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble); +} + +// Long type tests +TEST(PrimitiveLiteralTest, LongBasics) { + auto long_literal = PrimitiveLiteral::Long(1234567890L); + auto negative_long = PrimitiveLiteral::Long(-9876543210L); + + EXPECT_EQ(long_literal.type()->type_id(), TypeId::kLong); + EXPECT_EQ(negative_long.type()->type_id(), TypeId::kLong); + + EXPECT_EQ(long_literal.ToString(), "1234567890"); + EXPECT_EQ(negative_long.ToString(), "-9876543210"); +} + +TEST(PrimitiveLiteralTest, LongComparison) { + auto long1 = PrimitiveLiteral::Long(100L); + auto long2 = PrimitiveLiteral::Long(200L); + auto long3 = PrimitiveLiteral::Long(100L); + + EXPECT_EQ(long1 <=> long3, std::partial_ordering::equivalent); + EXPECT_EQ(long1 <=> long2, std::partial_ordering::less); + EXPECT_EQ(long2 <=> long1, std::partial_ordering::greater); +} + +TEST(PrimitiveLiteralTest, LongCastTo) { + auto long_literal = PrimitiveLiteral::Long(42L); + + // Cast to Int (within range) + auto int_result = long_literal.CastTo(std::make_shared()); + ASSERT_THAT(int_result, IsOk()); + EXPECT_EQ(int_result->type()->type_id(), TypeId::kInt); + EXPECT_EQ(int_result->ToString(), "42"); + + // Cast to Float + auto float_result = long_literal.CastTo(std::make_shared()); + ASSERT_THAT(float_result, IsOk()); + EXPECT_EQ(float_result->type()->type_id(), TypeId::kFloat); + + // Cast to Double + auto double_result = long_literal.CastTo(std::make_shared()); + ASSERT_THAT(double_result, IsOk()); + EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble); +} + +TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { + // Test overflow cases + auto max_long = PrimitiveLiteral::Long( + static_cast(std::numeric_limits::max()) + 1); + auto min_long = PrimitiveLiteral::Long( + static_cast(std::numeric_limits::min()) - 1); + + auto max_result = max_long.CastTo(std::make_shared()); + ASSERT_THAT(max_result, IsOk()); + EXPECT_TRUE(max_result->isAboveMax()); + + auto min_result = min_long.CastTo(std::make_shared()); + ASSERT_THAT(min_result, IsOk()); + EXPECT_TRUE(min_result->isBelowMin()); +} + +// Float type tests +TEST(PrimitiveLiteralTest, FloatBasics) { + auto float_literal = PrimitiveLiteral::Float(3.14f); + auto negative_float = PrimitiveLiteral::Float(-2.71f); + + EXPECT_EQ(float_literal.type()->type_id(), TypeId::kFloat); + EXPECT_EQ(negative_float.type()->type_id(), TypeId::kFloat); + + EXPECT_EQ(float_literal.ToString(), "3.140000"); + EXPECT_EQ(negative_float.ToString(), "-2.710000"); +} + +TEST(PrimitiveLiteralTest, FloatComparison) { + auto float1 = PrimitiveLiteral::Float(1.5f); + auto float2 = PrimitiveLiteral::Float(2.5f); + auto float3 = PrimitiveLiteral::Float(1.5f); + + EXPECT_EQ(float1 <=> float3, std::partial_ordering::equivalent); + EXPECT_EQ(float1 <=> float2, std::partial_ordering::less); + EXPECT_EQ(float2 <=> float1, std::partial_ordering::greater); +} + +TEST(PrimitiveLiteralTest, FloatCastTo) { + auto float_literal = PrimitiveLiteral::Float(3.14f); + + // Cast to Double + auto double_result = float_literal.CastTo(std::make_shared()); + ASSERT_THAT(double_result, IsOk()); + EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble); +} + +// Double type tests +TEST(PrimitiveLiteralTest, DoubleBasics) { + auto double_literal = PrimitiveLiteral::Double(3.141592653589793); + auto negative_double = PrimitiveLiteral::Double(-2.718281828459045); + + EXPECT_EQ(double_literal.type()->type_id(), TypeId::kDouble); + EXPECT_EQ(negative_double.type()->type_id(), TypeId::kDouble); + + EXPECT_EQ(double_literal.ToString(), "3.141593"); + EXPECT_EQ(negative_double.ToString(), "-2.718282"); +} + +TEST(PrimitiveLiteralTest, DoubleComparison) { + auto double1 = PrimitiveLiteral::Double(1.5); + auto double2 = PrimitiveLiteral::Double(2.5); + auto double3 = PrimitiveLiteral::Double(1.5); + + EXPECT_EQ(double1 <=> double3, std::partial_ordering::equivalent); + EXPECT_EQ(double1 <=> double2, std::partial_ordering::less); + EXPECT_EQ(double2 <=> double1, std::partial_ordering::greater); +} + +// String type tests +TEST(PrimitiveLiteralTest, StringBasics) { + auto string_literal = PrimitiveLiteral::String("hello world"); + auto empty_string = PrimitiveLiteral::String(""); + + EXPECT_EQ(string_literal.type()->type_id(), TypeId::kString); + EXPECT_EQ(empty_string.type()->type_id(), TypeId::kString); + + EXPECT_EQ(string_literal.ToString(), "hello world"); + EXPECT_EQ(empty_string.ToString(), ""); +} + +TEST(PrimitiveLiteralTest, StringComparison) { + auto string1 = PrimitiveLiteral::String("apple"); + auto string2 = PrimitiveLiteral::String("banana"); + auto string3 = PrimitiveLiteral::String("apple"); + + EXPECT_EQ(string1 <=> string3, std::partial_ordering::equivalent); + EXPECT_EQ(string1 <=> string2, std::partial_ordering::less); + EXPECT_EQ(string2 <=> string1, std::partial_ordering::greater); +} + +// Binary type tests +TEST(PrimitiveLiteralTest, BinaryBasics) { + std::vector data = {0x01, 0x02, 0x03, 0xFF}; + auto binary_literal = PrimitiveLiteral::Binary(data); + auto empty_binary = PrimitiveLiteral::Binary({}); + + EXPECT_EQ(binary_literal.type()->type_id(), TypeId::kBinary); + EXPECT_EQ(empty_binary.type()->type_id(), TypeId::kBinary); + + EXPECT_EQ(binary_literal.ToString(), "010203FF"); + EXPECT_EQ(empty_binary.ToString(), ""); +} + +TEST(PrimitiveLiteralTest, BinaryComparison) { + std::vector data1 = {0x01, 0x02}; + std::vector data2 = {0x01, 0x03}; + std::vector data3 = {0x01, 0x02}; + + auto binary1 = PrimitiveLiteral::Binary(data1); + auto binary2 = PrimitiveLiteral::Binary(data2); + auto binary3 = PrimitiveLiteral::Binary(data3); + + EXPECT_EQ(binary1 <=> binary3, std::partial_ordering::equivalent); + EXPECT_EQ(binary1 <=> binary2, std::partial_ordering::less); + EXPECT_EQ(binary2 <=> binary1, std::partial_ordering::greater); +} + +// Cross-type comparison tests +TEST(PrimitiveLiteralTest, CrossTypeComparison) { + auto int_literal = PrimitiveLiteral::Int(42); + auto string_literal = PrimitiveLiteral::String("42"); + + // Different types should return unordered + EXPECT_EQ(int_literal <=> string_literal, std::partial_ordering::unordered); +} + +// Special value tests +TEST(PrimitiveLiteralTest, SpecialValues) { + auto int_literal = PrimitiveLiteral::Int(42); + + EXPECT_FALSE(int_literal.isAboveMax()); + EXPECT_FALSE(int_literal.isBelowMin()); +} + +// Same type cast test +TEST(PrimitiveLiteralTest, SameTypeCast) { + auto int_literal = PrimitiveLiteral::Int(42); + + auto same_type_result = int_literal.CastTo(std::make_shared()); + ASSERT_THAT(same_type_result, IsOk()); + EXPECT_EQ(same_type_result->type()->type_id(), TypeId::kInt); + EXPECT_EQ(same_type_result->ToString(), "42"); +} + +// Float special values tests +TEST(PrimitiveLiteralTest, FloatSpecialValuesComparison) { + // Create special float values + auto neg_nan = PrimitiveLiteral::Float(-std::numeric_limits::quiet_NaN()); + auto neg_inf = PrimitiveLiteral::Float(-std::numeric_limits::infinity()); + auto neg_value = PrimitiveLiteral::Float(-1.5f); + auto neg_zero = PrimitiveLiteral::Float(-0.0f); + auto pos_zero = PrimitiveLiteral::Float(0.0f); + auto pos_value = PrimitiveLiteral::Float(1.5f); + auto pos_inf = PrimitiveLiteral::Float(std::numeric_limits::infinity()); + auto pos_nan = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); + + // Test the ordering: -NaN < -Infinity < -value < -0 < 0 < value < Infinity < NaN + EXPECT_EQ(neg_nan <=> neg_inf, std::partial_ordering::less); + EXPECT_EQ(neg_inf <=> neg_value, std::partial_ordering::less); + EXPECT_EQ(neg_value <=> neg_zero, std::partial_ordering::less); + EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); + EXPECT_EQ(pos_zero <=> pos_value, std::partial_ordering::less); + EXPECT_EQ(pos_value <=> pos_inf, std::partial_ordering::less); + EXPECT_EQ(pos_inf <=> pos_nan, std::partial_ordering::less); +} + +TEST(PrimitiveLiteralTest, FloatNaNComparison) { + auto nan1 = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); + auto nan2 = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); + auto signaling_nan = + PrimitiveLiteral::Float(std::numeric_limits::signaling_NaN()); + + // NaN should be equal to itself in strong ordering + EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); + EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); +} + +TEST(PrimitiveLiteralTest, FloatInfinityComparison) { + auto neg_inf = PrimitiveLiteral::Float(-std::numeric_limits::infinity()); + auto pos_inf = PrimitiveLiteral::Float(std::numeric_limits::infinity()); + auto max_value = PrimitiveLiteral::Float(std::numeric_limits::max()); + auto min_value = PrimitiveLiteral::Float(std::numeric_limits::lowest()); + + EXPECT_EQ(neg_inf <=> min_value, std::partial_ordering::less); + EXPECT_EQ(max_value <=> pos_inf, std::partial_ordering::less); + EXPECT_EQ(neg_inf <=> pos_inf, std::partial_ordering::less); +} + +TEST(PrimitiveLiteralTest, FloatZeroComparison) { + auto neg_zero = PrimitiveLiteral::Float(-0.0f); + auto pos_zero = PrimitiveLiteral::Float(0.0f); + + // -0 should be less than +0 + EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); +} + +// Double special values tests +TEST(PrimitiveLiteralTest, DoubleSpecialValuesComparison) { + // Create special double values + auto neg_nan = PrimitiveLiteral::Double(-std::numeric_limits::quiet_NaN()); + auto neg_inf = PrimitiveLiteral::Double(-std::numeric_limits::infinity()); + auto neg_value = PrimitiveLiteral::Double(-1.5); + auto neg_zero = PrimitiveLiteral::Double(-0.0); + auto pos_zero = PrimitiveLiteral::Double(0.0); + auto pos_value = PrimitiveLiteral::Double(1.5); + auto pos_inf = PrimitiveLiteral::Double(std::numeric_limits::infinity()); + auto pos_nan = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); + + // Test the ordering: -NaN < -Infinity < -value < -0 < 0 < value < Infinity < NaN + EXPECT_EQ(neg_nan <=> neg_inf, std::partial_ordering::less); + EXPECT_EQ(neg_inf <=> neg_value, std::partial_ordering::less); + EXPECT_EQ(neg_value <=> neg_zero, std::partial_ordering::less); + EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); + EXPECT_EQ(pos_zero <=> pos_value, std::partial_ordering::less); + EXPECT_EQ(pos_value <=> pos_inf, std::partial_ordering::less); + EXPECT_EQ(pos_inf <=> pos_nan, std::partial_ordering::less); +} + +TEST(PrimitiveLiteralTest, DoubleNaNComparison) { + auto nan1 = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); + auto nan2 = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); + auto signaling_nan = + PrimitiveLiteral::Double(std::numeric_limits::signaling_NaN()); + + // NaN should be equal to itself in strong ordering + EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); + EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); +} + +TEST(PrimitiveLiteralTest, DoubleInfinityComparison) { + auto neg_inf = PrimitiveLiteral::Double(-std::numeric_limits::infinity()); + auto pos_inf = PrimitiveLiteral::Double(std::numeric_limits::infinity()); + auto max_value = PrimitiveLiteral::Double(std::numeric_limits::max()); + auto min_value = PrimitiveLiteral::Double(std::numeric_limits::lowest()); + + EXPECT_EQ(neg_inf <=> min_value, std::partial_ordering::less); + EXPECT_EQ(max_value <=> pos_inf, std::partial_ordering::less); + EXPECT_EQ(neg_inf <=> pos_inf, std::partial_ordering::less); +} + +TEST(PrimitiveLiteralTest, DoubleZeroComparison) { + auto neg_zero = PrimitiveLiteral::Double(-0.0); + auto pos_zero = PrimitiveLiteral::Double(0.0); + + // -0 should be less than +0 + EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); +} + +} // namespace iceberg From d125408ebee526aff352c4d233ef7749199235c5 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 6 Jun 2025 23:25:41 +0800 Subject: [PATCH 09/19] Fix lint --- test/literal_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/literal_test.cc b/test/literal_test.cc index 8b6d728f..336e6dba 100644 --- a/test/literal_test.cc +++ b/test/literal_test.cc @@ -20,6 +20,7 @@ #include "iceberg/literal.h" #include +#include #include #include @@ -184,8 +185,8 @@ TEST(PrimitiveLiteralTest, FloatCastTo) { // Double type tests TEST(PrimitiveLiteralTest, DoubleBasics) { - auto double_literal = PrimitiveLiteral::Double(3.141592653589793); - auto negative_double = PrimitiveLiteral::Double(-2.718281828459045); + auto double_literal = PrimitiveLiteral::Double(std::numbers::pi); + auto negative_double = PrimitiveLiteral::Double(-std::numbers::e); EXPECT_EQ(double_literal.type()->type_id(), TypeId::kDouble); EXPECT_EQ(negative_double.type()->type_id(), TypeId::kDouble); From 012966f901cb4adf3dc0361c27cb86d00a6219f0 Mon Sep 17 00:00:00 2001 From: mwish Date: Tue, 10 Jun 2025 11:09:22 +0800 Subject: [PATCH 10/19] some NotImplement -> NotSupported --- src/iceberg/literal.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/iceberg/literal.cc b/src/iceberg/literal.cc index a20082c7..acf1c8e2 100644 --- a/src/iceberg/literal.cc +++ b/src/iceberg/literal.cc @@ -130,7 +130,6 @@ Result PrimitiveLiteral::CastFromInt(TypeId target_type_id) co return PrimitiveLiteral::Float(static_cast(int_val)); case TypeId::kDouble: return PrimitiveLiteral::Double(static_cast(int_val)); - // TODO(mwish): Supports casts to date and literal default: return NotSupported("Cast from Int to {} is not implemented", static_cast(target_type_id)); @@ -156,8 +155,8 @@ Result PrimitiveLiteral::CastFromLong(TypeId target_type_id) c case TypeId::kDouble: return PrimitiveLiteral::Double(static_cast(long_val)); default: - return NotImplemented("Cast from Long to {} is not implemented", - static_cast(target_type_id)); + return NotSupported("Cast from Long to {} is not supported", + static_cast(target_type_id)); } } @@ -168,8 +167,8 @@ Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) case TypeId::kDouble: return PrimitiveLiteral::Double(static_cast(float_val)); default: - return NotImplemented("Cast from Float to {} is not implemented", - static_cast(target_type_id)); + return NotSupported("Cast from Float to {} is not supported", + static_cast(target_type_id)); } } From 88f67dc66d9fe806abf8246f514dd7b2a90233b8 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 16:03:33 +0800 Subject: [PATCH 11/19] Move literal to expression dir --- src/iceberg/CMakeLists.txt | 2 +- src/iceberg/{ => expression}/literal.cc | 3 ++- src/iceberg/{ => expression}/literal.h | 0 test/literal_test.cc | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) rename src/iceberg/{ => expression}/literal.cc (99%) rename src/iceberg/{ => expression}/literal.h (100%) diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 73c5a9bc..851ddcaf 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -22,9 +22,9 @@ set(ICEBERG_SOURCES catalog/in_memory_catalog.cc demo.cc expression/expression.cc + expression/literal.cc file_reader.cc json_internal.cc - literal.cc manifest_entry.cc manifest_list.cc metadata_columns.cc diff --git a/src/iceberg/literal.cc b/src/iceberg/expression/literal.cc similarity index 99% rename from src/iceberg/literal.cc rename to src/iceberg/expression/literal.cc index a20082c7..fe9accb6 100644 --- a/src/iceberg/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/literal.h" +#include "iceberg/expression/literal.h" #include #include @@ -320,4 +320,5 @@ bool PrimitiveLiteral::isBelowMin() const { bool PrimitiveLiteral::isAboveMax() const { return std::holds_alternative(value_); } + } // namespace iceberg diff --git a/src/iceberg/literal.h b/src/iceberg/expression/literal.h similarity index 100% rename from src/iceberg/literal.h rename to src/iceberg/expression/literal.h diff --git a/test/literal_test.cc b/test/literal_test.cc index 336e6dba..6245fedf 100644 --- a/test/literal_test.cc +++ b/test/literal_test.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/literal.h" +#include "iceberg/expression/literal.h" #include #include From 694cb554406f9007cf586113707acf5b702d8c86 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 16:40:42 +0800 Subject: [PATCH 12/19] Apply suggestions and create PrimitiveLiteralCaster --- src/iceberg/expression/literal.cc | 240 ++++++++++++++++++------------ src/iceberg/expression/literal.h | 38 ++--- test/literal_test.cc | 8 +- 3 files changed, 161 insertions(+), 125 deletions(-) diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index fe9accb6..6cfc6b92 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -27,102 +27,49 @@ namespace iceberg { -// Constructor -PrimitiveLiteral::PrimitiveLiteral(PrimitiveLiteralValue value, - std::shared_ptr type) - : value_(std::move(value)), type_(std::move(type)) {} - -// Factory methods -PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) { - return {PrimitiveLiteralValue{value}, std::make_shared()}; -} - -PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) { - return {PrimitiveLiteralValue{value}, std::make_shared()}; -} - -PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) { - return {PrimitiveLiteralValue{value}, std::make_shared()}; -} - -PrimitiveLiteral PrimitiveLiteral::Float(float value) { - return {PrimitiveLiteralValue{value}, std::make_shared()}; -} - -PrimitiveLiteral PrimitiveLiteral::Double(double value) { - return {PrimitiveLiteralValue{value}, std::make_shared()}; +/// \brief PrimitiveLiteralCaster handles type casting operations for PrimitiveLiteral. +/// This is an internal implementation class. +class PrimitiveLiteralCaster { + public: + /// Cast a PrimitiveLiteral to the target type. + static Result CastTo( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + + /// Create a literal representing a value below the minimum for the given type. + static PrimitiveLiteral BelowMinLiteral(std::shared_ptr type); + + /// Create a literal representing a value above the maximum for the given type. + static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr type); + + private: + /// Cast from Int type to target type. + static Result CastFromInt( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + + /// Cast from Long type to target type. + static Result CastFromLong( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + + /// Cast from Float type to target type. + static Result CastFromFloat( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type); +}; + +PrimitiveLiteral PrimitiveLiteralCaster::BelowMinLiteral( + std::shared_ptr type) { + return PrimitiveLiteral(PrimitiveLiteral::BelowMin{}, std::move(type)); } -PrimitiveLiteral PrimitiveLiteral::String(std::string value) { - return {PrimitiveLiteralValue{std::move(value)}, std::make_shared()}; +PrimitiveLiteral PrimitiveLiteralCaster::AboveMaxLiteral( + std::shared_ptr type) { + return PrimitiveLiteral(PrimitiveLiteral::AboveMax{}, std::move(type)); } -PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { - return {PrimitiveLiteralValue{std::move(value)}, std::make_shared()}; -} - -PrimitiveLiteral PrimitiveLiteral::BelowMinLiteral(std::shared_ptr type) { - return {PrimitiveLiteralValue{BelowMin{}}, std::move(type)}; -} - -PrimitiveLiteral PrimitiveLiteral::AboveMaxLiteral(std::shared_ptr type) { - return {PrimitiveLiteralValue{AboveMax{}}, std::move(type)}; -} - -Result PrimitiveLiteral::Deserialize(std::span data) { - return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet"); -} - -Result> PrimitiveLiteral::Serialize() const { - return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet"); -} - -// Getters - -const std::shared_ptr& PrimitiveLiteral::type() const { return type_; } - -// Cast method -Result PrimitiveLiteral::CastTo( - const std::shared_ptr& target_type) const { - if (*type_ == *target_type) { - // If types are the same, return a copy of the current literal - return PrimitiveLiteral(value_, target_type); - } - - // Handle special values - if (std::holds_alternative(value_) || - std::holds_alternative(value_)) { - // Cannot cast type for special values - return NotSupported("Cannot cast type for {}", ToString()); - } - - auto source_type_id = type_->type_id(); +Result PrimitiveLiteralCaster::CastFromInt( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { + auto int_val = std::get(literal.value_); auto target_type_id = target_type->type_id(); - // Delegate to specific cast functions based on source type - switch (source_type_id) { - case TypeId::kInt: - return CastFromInt(target_type_id); - case TypeId::kLong: - return CastFromLong(target_type_id); - case TypeId::kFloat: - return CastFromFloat(target_type_id); - case TypeId::kDouble: - case TypeId::kBoolean: - case TypeId::kString: - case TypeId::kBinary: - break; - default: - break; - } - - return NotSupported("Cast from {} to {} is not implemented", type_->ToString(), - target_type->ToString()); -} - -Result PrimitiveLiteral::CastFromInt(TypeId target_type_id) const { - auto int_val = std::get(value_); - switch (target_type_id) { case TypeId::kLong: return PrimitiveLiteral::Long(static_cast(int_val)); @@ -137,17 +84,19 @@ Result PrimitiveLiteral::CastFromInt(TypeId target_type_id) co } } -Result PrimitiveLiteral::CastFromLong(TypeId target_type_id) const { - auto long_val = std::get(value_); +Result PrimitiveLiteralCaster::CastFromLong( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { + auto long_val = std::get(literal.value_); + auto target_type_id = target_type->type_id(); switch (target_type_id) { case TypeId::kInt: { // Check for overflow if (long_val >= std::numeric_limits::max()) { - return PrimitiveLiteral::AboveMaxLiteral(type_); + return AboveMaxLiteral(target_type); } if (long_val <= std::numeric_limits::min()) { - return PrimitiveLiteral::BelowMinLiteral(type_); + return BelowMinLiteral(target_type); } return PrimitiveLiteral::Int(static_cast(long_val)); } @@ -161,8 +110,10 @@ Result PrimitiveLiteral::CastFromLong(TypeId target_type_id) c } } -Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) const { - auto float_val = std::get(value_); +Result PrimitiveLiteralCaster::CastFromFloat( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { + auto float_val = std::get(literal.value_); + auto target_type_id = target_type->type_id(); switch (target_type_id) { case TypeId::kDouble: @@ -173,6 +124,58 @@ Result PrimitiveLiteral::CastFromFloat(TypeId target_type_id) } } +// Constructor +PrimitiveLiteral::PrimitiveLiteral(Value value, std::shared_ptr type) + : value_(std::move(value)), type_(std::move(type)) {} + +// Factory methods +PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) { + return {Value{value}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) { + return {Value{value}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) { + return {Value{value}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::Float(float value) { + return {Value{value}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::Double(double value) { + return {Value{value}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::String(std::string value) { + return {Value{std::move(value)}, std::make_shared()}; +} + +PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { + return {Value{std::move(value)}, std::make_shared()}; +} + +Result PrimitiveLiteral::Deserialize( + std::span data, std::shared_ptr type) { + return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet"); +} + +Result> PrimitiveLiteral::Serialize() const { + return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet"); +} + +// Getters + +const std::shared_ptr& PrimitiveLiteral::type() const { return type_; } + +// Cast method +Result PrimitiveLiteral::CastTo( + const std::shared_ptr& target_type) const { + return PrimitiveLiteralCaster::CastTo(*this, target_type); +} + // Template function for floating point comparison following Iceberg rules: // -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign template @@ -205,7 +208,7 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe } // If either value is AboveMax or BelowMin, comparison is unordered - if (isAboveMax() || isBelowMin() || other.isAboveMax() || other.isBelowMin()) { + if (IsAboveMax() || IsBelowMin() || other.IsAboveMax() || other.IsBelowMin()) { return std::partial_ordering::unordered; } @@ -313,12 +316,51 @@ std::string PrimitiveLiteral::ToString() const { } } -bool PrimitiveLiteral::isBelowMin() const { +bool PrimitiveLiteral::IsBelowMin() const { return std::holds_alternative(value_); } -bool PrimitiveLiteral::isAboveMax() const { +bool PrimitiveLiteral::IsAboveMax() const { return std::holds_alternative(value_); } +// PrimitiveLiteralCaster implementation + +Result PrimitiveLiteralCaster::CastTo( + const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { + if (*literal.type_ == *target_type) { + // If types are the same, return a copy of the current literal + return PrimitiveLiteral(literal.value_, target_type); + } + + // Handle special values + if (std::holds_alternative(literal.value_) || + std::holds_alternative(literal.value_)) { + // Cannot cast type for special values + return NotSupported("Cannot cast type for {}", literal.ToString()); + } + + auto source_type_id = literal.type_->type_id(); + + // Delegate to specific cast functions based on source type + switch (source_type_id) { + case TypeId::kInt: + return CastFromInt(literal, target_type); + case TypeId::kLong: + return CastFromLong(literal, target_type); + case TypeId::kFloat: + return CastFromFloat(literal, target_type); + case TypeId::kDouble: + case TypeId::kBoolean: + case TypeId::kString: + case TypeId::kBinary: + break; + default: + break; + } + + return NotSupported("Cast from {} to {} is not implemented", literal.type_->ToString(), + target_type->ToString()); +} + } // namespace iceberg diff --git a/src/iceberg/expression/literal.h b/src/iceberg/expression/literal.h index 6397f223..3ba6d8a5 100644 --- a/src/iceberg/expression/literal.h +++ b/src/iceberg/expression/literal.h @@ -53,16 +53,15 @@ class ICEBERG_EXPORT PrimitiveLiteral { std::strong_ordering operator<=>(const AboveMax&) const = default; }; - using PrimitiveLiteralValue = - std::variant, // for binary, fixed - std::array, // for uuid and decimal - BelowMin, AboveMax>; + using Value = std::variant, // for binary, fixed + std::array, // for uuid and decimal + BelowMin, AboveMax>; public: /// Factory methods for primitive types @@ -78,7 +77,8 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. - static Result Deserialize(std::span data); + static Result Deserialize(std::span data, + std::shared_ptr type); /// Serialize iceberg literal to bytes. /// @@ -114,24 +114,18 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// and should not be AboveMax or BelowMin. std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; - bool isAboveMax() const; - bool isBelowMin() const; + bool IsAboveMax() const; + bool IsBelowMin() const; std::string ToString() const; private: - PrimitiveLiteral(PrimitiveLiteralValue value, std::shared_ptr type); + PrimitiveLiteral(Value value, std::shared_ptr type); - static PrimitiveLiteral BelowMinLiteral(std::shared_ptr type); - static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr type); - - // Helper methods for type casting - Result CastFromInt(TypeId target_type_id) const; - Result CastFromLong(TypeId target_type_id) const; - Result CastFromFloat(TypeId target_type_id) const; + friend class PrimitiveLiteralCaster; private: - PrimitiveLiteralValue value_; + Value value_; std::shared_ptr type_; }; diff --git a/test/literal_test.cc b/test/literal_test.cc index 6245fedf..60018770 100644 --- a/test/literal_test.cc +++ b/test/literal_test.cc @@ -145,11 +145,11 @@ TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { auto max_result = max_long.CastTo(std::make_shared()); ASSERT_THAT(max_result, IsOk()); - EXPECT_TRUE(max_result->isAboveMax()); + EXPECT_TRUE(max_result->IsAboveMax()); auto min_result = min_long.CastTo(std::make_shared()); ASSERT_THAT(min_result, IsOk()); - EXPECT_TRUE(min_result->isBelowMin()); + EXPECT_TRUE(min_result->IsBelowMin()); } // Float type tests @@ -267,8 +267,8 @@ TEST(PrimitiveLiteralTest, CrossTypeComparison) { TEST(PrimitiveLiteralTest, SpecialValues) { auto int_literal = PrimitiveLiteral::Int(42); - EXPECT_FALSE(int_literal.isAboveMax()); - EXPECT_FALSE(int_literal.isBelowMin()); + EXPECT_FALSE(int_literal.IsAboveMax()); + EXPECT_FALSE(int_literal.IsBelowMin()); } // Same type cast test From 1d7f90488f459432d6f3447ad1fbf2acffdd683e Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 18:09:29 +0800 Subject: [PATCH 13/19] rename variables --- src/iceberg/expression/literal.cc | 119 +++++++++++----------- src/iceberg/expression/literal.h | 31 +++--- test/literal_test.cc | 162 +++++++++++++++--------------- 3 files changed, 151 insertions(+), 161 deletions(-) diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index 17f1bb39..fd48b126 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -27,64 +27,62 @@ namespace iceberg { -/// \brief PrimitiveLiteralCaster handles type casting operations for PrimitiveLiteral. +/// \brief LiteralCaster handles type casting operations for Literal. /// This is an internal implementation class. -class PrimitiveLiteralCaster { +class LiteralCaster { public: - /// Cast a PrimitiveLiteral to the target type. - static Result CastTo( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + /// Cast a Literal to the target type. + static Result CastTo(const Literal& literal, + const std::shared_ptr& target_type); /// Create a literal representing a value below the minimum for the given type. - static PrimitiveLiteral BelowMinLiteral(std::shared_ptr type); + static Literal BelowMinLiteral(std::shared_ptr type); /// Create a literal representing a value above the maximum for the given type. - static PrimitiveLiteral AboveMaxLiteral(std::shared_ptr type); + static Literal AboveMaxLiteral(std::shared_ptr type); private: /// Cast from Int type to target type. - static Result CastFromInt( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + static Result CastFromInt(const Literal& literal, + const std::shared_ptr& target_type); /// Cast from Long type to target type. - static Result CastFromLong( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + static Result CastFromLong(const Literal& literal, + const std::shared_ptr& target_type); /// Cast from Float type to target type. - static Result CastFromFloat( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type); + static Result CastFromFloat(const Literal& literal, + const std::shared_ptr& target_type); }; -PrimitiveLiteral PrimitiveLiteralCaster::BelowMinLiteral( - std::shared_ptr type) { - return PrimitiveLiteral(PrimitiveLiteral::BelowMin{}, std::move(type)); +Literal LiteralCaster::BelowMinLiteral(std::shared_ptr type) { + return Literal(Literal::BelowMin{}, std::move(type)); } -PrimitiveLiteral PrimitiveLiteralCaster::AboveMaxLiteral( - std::shared_ptr type) { - return PrimitiveLiteral(PrimitiveLiteral::AboveMax{}, std::move(type)); +Literal LiteralCaster::AboveMaxLiteral(std::shared_ptr type) { + return Literal(Literal::AboveMax{}, std::move(type)); } -Result PrimitiveLiteralCaster::CastFromInt( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { +Result LiteralCaster::CastFromInt( + const Literal& literal, const std::shared_ptr& target_type) { auto int_val = std::get(literal.value_); auto target_type_id = target_type->type_id(); switch (target_type_id) { case TypeId::kLong: - return PrimitiveLiteral::Long(static_cast(int_val)); + return Literal::Long(static_cast(int_val)); case TypeId::kFloat: - return PrimitiveLiteral::Float(static_cast(int_val)); + return Literal::Float(static_cast(int_val)); case TypeId::kDouble: - return PrimitiveLiteral::Double(static_cast(int_val)); + return Literal::Double(static_cast(int_val)); default: return NotSupported("Cast from Int to {} is not implemented", static_cast(target_type_id)); } } -Result PrimitiveLiteralCaster::CastFromLong( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { +Result LiteralCaster::CastFromLong( + const Literal& literal, const std::shared_ptr& target_type) { auto long_val = std::get(literal.value_); auto target_type_id = target_type->type_id(); @@ -97,26 +95,26 @@ Result PrimitiveLiteralCaster::CastFromLong( if (long_val <= std::numeric_limits::min()) { return BelowMinLiteral(target_type); } - return PrimitiveLiteral::Int(static_cast(long_val)); + return Literal::Int(static_cast(long_val)); } case TypeId::kFloat: - return PrimitiveLiteral::Float(static_cast(long_val)); + return Literal::Float(static_cast(long_val)); case TypeId::kDouble: - return PrimitiveLiteral::Double(static_cast(long_val)); + return Literal::Double(static_cast(long_val)); default: return NotSupported("Cast from Long to {} is not supported", static_cast(target_type_id)); } } -Result PrimitiveLiteralCaster::CastFromFloat( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { +Result LiteralCaster::CastFromFloat( + const Literal& literal, const std::shared_ptr& target_type) { auto float_val = std::get(literal.value_); auto target_type_id = target_type->type_id(); switch (target_type_id) { case TypeId::kDouble: - return PrimitiveLiteral::Double(static_cast(float_val)); + return Literal::Double(static_cast(float_val)); default: return NotSupported("Cast from Float to {} is not supported", static_cast(target_type_id)); @@ -124,55 +122,54 @@ Result PrimitiveLiteralCaster::CastFromFloat( } // Constructor -PrimitiveLiteral::PrimitiveLiteral(Value value, std::shared_ptr type) +Literal::Literal(Value value, std::shared_ptr type) : value_(std::move(value)), type_(std::move(type)) {} // Factory methods -PrimitiveLiteral PrimitiveLiteral::Boolean(bool value) { +Literal Literal::Boolean(bool value) { return {Value{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Int(int32_t value) { +Literal Literal::Int(int32_t value) { return {Value{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Long(int64_t value) { +Literal Literal::Long(int64_t value) { return {Value{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Float(float value) { +Literal Literal::Float(float value) { return {Value{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Double(double value) { +Literal Literal::Double(double value) { return {Value{value}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::String(std::string value) { +Literal Literal::String(std::string value) { return {Value{std::move(value)}, std::make_shared()}; } -PrimitiveLiteral PrimitiveLiteral::Binary(std::vector value) { +Literal Literal::Binary(std::vector value) { return {Value{std::move(value)}, std::make_shared()}; } -Result PrimitiveLiteral::Deserialize( - std::span data, std::shared_ptr type) { - return NotImplemented("Deserialization of PrimitiveLiteral is not implemented yet"); +Result Literal::Deserialize(std::span data, + std::shared_ptr type) { + return NotImplemented("Deserialization of Literal is not implemented yet"); } -Result> PrimitiveLiteral::Serialize() const { - return NotImplemented("Serialization of PrimitiveLiteral is not implemented yet"); +Result> Literal::Serialize() const { + return NotImplemented("Serialization of Literal is not implemented yet"); } // Getters -const std::shared_ptr& PrimitiveLiteral::type() const { return type_; } +const std::shared_ptr& Literal::type() const { return type_; } // Cast method -Result PrimitiveLiteral::CastTo( - const std::shared_ptr& target_type) const { - return PrimitiveLiteralCaster::CastTo(*this, target_type); +Result Literal::CastTo(const std::shared_ptr& target_type) const { + return LiteralCaster::CastTo(*this, target_type); } // Template function for floating point comparison following Iceberg rules: @@ -200,7 +197,7 @@ std::partial_ordering iceberg_float_compare(T lhs, T rhs) { } // Three-way comparison operator -std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& other) const { +std::partial_ordering Literal::operator<=>(const Literal& other) const { // If types are different, comparison is unordered if (type_->type_id() != other.type_->type_id()) { return std::partial_ordering::unordered; @@ -264,7 +261,7 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe } } -std::string PrimitiveLiteral::ToString() const { +std::string Literal::ToString() const { if (std::holds_alternative(value_)) { return "BelowMin"; } @@ -315,26 +312,22 @@ std::string PrimitiveLiteral::ToString() const { } } -bool PrimitiveLiteral::IsBelowMin() const { - return std::holds_alternative(value_); -} +bool Literal::IsBelowMin() const { return std::holds_alternative(value_); } -bool PrimitiveLiteral::IsAboveMax() const { - return std::holds_alternative(value_); -} +bool Literal::IsAboveMax() const { return std::holds_alternative(value_); } -// PrimitiveLiteralCaster implementation +// LiteralCaster implementation -Result PrimitiveLiteralCaster::CastTo( - const PrimitiveLiteral& literal, const std::shared_ptr& target_type) { +Result LiteralCaster::CastTo(const Literal& literal, + const std::shared_ptr& target_type) { if (*literal.type_ == *target_type) { // If types are the same, return a copy of the current literal - return PrimitiveLiteral(literal.value_, target_type); + return Literal(literal.value_, target_type); } // Handle special values - if (std::holds_alternative(literal.value_) || - std::holds_alternative(literal.value_)) { + if (std::holds_alternative(literal.value_) || + std::holds_alternative(literal.value_)) { // Cannot cast type for special values return NotSupported("Cannot cast type for {}", literal.ToString()); } diff --git a/src/iceberg/expression/literal.h b/src/iceberg/expression/literal.h index 3ba6d8a5..fa1cce5f 100644 --- a/src/iceberg/expression/literal.h +++ b/src/iceberg/expression/literal.h @@ -30,8 +30,8 @@ namespace iceberg { -/// \brief PrimitiveLiteral is a literal value that is associated with a primitive type. -class ICEBERG_EXPORT PrimitiveLiteral { +/// \brief Literal is a literal value that is associated with a primitive type. +class ICEBERG_EXPORT Literal { private: /// \brief Exception type for values that are below the minimum allowed value for a /// primitive type. @@ -65,20 +65,20 @@ class ICEBERG_EXPORT PrimitiveLiteral { public: /// Factory methods for primitive types - static PrimitiveLiteral Boolean(bool value); - static PrimitiveLiteral Int(int32_t value); - static PrimitiveLiteral Long(int64_t value); - static PrimitiveLiteral Float(float value); - static PrimitiveLiteral Double(double value); - static PrimitiveLiteral String(std::string value); - static PrimitiveLiteral Binary(std::vector value); + static Literal Boolean(bool value); + static Literal Int(int32_t value); + static Literal Long(int64_t value); + static Literal Float(float value); + static Literal Double(double value); + static Literal String(std::string value); + static Literal Binary(std::vector value); /// Create iceberg literal from bytes. /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. - static Result Deserialize(std::span data, - std::shared_ptr type); + static Result Deserialize(std::span data, + std::shared_ptr type); /// Serialize iceberg literal to bytes. /// @@ -107,12 +107,11 @@ class ICEBERG_EXPORT PrimitiveLiteral { /// \param target_type A primitive PrimitiveType /// \return A Result containing a literal of the given type or an error if conversion /// was not valid - Result CastTo( - const std::shared_ptr& target_type) const; + Result CastTo(const std::shared_ptr& target_type) const; /// Compare two PrimitiveLiterals. Both literals must have the same type /// and should not be AboveMax or BelowMin. - std::partial_ordering operator<=>(const PrimitiveLiteral& other) const; + std::partial_ordering operator<=>(const Literal& other) const; bool IsAboveMax() const; bool IsBelowMin() const; @@ -120,9 +119,9 @@ class ICEBERG_EXPORT PrimitiveLiteral { std::string ToString() const; private: - PrimitiveLiteral(Value value, std::shared_ptr type); + Literal(Value value, std::shared_ptr type); - friend class PrimitiveLiteralCaster; + friend class LiteralCaster; private: Value value_; diff --git a/test/literal_test.cc b/test/literal_test.cc index 60018770..74c87ba6 100644 --- a/test/literal_test.cc +++ b/test/literal_test.cc @@ -32,8 +32,8 @@ namespace iceberg { // Boolean type tests TEST(PrimitiveLiteralTest, BooleanBasics) { - auto true_literal = PrimitiveLiteral::Boolean(true); - auto false_literal = PrimitiveLiteral::Boolean(false); + auto true_literal = Literal::Boolean(true); + auto false_literal = Literal::Boolean(false); EXPECT_EQ(true_literal.type()->type_id(), TypeId::kBoolean); EXPECT_EQ(false_literal.type()->type_id(), TypeId::kBoolean); @@ -43,9 +43,9 @@ TEST(PrimitiveLiteralTest, BooleanBasics) { } TEST(PrimitiveLiteralTest, BooleanComparison) { - auto true_literal = PrimitiveLiteral::Boolean(true); - auto false_literal = PrimitiveLiteral::Boolean(false); - auto another_true = PrimitiveLiteral::Boolean(true); + auto true_literal = Literal::Boolean(true); + auto false_literal = Literal::Boolean(false); + auto another_true = Literal::Boolean(true); EXPECT_EQ(true_literal <=> another_true, std::partial_ordering::equivalent); EXPECT_EQ(true_literal <=> false_literal, std::partial_ordering::greater); @@ -54,8 +54,8 @@ TEST(PrimitiveLiteralTest, BooleanComparison) { // Int type tests TEST(PrimitiveLiteralTest, IntBasics) { - auto int_literal = PrimitiveLiteral::Int(42); - auto negative_int = PrimitiveLiteral::Int(-123); + auto int_literal = Literal::Int(42); + auto negative_int = Literal::Int(-123); EXPECT_EQ(int_literal.type()->type_id(), TypeId::kInt); EXPECT_EQ(negative_int.type()->type_id(), TypeId::kInt); @@ -65,9 +65,9 @@ TEST(PrimitiveLiteralTest, IntBasics) { } TEST(PrimitiveLiteralTest, IntComparison) { - auto int1 = PrimitiveLiteral::Int(10); - auto int2 = PrimitiveLiteral::Int(20); - auto int3 = PrimitiveLiteral::Int(10); + auto int1 = Literal::Int(10); + auto int2 = Literal::Int(20); + auto int3 = Literal::Int(10); EXPECT_EQ(int1 <=> int3, std::partial_ordering::equivalent); EXPECT_EQ(int1 <=> int2, std::partial_ordering::less); @@ -75,7 +75,7 @@ TEST(PrimitiveLiteralTest, IntComparison) { } TEST(PrimitiveLiteralTest, IntCastTo) { - auto int_literal = PrimitiveLiteral::Int(42); + auto int_literal = Literal::Int(42); // Cast to Long auto long_result = int_literal.CastTo(std::make_shared()); @@ -96,8 +96,8 @@ TEST(PrimitiveLiteralTest, IntCastTo) { // Long type tests TEST(PrimitiveLiteralTest, LongBasics) { - auto long_literal = PrimitiveLiteral::Long(1234567890L); - auto negative_long = PrimitiveLiteral::Long(-9876543210L); + auto long_literal = Literal::Long(1234567890L); + auto negative_long = Literal::Long(-9876543210L); EXPECT_EQ(long_literal.type()->type_id(), TypeId::kLong); EXPECT_EQ(negative_long.type()->type_id(), TypeId::kLong); @@ -107,9 +107,9 @@ TEST(PrimitiveLiteralTest, LongBasics) { } TEST(PrimitiveLiteralTest, LongComparison) { - auto long1 = PrimitiveLiteral::Long(100L); - auto long2 = PrimitiveLiteral::Long(200L); - auto long3 = PrimitiveLiteral::Long(100L); + auto long1 = Literal::Long(100L); + auto long2 = Literal::Long(200L); + auto long3 = Literal::Long(100L); EXPECT_EQ(long1 <=> long3, std::partial_ordering::equivalent); EXPECT_EQ(long1 <=> long2, std::partial_ordering::less); @@ -117,7 +117,7 @@ TEST(PrimitiveLiteralTest, LongComparison) { } TEST(PrimitiveLiteralTest, LongCastTo) { - auto long_literal = PrimitiveLiteral::Long(42L); + auto long_literal = Literal::Long(42L); // Cast to Int (within range) auto int_result = long_literal.CastTo(std::make_shared()); @@ -138,10 +138,10 @@ TEST(PrimitiveLiteralTest, LongCastTo) { TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { // Test overflow cases - auto max_long = PrimitiveLiteral::Long( - static_cast(std::numeric_limits::max()) + 1); - auto min_long = PrimitiveLiteral::Long( - static_cast(std::numeric_limits::min()) - 1); + auto max_long = + Literal::Long(static_cast(std::numeric_limits::max()) + 1); + auto min_long = + Literal::Long(static_cast(std::numeric_limits::min()) - 1); auto max_result = max_long.CastTo(std::make_shared()); ASSERT_THAT(max_result, IsOk()); @@ -154,8 +154,8 @@ TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { // Float type tests TEST(PrimitiveLiteralTest, FloatBasics) { - auto float_literal = PrimitiveLiteral::Float(3.14f); - auto negative_float = PrimitiveLiteral::Float(-2.71f); + auto float_literal = Literal::Float(3.14f); + auto negative_float = Literal::Float(-2.71f); EXPECT_EQ(float_literal.type()->type_id(), TypeId::kFloat); EXPECT_EQ(negative_float.type()->type_id(), TypeId::kFloat); @@ -165,9 +165,9 @@ TEST(PrimitiveLiteralTest, FloatBasics) { } TEST(PrimitiveLiteralTest, FloatComparison) { - auto float1 = PrimitiveLiteral::Float(1.5f); - auto float2 = PrimitiveLiteral::Float(2.5f); - auto float3 = PrimitiveLiteral::Float(1.5f); + auto float1 = Literal::Float(1.5f); + auto float2 = Literal::Float(2.5f); + auto float3 = Literal::Float(1.5f); EXPECT_EQ(float1 <=> float3, std::partial_ordering::equivalent); EXPECT_EQ(float1 <=> float2, std::partial_ordering::less); @@ -175,7 +175,7 @@ TEST(PrimitiveLiteralTest, FloatComparison) { } TEST(PrimitiveLiteralTest, FloatCastTo) { - auto float_literal = PrimitiveLiteral::Float(3.14f); + auto float_literal = Literal::Float(3.14f); // Cast to Double auto double_result = float_literal.CastTo(std::make_shared()); @@ -185,8 +185,8 @@ TEST(PrimitiveLiteralTest, FloatCastTo) { // Double type tests TEST(PrimitiveLiteralTest, DoubleBasics) { - auto double_literal = PrimitiveLiteral::Double(std::numbers::pi); - auto negative_double = PrimitiveLiteral::Double(-std::numbers::e); + auto double_literal = Literal::Double(std::numbers::pi); + auto negative_double = Literal::Double(-std::numbers::e); EXPECT_EQ(double_literal.type()->type_id(), TypeId::kDouble); EXPECT_EQ(negative_double.type()->type_id(), TypeId::kDouble); @@ -196,9 +196,9 @@ TEST(PrimitiveLiteralTest, DoubleBasics) { } TEST(PrimitiveLiteralTest, DoubleComparison) { - auto double1 = PrimitiveLiteral::Double(1.5); - auto double2 = PrimitiveLiteral::Double(2.5); - auto double3 = PrimitiveLiteral::Double(1.5); + auto double1 = Literal::Double(1.5); + auto double2 = Literal::Double(2.5); + auto double3 = Literal::Double(1.5); EXPECT_EQ(double1 <=> double3, std::partial_ordering::equivalent); EXPECT_EQ(double1 <=> double2, std::partial_ordering::less); @@ -207,8 +207,8 @@ TEST(PrimitiveLiteralTest, DoubleComparison) { // String type tests TEST(PrimitiveLiteralTest, StringBasics) { - auto string_literal = PrimitiveLiteral::String("hello world"); - auto empty_string = PrimitiveLiteral::String(""); + auto string_literal = Literal::String("hello world"); + auto empty_string = Literal::String(""); EXPECT_EQ(string_literal.type()->type_id(), TypeId::kString); EXPECT_EQ(empty_string.type()->type_id(), TypeId::kString); @@ -218,9 +218,9 @@ TEST(PrimitiveLiteralTest, StringBasics) { } TEST(PrimitiveLiteralTest, StringComparison) { - auto string1 = PrimitiveLiteral::String("apple"); - auto string2 = PrimitiveLiteral::String("banana"); - auto string3 = PrimitiveLiteral::String("apple"); + auto string1 = Literal::String("apple"); + auto string2 = Literal::String("banana"); + auto string3 = Literal::String("apple"); EXPECT_EQ(string1 <=> string3, std::partial_ordering::equivalent); EXPECT_EQ(string1 <=> string2, std::partial_ordering::less); @@ -230,8 +230,8 @@ TEST(PrimitiveLiteralTest, StringComparison) { // Binary type tests TEST(PrimitiveLiteralTest, BinaryBasics) { std::vector data = {0x01, 0x02, 0x03, 0xFF}; - auto binary_literal = PrimitiveLiteral::Binary(data); - auto empty_binary = PrimitiveLiteral::Binary({}); + auto binary_literal = Literal::Binary(data); + auto empty_binary = Literal::Binary({}); EXPECT_EQ(binary_literal.type()->type_id(), TypeId::kBinary); EXPECT_EQ(empty_binary.type()->type_id(), TypeId::kBinary); @@ -245,9 +245,9 @@ TEST(PrimitiveLiteralTest, BinaryComparison) { std::vector data2 = {0x01, 0x03}; std::vector data3 = {0x01, 0x02}; - auto binary1 = PrimitiveLiteral::Binary(data1); - auto binary2 = PrimitiveLiteral::Binary(data2); - auto binary3 = PrimitiveLiteral::Binary(data3); + auto binary1 = Literal::Binary(data1); + auto binary2 = Literal::Binary(data2); + auto binary3 = Literal::Binary(data3); EXPECT_EQ(binary1 <=> binary3, std::partial_ordering::equivalent); EXPECT_EQ(binary1 <=> binary2, std::partial_ordering::less); @@ -256,8 +256,8 @@ TEST(PrimitiveLiteralTest, BinaryComparison) { // Cross-type comparison tests TEST(PrimitiveLiteralTest, CrossTypeComparison) { - auto int_literal = PrimitiveLiteral::Int(42); - auto string_literal = PrimitiveLiteral::String("42"); + auto int_literal = Literal::Int(42); + auto string_literal = Literal::String("42"); // Different types should return unordered EXPECT_EQ(int_literal <=> string_literal, std::partial_ordering::unordered); @@ -265,7 +265,7 @@ TEST(PrimitiveLiteralTest, CrossTypeComparison) { // Special value tests TEST(PrimitiveLiteralTest, SpecialValues) { - auto int_literal = PrimitiveLiteral::Int(42); + auto int_literal = Literal::Int(42); EXPECT_FALSE(int_literal.IsAboveMax()); EXPECT_FALSE(int_literal.IsBelowMin()); @@ -273,7 +273,7 @@ TEST(PrimitiveLiteralTest, SpecialValues) { // Same type cast test TEST(PrimitiveLiteralTest, SameTypeCast) { - auto int_literal = PrimitiveLiteral::Int(42); + auto int_literal = Literal::Int(42); auto same_type_result = int_literal.CastTo(std::make_shared()); ASSERT_THAT(same_type_result, IsOk()); @@ -284,14 +284,14 @@ TEST(PrimitiveLiteralTest, SameTypeCast) { // Float special values tests TEST(PrimitiveLiteralTest, FloatSpecialValuesComparison) { // Create special float values - auto neg_nan = PrimitiveLiteral::Float(-std::numeric_limits::quiet_NaN()); - auto neg_inf = PrimitiveLiteral::Float(-std::numeric_limits::infinity()); - auto neg_value = PrimitiveLiteral::Float(-1.5f); - auto neg_zero = PrimitiveLiteral::Float(-0.0f); - auto pos_zero = PrimitiveLiteral::Float(0.0f); - auto pos_value = PrimitiveLiteral::Float(1.5f); - auto pos_inf = PrimitiveLiteral::Float(std::numeric_limits::infinity()); - auto pos_nan = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); + auto neg_nan = Literal::Float(-std::numeric_limits::quiet_NaN()); + auto neg_inf = Literal::Float(-std::numeric_limits::infinity()); + auto neg_value = Literal::Float(-1.5f); + auto neg_zero = Literal::Float(-0.0f); + auto pos_zero = Literal::Float(0.0f); + auto pos_value = Literal::Float(1.5f); + auto pos_inf = Literal::Float(std::numeric_limits::infinity()); + auto pos_nan = Literal::Float(std::numeric_limits::quiet_NaN()); // Test the ordering: -NaN < -Infinity < -value < -0 < 0 < value < Infinity < NaN EXPECT_EQ(neg_nan <=> neg_inf, std::partial_ordering::less); @@ -304,10 +304,9 @@ TEST(PrimitiveLiteralTest, FloatSpecialValuesComparison) { } TEST(PrimitiveLiteralTest, FloatNaNComparison) { - auto nan1 = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); - auto nan2 = PrimitiveLiteral::Float(std::numeric_limits::quiet_NaN()); - auto signaling_nan = - PrimitiveLiteral::Float(std::numeric_limits::signaling_NaN()); + auto nan1 = Literal::Float(std::numeric_limits::quiet_NaN()); + auto nan2 = Literal::Float(std::numeric_limits::quiet_NaN()); + auto signaling_nan = Literal::Float(std::numeric_limits::signaling_NaN()); // NaN should be equal to itself in strong ordering EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); @@ -315,10 +314,10 @@ TEST(PrimitiveLiteralTest, FloatNaNComparison) { } TEST(PrimitiveLiteralTest, FloatInfinityComparison) { - auto neg_inf = PrimitiveLiteral::Float(-std::numeric_limits::infinity()); - auto pos_inf = PrimitiveLiteral::Float(std::numeric_limits::infinity()); - auto max_value = PrimitiveLiteral::Float(std::numeric_limits::max()); - auto min_value = PrimitiveLiteral::Float(std::numeric_limits::lowest()); + auto neg_inf = Literal::Float(-std::numeric_limits::infinity()); + auto pos_inf = Literal::Float(std::numeric_limits::infinity()); + auto max_value = Literal::Float(std::numeric_limits::max()); + auto min_value = Literal::Float(std::numeric_limits::lowest()); EXPECT_EQ(neg_inf <=> min_value, std::partial_ordering::less); EXPECT_EQ(max_value <=> pos_inf, std::partial_ordering::less); @@ -326,8 +325,8 @@ TEST(PrimitiveLiteralTest, FloatInfinityComparison) { } TEST(PrimitiveLiteralTest, FloatZeroComparison) { - auto neg_zero = PrimitiveLiteral::Float(-0.0f); - auto pos_zero = PrimitiveLiteral::Float(0.0f); + auto neg_zero = Literal::Float(-0.0f); + auto pos_zero = Literal::Float(0.0f); // -0 should be less than +0 EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); @@ -336,14 +335,14 @@ TEST(PrimitiveLiteralTest, FloatZeroComparison) { // Double special values tests TEST(PrimitiveLiteralTest, DoubleSpecialValuesComparison) { // Create special double values - auto neg_nan = PrimitiveLiteral::Double(-std::numeric_limits::quiet_NaN()); - auto neg_inf = PrimitiveLiteral::Double(-std::numeric_limits::infinity()); - auto neg_value = PrimitiveLiteral::Double(-1.5); - auto neg_zero = PrimitiveLiteral::Double(-0.0); - auto pos_zero = PrimitiveLiteral::Double(0.0); - auto pos_value = PrimitiveLiteral::Double(1.5); - auto pos_inf = PrimitiveLiteral::Double(std::numeric_limits::infinity()); - auto pos_nan = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); + auto neg_nan = Literal::Double(-std::numeric_limits::quiet_NaN()); + auto neg_inf = Literal::Double(-std::numeric_limits::infinity()); + auto neg_value = Literal::Double(-1.5); + auto neg_zero = Literal::Double(-0.0); + auto pos_zero = Literal::Double(0.0); + auto pos_value = Literal::Double(1.5); + auto pos_inf = Literal::Double(std::numeric_limits::infinity()); + auto pos_nan = Literal::Double(std::numeric_limits::quiet_NaN()); // Test the ordering: -NaN < -Infinity < -value < -0 < 0 < value < Infinity < NaN EXPECT_EQ(neg_nan <=> neg_inf, std::partial_ordering::less); @@ -356,10 +355,9 @@ TEST(PrimitiveLiteralTest, DoubleSpecialValuesComparison) { } TEST(PrimitiveLiteralTest, DoubleNaNComparison) { - auto nan1 = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); - auto nan2 = PrimitiveLiteral::Double(std::numeric_limits::quiet_NaN()); - auto signaling_nan = - PrimitiveLiteral::Double(std::numeric_limits::signaling_NaN()); + auto nan1 = Literal::Double(std::numeric_limits::quiet_NaN()); + auto nan2 = Literal::Double(std::numeric_limits::quiet_NaN()); + auto signaling_nan = Literal::Double(std::numeric_limits::signaling_NaN()); // NaN should be equal to itself in strong ordering EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); @@ -367,10 +365,10 @@ TEST(PrimitiveLiteralTest, DoubleNaNComparison) { } TEST(PrimitiveLiteralTest, DoubleInfinityComparison) { - auto neg_inf = PrimitiveLiteral::Double(-std::numeric_limits::infinity()); - auto pos_inf = PrimitiveLiteral::Double(std::numeric_limits::infinity()); - auto max_value = PrimitiveLiteral::Double(std::numeric_limits::max()); - auto min_value = PrimitiveLiteral::Double(std::numeric_limits::lowest()); + auto neg_inf = Literal::Double(-std::numeric_limits::infinity()); + auto pos_inf = Literal::Double(std::numeric_limits::infinity()); + auto max_value = Literal::Double(std::numeric_limits::max()); + auto min_value = Literal::Double(std::numeric_limits::lowest()); EXPECT_EQ(neg_inf <=> min_value, std::partial_ordering::less); EXPECT_EQ(max_value <=> pos_inf, std::partial_ordering::less); @@ -378,8 +376,8 @@ TEST(PrimitiveLiteralTest, DoubleInfinityComparison) { } TEST(PrimitiveLiteralTest, DoubleZeroComparison) { - auto neg_zero = PrimitiveLiteral::Double(-0.0); - auto pos_zero = PrimitiveLiteral::Double(0.0); + auto neg_zero = Literal::Double(-0.0); + auto pos_zero = Literal::Double(0.0); // -0 should be less than +0 EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less); From 470259d13d97238144c645e8ae6569f7cd85e7ca Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 22:49:48 +0800 Subject: [PATCH 14/19] Apply suggestions from code review Co-authored-by: Gang Wu --- src/iceberg/expression/literal.h | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/iceberg/expression/literal.h b/src/iceberg/expression/literal.h index fa1cce5f..89a81dc1 100644 --- a/src/iceberg/expression/literal.h +++ b/src/iceberg/expression/literal.h @@ -33,21 +33,17 @@ namespace iceberg { /// \brief Literal is a literal value that is associated with a primitive type. class ICEBERG_EXPORT Literal { private: - /// \brief Exception type for values that are below the minimum allowed value for a + /// \brief Sentinel value to indicate that the literal value is below the valid range + /// of a specific primitive type. It can happen when casting a literal to a narrower /// primitive type. - /// - /// When casting a value to a narrow primitive type, if the value exceeds the maximum of - /// target type, it might be above the maximum allowed value for that type. struct BelowMin { bool operator==(const BelowMin&) const = default; std::strong_ordering operator<=>(const BelowMin&) const = default; }; - /// \brief Exception type for values that are above the maximum allowed value for a + /// \brief Sentinel value to indicate that the literal value is above the valid range + /// of a specific primitive type. It can happen when casting a literal to a narrower /// primitive type. - /// - /// When casting a value to a narrow primitive type, if the value exceeds the maximum of - /// target type, it might be above the maximum allowed value for that type. struct AboveMax { bool operator==(const AboveMax&) const = default; std::strong_ordering operator<=>(const AboveMax&) const = default; @@ -64,7 +60,7 @@ class ICEBERG_EXPORT Literal { BelowMin, AboveMax>; public: - /// Factory methods for primitive types + /// \brief Factory methods for primitive types static Literal Boolean(bool value); static Literal Int(int32_t value); static Literal Long(int64_t value); @@ -73,23 +69,23 @@ class ICEBERG_EXPORT Literal { static Literal String(std::string value); static Literal Binary(std::vector value); - /// Create iceberg literal from bytes. + /// \brief Restore a literal from single-value serialization. /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. static Result Deserialize(std::span data, std::shared_ptr type); - /// Serialize iceberg literal to bytes. + /// \brief Perform single-value serialization. /// /// See [this spec](https://iceberg.apache.org/spec/#binary-single-value-serialization) /// for reference. Result> Serialize() const; - /// Get the Iceberg Type of the literal. + /// \brief Get the literal type. const std::shared_ptr& type() const; - /// Converts this literal to a literal of the given type. + /// \brief Converts this literal to a literal of the given type. /// /// When a predicate is bound to a concrete data column, literals are converted to match /// the bound column's type. This conversion process is more narrow than a cast and is @@ -109,7 +105,7 @@ class ICEBERG_EXPORT Literal { /// was not valid Result CastTo(const std::shared_ptr& target_type) const; - /// Compare two PrimitiveLiterals. Both literals must have the same type + /// \brief Compare two PrimitiveLiterals. Both literals must have the same type /// and should not be AboveMax or BelowMin. std::partial_ordering operator<=>(const Literal& other) const; From 56c265ef663dd533e2ccc35d177193825f098d09 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 22:58:30 +0800 Subject: [PATCH 15/19] Resolve comments --- src/iceberg/expression/literal.cc | 26 +++++++++++++------------- test/CMakeLists.txt | 7 +------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index fd48b126..ce30b511 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -77,7 +77,7 @@ Result LiteralCaster::CastFromInt( return Literal::Double(static_cast(int_val)); default: return NotSupported("Cast from Int to {} is not implemented", - static_cast(target_type_id)); + target_type->ToString()); } } @@ -103,7 +103,7 @@ Result LiteralCaster::CastFromLong( return Literal::Double(static_cast(long_val)); default: return NotSupported("Cast from Long to {} is not supported", - static_cast(target_type_id)); + target_type->ToString()); } } @@ -117,7 +117,7 @@ Result LiteralCaster::CastFromFloat( return Literal::Double(static_cast(float_val)); default: return NotSupported("Cast from Float to {} is not supported", - static_cast(target_type_id)); + target_type->ToString()); } } @@ -175,21 +175,21 @@ Result Literal::CastTo(const std::shared_ptr& target_typ // Template function for floating point comparison following Iceberg rules: // -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign template -std::partial_ordering iceberg_float_compare(T lhs, T rhs) { +std::strong_ordering CompareFloat(T lhs, T rhs) { + // If both are NaN, check their signs bool lhs_is_nan = std::isnan(lhs); bool rhs_is_nan = std::isnan(rhs); - - // If both are NaN, check their signs if (lhs_is_nan && rhs_is_nan) { bool lhs_is_negative = std::signbit(lhs); bool rhs_is_negative = std::signbit(rhs); if (lhs_is_negative == rhs_is_negative) { // Same sign NaN values are equivalent (no qNaN vs sNaN distinction) - return std::partial_ordering::equivalent; + return std::strong_ordering::equivalent; } // -NaN < NaN - return lhs_is_negative ? std::partial_ordering::less : std::partial_ordering::greater; + return lhs_is_negative ? std::strong_ordering::less + : std::strong_ordering::greater; } // For non-NaN values, use standard strong ordering @@ -233,14 +233,14 @@ std::partial_ordering Literal::operator<=>(const Literal& other) const { auto this_val = std::get(value_); auto other_val = std::get(other.value_); // Use strong_ordering for floating point as spec requests - return iceberg_float_compare(this_val, other_val); + return CompareFloat(this_val, other_val); } case TypeId::kDouble: { auto this_val = std::get(value_); auto other_val = std::get(other.value_); // Use strong_ordering for floating point as spec requests - return iceberg_float_compare(this_val, other_val); + return CompareFloat(this_val, other_val); } case TypeId::kString: { @@ -263,10 +263,10 @@ std::partial_ordering Literal::operator<=>(const Literal& other) const { std::string Literal::ToString() const { if (std::holds_alternative(value_)) { - return "BelowMin"; + return "belowMin"; } if (std::holds_alternative(value_)) { - return "AboveMax"; + return "aboveMax"; } switch (type_->type_id()) { @@ -293,7 +293,7 @@ std::string Literal::ToString() const { std::string result; result.reserve(binary_data.size() * 2); // 2 chars per byte for (const auto& byte : binary_data) { - result += std::format("{:02X}", byte); + std::format_to(std::back_inserter(result), "{:02X}", byte); } return result; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6ba48f30..64f8e03f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -50,7 +50,7 @@ target_link_libraries(catalog_test PRIVATE iceberg_static GTest::gtest_main GTes add_test(NAME catalog_test COMMAND catalog_test) add_executable(expression_test) -target_sources(expression_test PRIVATE expression_test.cc) +target_sources(expression_test PRIVATE expression_test.cc literal_test.cc) target_link_libraries(expression_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) add_test(NAME expression_test COMMAND expression_test) @@ -69,11 +69,6 @@ target_sources(util_test PRIVATE expected_test.cc formatter_test.cc config_test. target_link_libraries(util_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) add_test(NAME util_test COMMAND util_test) -add_executable(literal_test) -target_sources(literal_test PRIVATE literal_test.cc) -target_link_libraries(literal_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) -add_test(NAME literal_test COMMAND literal_test) - if(ICEBERG_BUILD_BUNDLE) add_executable(avro_test) target_sources(avro_test PRIVATE avro_test.cc avro_schema_test.cc avro_stream_test.cc) From c46ff6096eeef81487eb893aab5d231650b9c236 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 23:05:10 +0800 Subject: [PATCH 16/19] continue resolve comments --- src/iceberg/expression/literal.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/iceberg/expression/literal.h b/src/iceberg/expression/literal.h index 89a81dc1..17752c48 100644 --- a/src/iceberg/expression/literal.h +++ b/src/iceberg/expression/literal.h @@ -109,7 +109,16 @@ class ICEBERG_EXPORT Literal { /// and should not be AboveMax or BelowMin. std::partial_ordering operator<=>(const Literal& other) const; + /// Check if this literal represents a value above the maximum allowed value + /// for its type. This occurs when casting from a wider type to a narrower type + /// and the value exceeds the target type's maximum. + /// \return true if this literal represents an AboveMax value, false otherwise bool IsAboveMax() const; + + /// Check if this literal represents a value below the minimum allowed value + /// for its type. This occurs when casting from a wider type to a narrower type + /// and the value is less than the target type's minimum. + /// \return true if this literal represents a BelowMin value, false otherwise bool IsBelowMin() const; std::string ToString() const; From 197102eea03a93197751ffab1485baa6a61f20b7 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 20 Jun 2025 23:46:15 +0800 Subject: [PATCH 17/19] Fix lint --- src/iceberg/expression/literal.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index ce30b511..f0f5b9dd 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -188,8 +188,7 @@ std::strong_ordering CompareFloat(T lhs, T rhs) { return std::strong_ordering::equivalent; } // -NaN < NaN - return lhs_is_negative ? std::strong_ordering::less - : std::strong_ordering::greater; + return lhs_is_negative ? std::strong_ordering::less : std::strong_ordering::greater; } // For non-NaN values, use standard strong ordering From c9e5765fb99a3e4bc076dbca66600c5289c38109 Mon Sep 17 00:00:00 2001 From: mwish Date: Mon, 23 Jun 2025 23:30:32 +0800 Subject: [PATCH 18/19] Rename tests --- test/literal_test.cc | 58 ++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/test/literal_test.cc b/test/literal_test.cc index 74c87ba6..6821c569 100644 --- a/test/literal_test.cc +++ b/test/literal_test.cc @@ -31,7 +31,7 @@ namespace iceberg { // Boolean type tests -TEST(PrimitiveLiteralTest, BooleanBasics) { +TEST(LiteralTest, BooleanBasics) { auto true_literal = Literal::Boolean(true); auto false_literal = Literal::Boolean(false); @@ -42,7 +42,7 @@ TEST(PrimitiveLiteralTest, BooleanBasics) { EXPECT_EQ(false_literal.ToString(), "false"); } -TEST(PrimitiveLiteralTest, BooleanComparison) { +TEST(LiteralTest, BooleanComparison) { auto true_literal = Literal::Boolean(true); auto false_literal = Literal::Boolean(false); auto another_true = Literal::Boolean(true); @@ -53,7 +53,7 @@ TEST(PrimitiveLiteralTest, BooleanComparison) { } // Int type tests -TEST(PrimitiveLiteralTest, IntBasics) { +TEST(LiteralTest, IntBasics) { auto int_literal = Literal::Int(42); auto negative_int = Literal::Int(-123); @@ -64,7 +64,7 @@ TEST(PrimitiveLiteralTest, IntBasics) { EXPECT_EQ(negative_int.ToString(), "-123"); } -TEST(PrimitiveLiteralTest, IntComparison) { +TEST(LiteralTest, IntComparison) { auto int1 = Literal::Int(10); auto int2 = Literal::Int(20); auto int3 = Literal::Int(10); @@ -74,7 +74,7 @@ TEST(PrimitiveLiteralTest, IntComparison) { EXPECT_EQ(int2 <=> int1, std::partial_ordering::greater); } -TEST(PrimitiveLiteralTest, IntCastTo) { +TEST(LiteralTest, IntCastTo) { auto int_literal = Literal::Int(42); // Cast to Long @@ -95,7 +95,7 @@ TEST(PrimitiveLiteralTest, IntCastTo) { } // Long type tests -TEST(PrimitiveLiteralTest, LongBasics) { +TEST(LiteralTest, LongBasics) { auto long_literal = Literal::Long(1234567890L); auto negative_long = Literal::Long(-9876543210L); @@ -106,7 +106,7 @@ TEST(PrimitiveLiteralTest, LongBasics) { EXPECT_EQ(negative_long.ToString(), "-9876543210"); } -TEST(PrimitiveLiteralTest, LongComparison) { +TEST(LiteralTest, LongComparison) { auto long1 = Literal::Long(100L); auto long2 = Literal::Long(200L); auto long3 = Literal::Long(100L); @@ -116,7 +116,7 @@ TEST(PrimitiveLiteralTest, LongComparison) { EXPECT_EQ(long2 <=> long1, std::partial_ordering::greater); } -TEST(PrimitiveLiteralTest, LongCastTo) { +TEST(LiteralTest, LongCastTo) { auto long_literal = Literal::Long(42L); // Cast to Int (within range) @@ -136,7 +136,7 @@ TEST(PrimitiveLiteralTest, LongCastTo) { EXPECT_EQ(double_result->type()->type_id(), TypeId::kDouble); } -TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { +TEST(LiteralTest, LongCastToIntOverflow) { // Test overflow cases auto max_long = Literal::Long(static_cast(std::numeric_limits::max()) + 1); @@ -153,7 +153,7 @@ TEST(PrimitiveLiteralTest, LongCastToIntOverflow) { } // Float type tests -TEST(PrimitiveLiteralTest, FloatBasics) { +TEST(LiteralTest, FloatBasics) { auto float_literal = Literal::Float(3.14f); auto negative_float = Literal::Float(-2.71f); @@ -164,7 +164,7 @@ TEST(PrimitiveLiteralTest, FloatBasics) { EXPECT_EQ(negative_float.ToString(), "-2.710000"); } -TEST(PrimitiveLiteralTest, FloatComparison) { +TEST(LiteralTest, FloatComparison) { auto float1 = Literal::Float(1.5f); auto float2 = Literal::Float(2.5f); auto float3 = Literal::Float(1.5f); @@ -174,7 +174,7 @@ TEST(PrimitiveLiteralTest, FloatComparison) { EXPECT_EQ(float2 <=> float1, std::partial_ordering::greater); } -TEST(PrimitiveLiteralTest, FloatCastTo) { +TEST(LiteralTest, FloatCastTo) { auto float_literal = Literal::Float(3.14f); // Cast to Double @@ -184,7 +184,7 @@ TEST(PrimitiveLiteralTest, FloatCastTo) { } // Double type tests -TEST(PrimitiveLiteralTest, DoubleBasics) { +TEST(LiteralTest, DoubleBasics) { auto double_literal = Literal::Double(std::numbers::pi); auto negative_double = Literal::Double(-std::numbers::e); @@ -195,7 +195,7 @@ TEST(PrimitiveLiteralTest, DoubleBasics) { EXPECT_EQ(negative_double.ToString(), "-2.718282"); } -TEST(PrimitiveLiteralTest, DoubleComparison) { +TEST(LiteralTest, DoubleComparison) { auto double1 = Literal::Double(1.5); auto double2 = Literal::Double(2.5); auto double3 = Literal::Double(1.5); @@ -206,7 +206,7 @@ TEST(PrimitiveLiteralTest, DoubleComparison) { } // String type tests -TEST(PrimitiveLiteralTest, StringBasics) { +TEST(LiteralTest, StringBasics) { auto string_literal = Literal::String("hello world"); auto empty_string = Literal::String(""); @@ -217,7 +217,7 @@ TEST(PrimitiveLiteralTest, StringBasics) { EXPECT_EQ(empty_string.ToString(), ""); } -TEST(PrimitiveLiteralTest, StringComparison) { +TEST(LiteralTest, StringComparison) { auto string1 = Literal::String("apple"); auto string2 = Literal::String("banana"); auto string3 = Literal::String("apple"); @@ -228,7 +228,7 @@ TEST(PrimitiveLiteralTest, StringComparison) { } // Binary type tests -TEST(PrimitiveLiteralTest, BinaryBasics) { +TEST(LiteralTest, BinaryBasics) { std::vector data = {0x01, 0x02, 0x03, 0xFF}; auto binary_literal = Literal::Binary(data); auto empty_binary = Literal::Binary({}); @@ -240,7 +240,7 @@ TEST(PrimitiveLiteralTest, BinaryBasics) { EXPECT_EQ(empty_binary.ToString(), ""); } -TEST(PrimitiveLiteralTest, BinaryComparison) { +TEST(LiteralTest, BinaryComparison) { std::vector data1 = {0x01, 0x02}; std::vector data2 = {0x01, 0x03}; std::vector data3 = {0x01, 0x02}; @@ -255,7 +255,7 @@ TEST(PrimitiveLiteralTest, BinaryComparison) { } // Cross-type comparison tests -TEST(PrimitiveLiteralTest, CrossTypeComparison) { +TEST(LiteralTest, CrossTypeComparison) { auto int_literal = Literal::Int(42); auto string_literal = Literal::String("42"); @@ -264,7 +264,7 @@ TEST(PrimitiveLiteralTest, CrossTypeComparison) { } // Special value tests -TEST(PrimitiveLiteralTest, SpecialValues) { +TEST(LiteralTest, SpecialValues) { auto int_literal = Literal::Int(42); EXPECT_FALSE(int_literal.IsAboveMax()); @@ -272,7 +272,7 @@ TEST(PrimitiveLiteralTest, SpecialValues) { } // Same type cast test -TEST(PrimitiveLiteralTest, SameTypeCast) { +TEST(LiteralTest, SameTypeCast) { auto int_literal = Literal::Int(42); auto same_type_result = int_literal.CastTo(std::make_shared()); @@ -282,7 +282,7 @@ TEST(PrimitiveLiteralTest, SameTypeCast) { } // Float special values tests -TEST(PrimitiveLiteralTest, FloatSpecialValuesComparison) { +TEST(LiteralTest, FloatSpecialValuesComparison) { // Create special float values auto neg_nan = Literal::Float(-std::numeric_limits::quiet_NaN()); auto neg_inf = Literal::Float(-std::numeric_limits::infinity()); @@ -303,7 +303,7 @@ TEST(PrimitiveLiteralTest, FloatSpecialValuesComparison) { EXPECT_EQ(pos_inf <=> pos_nan, std::partial_ordering::less); } -TEST(PrimitiveLiteralTest, FloatNaNComparison) { +TEST(LiteralTest, FloatNaNComparison) { auto nan1 = Literal::Float(std::numeric_limits::quiet_NaN()); auto nan2 = Literal::Float(std::numeric_limits::quiet_NaN()); auto signaling_nan = Literal::Float(std::numeric_limits::signaling_NaN()); @@ -313,7 +313,7 @@ TEST(PrimitiveLiteralTest, FloatNaNComparison) { EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); } -TEST(PrimitiveLiteralTest, FloatInfinityComparison) { +TEST(LiteralTest, FloatInfinityComparison) { auto neg_inf = Literal::Float(-std::numeric_limits::infinity()); auto pos_inf = Literal::Float(std::numeric_limits::infinity()); auto max_value = Literal::Float(std::numeric_limits::max()); @@ -324,7 +324,7 @@ TEST(PrimitiveLiteralTest, FloatInfinityComparison) { EXPECT_EQ(neg_inf <=> pos_inf, std::partial_ordering::less); } -TEST(PrimitiveLiteralTest, FloatZeroComparison) { +TEST(LiteralTest, FloatZeroComparison) { auto neg_zero = Literal::Float(-0.0f); auto pos_zero = Literal::Float(0.0f); @@ -333,7 +333,7 @@ TEST(PrimitiveLiteralTest, FloatZeroComparison) { } // Double special values tests -TEST(PrimitiveLiteralTest, DoubleSpecialValuesComparison) { +TEST(LiteralTest, DoubleSpecialValuesComparison) { // Create special double values auto neg_nan = Literal::Double(-std::numeric_limits::quiet_NaN()); auto neg_inf = Literal::Double(-std::numeric_limits::infinity()); @@ -354,7 +354,7 @@ TEST(PrimitiveLiteralTest, DoubleSpecialValuesComparison) { EXPECT_EQ(pos_inf <=> pos_nan, std::partial_ordering::less); } -TEST(PrimitiveLiteralTest, DoubleNaNComparison) { +TEST(LiteralTest, DoubleNaNComparison) { auto nan1 = Literal::Double(std::numeric_limits::quiet_NaN()); auto nan2 = Literal::Double(std::numeric_limits::quiet_NaN()); auto signaling_nan = Literal::Double(std::numeric_limits::signaling_NaN()); @@ -364,7 +364,7 @@ TEST(PrimitiveLiteralTest, DoubleNaNComparison) { EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); } -TEST(PrimitiveLiteralTest, DoubleInfinityComparison) { +TEST(LiteralTest, DoubleInfinityComparison) { auto neg_inf = Literal::Double(-std::numeric_limits::infinity()); auto pos_inf = Literal::Double(std::numeric_limits::infinity()); auto max_value = Literal::Double(std::numeric_limits::max()); @@ -375,7 +375,7 @@ TEST(PrimitiveLiteralTest, DoubleInfinityComparison) { EXPECT_EQ(neg_inf <=> pos_inf, std::partial_ordering::less); } -TEST(PrimitiveLiteralTest, DoubleZeroComparison) { +TEST(LiteralTest, DoubleZeroComparison) { auto neg_zero = Literal::Double(-0.0); auto pos_zero = Literal::Double(0.0); From 644bb9dfd4bd36d773f50c56a59333372b296869 Mon Sep 17 00:00:00 2001 From: mwish Date: Tue, 24 Jun 2025 00:47:49 +0800 Subject: [PATCH 19/19] Compare float using <=> --- src/iceberg/expression/literal.cc | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index f0f5b9dd..8392f34c 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -177,22 +177,16 @@ Result Literal::CastTo(const std::shared_ptr& target_typ template std::strong_ordering CompareFloat(T lhs, T rhs) { // If both are NaN, check their signs - bool lhs_is_nan = std::isnan(lhs); - bool rhs_is_nan = std::isnan(rhs); - if (lhs_is_nan && rhs_is_nan) { - bool lhs_is_negative = std::signbit(lhs); - bool rhs_is_negative = std::signbit(rhs); - - if (lhs_is_negative == rhs_is_negative) { - // Same sign NaN values are equivalent (no qNaN vs sNaN distinction) - return std::strong_ordering::equivalent; - } - // -NaN < NaN - return lhs_is_negative ? std::strong_ordering::less : std::strong_ordering::greater; + bool all_nan = std::isnan(lhs) && std::isnan(rhs); + if (!all_nan) { + // If not both NaN, use strong ordering + return std::strong_order(lhs, rhs); } - - // For non-NaN values, use standard strong ordering - return std::strong_order(lhs, rhs); + // Same sign NaN values are equivalent (no qNaN vs sNaN distinction), + // and -NAN < NAN. + bool lhs_is_negative = std::signbit(lhs); + bool rhs_is_negative = std::signbit(rhs); + return lhs_is_negative <=> rhs_is_negative; } // Three-way comparison operator