This is an automated email from the ASF dual-hosted git repository.
fokko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new f2a79a1 feat: add `or` expression (#120)
f2a79a1 is described below
commit f2a79a14a4ef5d8200d0240d1122307be2a06e3f
Author: Ying Cai <ying.cai...@gmail.com>
AuthorDate: Wed Jun 25 23:03:00 2025 +0800
feat: add `or` expression (#120)
- or expression
- change negate return type form Result to Expression
- add some tests
---
src/iceberg/expression/expression.cc | 38 +++++++++---
src/iceberg/expression/expression.h | 48 ++++++++++++---
test/expression_test.cc | 114 +++++++++++++++++++++++++++++++----
3 files changed, 174 insertions(+), 26 deletions(-)
diff --git a/src/iceberg/expression/expression.cc
b/src/iceberg/expression/expression.cc
index 77f341e..c6fa940 100644
--- a/src/iceberg/expression/expression.cc
+++ b/src/iceberg/expression/expression.cc
@@ -21,8 +21,6 @@
#include <format>
-#include "iceberg/result.h"
-
namespace iceberg {
// True implementation
@@ -31,7 +29,7 @@ const std::shared_ptr<True>& True::Instance() {
return instance;
}
-Result<std::shared_ptr<Expression>> True::Negate() const { return
False::Instance(); }
+std::shared_ptr<Expression> True::Negate() const { return False::Instance(); }
// False implementation
const std::shared_ptr<False>& False::Instance() {
@@ -39,7 +37,7 @@ const std::shared_ptr<False>& False::Instance() {
return instance;
}
-Result<std::shared_ptr<Expression>> False::Negate() const { return
True::Instance(); }
+std::shared_ptr<Expression> False::Negate() const { return True::Instance(); }
// And implementation
And::And(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right)
@@ -49,9 +47,11 @@ std::string And::ToString() const {
return std::format("({} and {})", left_->ToString(), right_->ToString());
}
-Result<std::shared_ptr<Expression>> And::Negate() const {
- // TODO(yingcai-cy): Implement Or expression
- return InvalidExpression("And negation not yet implemented");
+std::shared_ptr<Expression> And::Negate() const {
+ // De Morgan's law: not(A and B) = (not A) or (not B)
+ auto left_negated = left_->Negate();
+ auto right_negated = right_->Negate();
+ return std::make_shared<Or>(left_negated, right_negated);
}
bool And::Equals(const Expression& expr) const {
@@ -63,4 +63,28 @@ bool And::Equals(const Expression& expr) const {
return false;
}
+// Or implementation
+Or::Or(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right)
+ : left_(std::move(left)), right_(std::move(right)) {}
+
+std::string Or::ToString() const {
+ return std::format("({} or {})", left_->ToString(), right_->ToString());
+}
+
+std::shared_ptr<Expression> Or::Negate() const {
+ // De Morgan's law: not(A or B) = (not A) and (not B)
+ auto left_negated = left_->Negate();
+ auto right_negated = right_->Negate();
+ return std::make_shared<And>(left_negated, right_negated);
+}
+
+bool Or::Equals(const Expression& expr) const {
+ if (expr.op() == Operation::kOr) {
+ const auto& other = static_cast<const Or&>(expr);
+ return (left_->Equals(*other.left()) && right_->Equals(*other.right())) ||
+ (left_->Equals(*other.right()) && right_->Equals(*other.left()));
+ }
+ return false;
+}
+
} // namespace iceberg
diff --git a/src/iceberg/expression/expression.h
b/src/iceberg/expression/expression.h
index 342122f..9ceae1c 100644
--- a/src/iceberg/expression/expression.h
+++ b/src/iceberg/expression/expression.h
@@ -25,9 +25,8 @@
#include <memory>
#include <string>
-#include "iceberg/expected.h"
+#include "iceberg/exception.h"
#include "iceberg/iceberg_export.h"
-#include "iceberg/result.h"
namespace iceberg {
@@ -67,8 +66,8 @@ class ICEBERG_EXPORT Expression {
virtual Operation op() const = 0;
/// \brief Returns the negation of this expression, equivalent to not(this).
- virtual Result<std::shared_ptr<Expression>> Negate() const {
- return InvalidExpression("Expression cannot be negated");
+ virtual std::shared_ptr<Expression> Negate() const {
+ throw IcebergError("Expression cannot be negated");
}
/// \brief Returns whether this expression will accept the same values as
another.
@@ -94,7 +93,7 @@ class ICEBERG_EXPORT True : public Expression {
std::string ToString() const override { return "true"; }
- Result<std::shared_ptr<Expression>> Negate() const override;
+ std::shared_ptr<Expression> Negate() const override;
bool Equals(const Expression& other) const override {
return other.op() == Operation::kTrue;
@@ -114,7 +113,7 @@ class ICEBERG_EXPORT False : public Expression {
std::string ToString() const override { return "false"; }
- Result<std::shared_ptr<Expression>> Negate() const override;
+ std::shared_ptr<Expression> Negate() const override;
bool Equals(const Expression& other) const override {
return other.op() == Operation::kFalse;
@@ -150,7 +149,42 @@ class ICEBERG_EXPORT And : public Expression {
std::string ToString() const override;
- Result<std::shared_ptr<Expression>> Negate() const override;
+ std::shared_ptr<Expression> Negate() const override;
+
+ bool Equals(const Expression& other) const override;
+
+ private:
+ std::shared_ptr<Expression> left_;
+ std::shared_ptr<Expression> right_;
+};
+
+/// \brief An Expression that represents a logical OR operation between two
expressions.
+///
+/// This expression evaluates to true if at least one of its child expressions
+/// evaluates to true.
+class ICEBERG_EXPORT Or : public Expression {
+ public:
+ /// \brief Constructs an Or expression from two sub-expressions.
+ ///
+ /// \param left The left operand of the OR expression
+ /// \param right The right operand of the OR expression
+ Or(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right);
+
+ /// \brief Returns the left operand of the OR expression.
+ ///
+ /// \return The left operand of the OR expression
+ const std::shared_ptr<Expression>& left() const { return left_; }
+
+ /// \brief Returns the right operand of the OR expression.
+ ///
+ /// \return The right operand of the OR expression
+ const std::shared_ptr<Expression>& right() const { return right_; }
+
+ Operation op() const override { return Operation::kOr; }
+
+ std::string ToString() const override;
+
+ std::shared_ptr<Expression> Negate() const override;
bool Equals(const Expression& other) const override;
diff --git a/test/expression_test.cc b/test/expression_test.cc
index f722d62..c14c7d9 100644
--- a/test/expression_test.cc
+++ b/test/expression_test.cc
@@ -30,25 +30,17 @@ TEST(TrueFalseTest, Basic) {
auto false_instance = False::Instance();
auto negated = false_instance->Negate();
- EXPECT_TRUE(negated.has_value());
-
// Check that negated expression is True
- auto true_expr = negated.value();
- EXPECT_EQ(true_expr->op(), Expression::Operation::kTrue);
-
- EXPECT_EQ(true_expr->ToString(), "true");
+ EXPECT_EQ(negated->op(), Expression::Operation::kTrue);
+ EXPECT_EQ(negated->ToString(), "true");
// Test negation of True returns false
auto true_instance = True::Instance();
negated = true_instance->Negate();
- EXPECT_TRUE(negated.has_value());
-
// Check that negated expression is False
- auto false_expr = negated.value();
- EXPECT_EQ(false_expr->op(), Expression::Operation::kFalse);
-
- EXPECT_EQ(false_expr->ToString(), "false");
+ EXPECT_EQ(negated->op(), Expression::Operation::kFalse);
+ EXPECT_EQ(negated->ToString(), "false");
}
TEST(ANDTest, Basic) {
@@ -64,4 +56,102 @@ TEST(ANDTest, Basic) {
EXPECT_EQ(and_expr->left()->op(), Expression::Operation::kTrue);
EXPECT_EQ(and_expr->right()->op(), Expression::Operation::kTrue);
}
+
+TEST(ORTest, Basic) {
+ // Create True and False expressions
+ auto true_expr = True::Instance();
+ auto false_expr = False::Instance();
+
+ // Create an OR expression
+ auto or_expr = std::make_shared<Or>(true_expr, false_expr);
+
+ EXPECT_EQ(or_expr->op(), Expression::Operation::kOr);
+ EXPECT_EQ(or_expr->ToString(), "(true or false)");
+ EXPECT_EQ(or_expr->left()->op(), Expression::Operation::kTrue);
+ EXPECT_EQ(or_expr->right()->op(), Expression::Operation::kFalse);
+}
+
+TEST(ORTest, Negation) {
+ // Test De Morgan's law: not(A or B) = (not A) and (not B)
+ auto true_expr = True::Instance();
+ auto false_expr = False::Instance();
+
+ auto or_expr = std::make_shared<Or>(true_expr, false_expr);
+ auto negated_or = or_expr->Negate();
+
+ // Should become AND expression
+ EXPECT_EQ(negated_or->op(), Expression::Operation::kAnd);
+ EXPECT_EQ(negated_or->ToString(), "(false and true)");
+}
+
+TEST(ORTest, Equals) {
+ auto true_expr = True::Instance();
+ auto false_expr = False::Instance();
+
+ // Test basic equality
+ auto or_expr1 = std::make_shared<Or>(true_expr, false_expr);
+ auto or_expr2 = std::make_shared<Or>(true_expr, false_expr);
+ EXPECT_TRUE(or_expr1->Equals(*or_expr2));
+
+ // Test commutativity: (A or B) equals (B or A)
+ auto or_expr3 = std::make_shared<Or>(false_expr, true_expr);
+ EXPECT_TRUE(or_expr1->Equals(*or_expr3));
+
+ // Test inequality with different expressions
+ auto or_expr4 = std::make_shared<Or>(true_expr, true_expr);
+ EXPECT_FALSE(or_expr1->Equals(*or_expr4));
+
+ // Test inequality with different operation types
+ auto and_expr = std::make_shared<And>(true_expr, false_expr);
+ EXPECT_FALSE(or_expr1->Equals(*and_expr));
+}
+
+TEST(ANDTest, Negation) {
+ // Test De Morgan's law: not(A and B) = (not A) or (not B)
+ auto true_expr = True::Instance();
+ auto false_expr = False::Instance();
+
+ auto and_expr = std::make_shared<And>(true_expr, false_expr);
+ auto negated_and = and_expr->Negate();
+
+ // Should become OR expression
+ EXPECT_EQ(negated_and->op(), Expression::Operation::kOr);
+ EXPECT_EQ(negated_and->ToString(), "(false or true)");
+}
+
+TEST(ANDTest, Equals) {
+ auto true_expr = True::Instance();
+ auto false_expr = False::Instance();
+
+ // Test basic equality
+ auto and_expr1 = std::make_shared<And>(true_expr, false_expr);
+ auto and_expr2 = std::make_shared<And>(true_expr, false_expr);
+ EXPECT_TRUE(and_expr1->Equals(*and_expr2));
+
+ // Test commutativity: (A and B) equals (B and A)
+ auto and_expr3 = std::make_shared<And>(false_expr, true_expr);
+ EXPECT_TRUE(and_expr1->Equals(*and_expr3));
+
+ // Test inequality with different expressions
+ auto and_expr4 = std::make_shared<And>(true_expr, true_expr);
+ EXPECT_FALSE(and_expr1->Equals(*and_expr4));
+
+ // Test inequality with different operation types
+ auto or_expr = std::make_shared<Or>(true_expr, false_expr);
+ EXPECT_FALSE(and_expr1->Equals(*or_expr));
+}
+
+TEST(ExpressionTest, BaseClassNegateThrowsException) {
+ // Create a mock expression that doesn't override Negate()
+ class MockExpression : public Expression {
+ public:
+ Operation op() const override { return Operation::kTrue; }
+ // Deliberately not overriding Negate() to test base class behavior
+ };
+
+ auto mock_expr = std::make_shared<MockExpression>();
+
+ // Should throw IcebergError when calling Negate() on base class
+ EXPECT_THROW(mock_expr->Negate(), IcebergError);
+}
} // namespace iceberg
