From cf5fa843a44288c87ce5814b7e3cec034a86fb84 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Tue, 28 Oct 2025 14:04:44 +0800 Subject: [PATCH] feat: add not expression --- src/iceberg/expression/expression.cc | 13 +++++++++ src/iceberg/expression/expression.h | 27 ++++++++++++++++++ src/iceberg/expression/expressions.cc | 19 +++++++++++++ src/iceberg/expression/expressions.h | 9 ++++++ src/iceberg/test/expression_test.cc | 41 +++++++++++++++++++++++++++ 5 files changed, 109 insertions(+) diff --git a/src/iceberg/expression/expression.cc b/src/iceberg/expression/expression.cc index f6f6d0f70..070d5fe5f 100644 --- a/src/iceberg/expression/expression.cc +++ b/src/iceberg/expression/expression.cc @@ -22,6 +22,7 @@ #include #include +#include "iceberg/util/checked_cast.h" #include "iceberg/util/formatter_internal.h" #include "iceberg/util/macros.h" @@ -91,6 +92,18 @@ bool Or::Equals(const Expression& expr) const { return false; } +// Not implementation +Not::Not(std::shared_ptr child) : child_(std::move(child)) {} + +std::string Not::ToString() const { return std::format("not({})", child_->ToString()); } + +Result> Not::Negate() const { return child_; } + +bool Not::Equals(const Expression& other) const { + return other.op() == Operation::kNot && + internal::checked_cast(other).child_->Equals(*child_); +} + std::string_view ToString(Expression::Operation op) { switch (op) { case Expression::Operation::kAnd: diff --git a/src/iceberg/expression/expression.h b/src/iceberg/expression/expression.h index e0708c4eb..9a8052209 100644 --- a/src/iceberg/expression/expression.h +++ b/src/iceberg/expression/expression.h @@ -194,6 +194,33 @@ class ICEBERG_EXPORT Or : public Expression { std::shared_ptr right_; }; +/// \brief An Expression that represents logical NOT operation. +/// +/// This expression negates its child expression. +class ICEBERG_EXPORT Not : public Expression { + public: + /// \brief Constructs a Not expression from a child expression. + /// + /// \param child The expression to negate + explicit Not(std::shared_ptr child); + + /// \brief Returns the child expression. + /// + /// \return The child expression being negated + const std::shared_ptr& child() const { return child_; } + + Operation op() const override { return Operation::kNot; } + + std::string ToString() const override; + + Result> Negate() const override; + + bool Equals(const Expression& other) const override; + + private: + std::shared_ptr child_; +}; + /// \brief Returns a string representation of an expression operation. ICEBERG_EXPORT std::string_view ToString(Expression::Operation op); diff --git a/src/iceberg/expression/expressions.cc b/src/iceberg/expression/expressions.cc index a775c9934..686a55de5 100644 --- a/src/iceberg/expression/expressions.cc +++ b/src/iceberg/expression/expressions.cc @@ -25,6 +25,25 @@ namespace iceberg { +// Logical NOT operation +std::shared_ptr Expressions::Not(std::shared_ptr child) { + if (child->op() == Expression::Operation::kTrue) { + return AlwaysFalse(); + } + + if (child->op() == Expression::Operation::kFalse) { + return AlwaysTrue(); + } + + // not(not(x)) = x + if (child->op() == Expression::Operation::kNot) { + const auto& not_expr = static_cast(*child); + return not_expr.child(); + } + + return std::make_shared<::iceberg::Not>(std::move(child)); +} + // Transform functions std::shared_ptr Expressions::Bucket(std::string name, diff --git a/src/iceberg/expression/expressions.h b/src/iceberg/expression/expressions.h index 7d9f9a1db..66083da06 100644 --- a/src/iceberg/expression/expressions.h +++ b/src/iceberg/expression/expressions.h @@ -92,6 +92,15 @@ class ICEBERG_EXPORT Expressions { } } + /// \brief Create a NOT expression. + /// + /// \param child The expression to negate + /// \return A negated expression with optimizations applied: + /// - not(true) returns false + /// - not(false) returns true + /// - not(not(x)) returns x + static std::shared_ptr Not(std::shared_ptr child); + // Transform functions /// \brief Create a bucket transform term. diff --git a/src/iceberg/test/expression_test.cc b/src/iceberg/test/expression_test.cc index cce7c5334..326dadcb1 100644 --- a/src/iceberg/test/expression_test.cc +++ b/src/iceberg/test/expression_test.cc @@ -165,4 +165,45 @@ TEST(ExpressionTest, BaseClassNegateErrorOut) { auto negate_result = mock_expr->Negate(); EXPECT_THAT(negate_result, IsError(ErrorKind::kNotSupported)); } + +TEST(NotTest, Basic) { + auto true_expr = True::Instance(); + auto not_expr = std::make_shared(true_expr); + + EXPECT_EQ(not_expr->op(), Expression::Operation::kNot); + EXPECT_EQ(not_expr->ToString(), "not(true)"); + EXPECT_EQ(not_expr->child()->op(), Expression::Operation::kTrue); +} + +TEST(NotTest, Negation) { + // Test that not(not(x)) = x + auto true_expr = True::Instance(); + auto not_expr = std::make_shared(true_expr); + + auto negated_result = not_expr->Negate(); + ASSERT_THAT(negated_result, IsOk()); + auto negated = negated_result.value(); + + // Should return the original true expression + EXPECT_EQ(negated->op(), Expression::Operation::kTrue); +} + +TEST(NotTest, Equals) { + auto true_expr = True::Instance(); + auto false_expr = False::Instance(); + + // Test basic equality + auto not_expr1 = std::make_shared(true_expr); + auto not_expr2 = std::make_shared(true_expr); + EXPECT_TRUE(not_expr1->Equals(*not_expr2)); + + // Test inequality with different child expressions + auto not_expr3 = std::make_shared(false_expr); + EXPECT_FALSE(not_expr1->Equals(*not_expr3)); + + // Test inequality with different operation types + auto and_expr = std::make_shared(true_expr, false_expr); + EXPECT_FALSE(not_expr1->Equals(*and_expr)); +} + } // namespace iceberg