Skip to content

Commit f2a79a1

Browse files
yingcai-cyyingcai-cy
authored
feat: add or expression (#120)
- or expression - change negate return type form Result to Expression - add some tests
1 parent 5e26ef6 commit f2a79a1

File tree

3 files changed

+174
-26
lines changed

3 files changed

+174
-26
lines changed

‎src/iceberg/expression/expression.cc‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121

2222
#include <format>
2323

24-
#include "iceberg/result.h"
25-
2624
namespace iceberg {
2725

2826
// True implementation
@@ -31,15 +29,15 @@ const std::shared_ptr<True>& True::Instance() {
3129
return instance;
3230
}
3331

34-
Result<std::shared_ptr<Expression>> True::Negate() const { return False::Instance(); }
32+
std::shared_ptr<Expression> True::Negate() const { return False::Instance(); }
3533

3634
// False implementation
3735
const std::shared_ptr<False>& False::Instance() {
3836
static const std::shared_ptr<False> instance = std::shared_ptr<False>(new False());
3937
return instance;
4038
}
4139

42-
Result<std::shared_ptr<Expression>> False::Negate() const { return True::Instance(); }
40+
std::shared_ptr<Expression> False::Negate() const { return True::Instance(); }
4341

4442
// And implementation
4543
And::And(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right)
@@ -49,9 +47,11 @@ std::string And::ToString() const {
4947
return std::format("({} and {})", left_->ToString(), right_->ToString());
5048
}
5149

52-
Result<std::shared_ptr<Expression>> And::Negate() const {
53-
// TODO(yingcai-cy): Implement Or expression
54-
return InvalidExpression("And negation not yet implemented");
50+
std::shared_ptr<Expression> And::Negate() const {
51+
// De Morgan's law: not(A and B) = (not A) or (not B)
52+
auto left_negated = left_->Negate();
53+
auto right_negated = right_->Negate();
54+
return std::make_shared<Or>(left_negated, right_negated);
5555
}
5656

5757
bool And::Equals(const Expression& expr) const {
@@ -63,4 +63,28 @@ bool And::Equals(const Expression& expr) const {
6363
return false;
6464
}
6565

66+
// Or implementation
67+
Or::Or(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right)
68+
: left_(std::move(left)), right_(std::move(right)) {}
69+
70+
std::string Or::ToString() const {
71+
return std::format("({} or {})", left_->ToString(), right_->ToString());
72+
}
73+
74+
std::shared_ptr<Expression> Or::Negate() const {
75+
// De Morgan's law: not(A or B) = (not A) and (not B)
76+
auto left_negated = left_->Negate();
77+
auto right_negated = right_->Negate();
78+
return std::make_shared<And>(left_negated, right_negated);
79+
}
80+
81+
bool Or::Equals(const Expression& expr) const {
82+
if (expr.op() == Operation::kOr) {
83+
const auto& other = static_cast<const Or&>(expr);
84+
return (left_->Equals(*other.left()) && right_->Equals(*other.right())) ||
85+
(left_->Equals(*other.right()) && right_->Equals(*other.left()));
86+
}
87+
return false;
88+
}
89+
6690
} // namespace iceberg

‎src/iceberg/expression/expression.h‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@
2525
#include <memory>
2626
#include <string>
2727

28-
#include "iceberg/expected.h"
28+
#include "iceberg/exception.h"
2929
#include "iceberg/iceberg_export.h"
30-
#include "iceberg/result.h"
3130

3231
namespace iceberg {
3332

@@ -67,8 +66,8 @@ class ICEBERG_EXPORT Expression {
6766
virtual Operation op() const = 0;
6867

6968
/// \brief Returns the negation of this expression, equivalent to not(this).
70-
virtual Result<std::shared_ptr<Expression>> Negate() const {
71-
return InvalidExpression("Expression cannot be negated");
69+
virtual std::shared_ptr<Expression> Negate() const {
70+
throw IcebergError("Expression cannot be negated");
7271
}
7372

7473
/// \brief Returns whether this expression will accept the same values as another.
@@ -94,7 +93,7 @@ class ICEBERG_EXPORT True : public Expression {
9493

9594
std::string ToString() const override { return "true"; }
9695

97-
Result<std::shared_ptr<Expression>> Negate() const override;
96+
std::shared_ptr<Expression> Negate() const override;
9897

9998
bool Equals(const Expression& other) const override {
10099
return other.op() == Operation::kTrue;
@@ -114,7 +113,7 @@ class ICEBERG_EXPORT False : public Expression {
114113

115114
std::string ToString() const override { return "false"; }
116115

117-
Result<std::shared_ptr<Expression>> Negate() const override;
116+
std::shared_ptr<Expression> Negate() const override;
118117

119118
bool Equals(const Expression& other) const override {
120119
return other.op() == Operation::kFalse;
@@ -150,7 +149,42 @@ class ICEBERG_EXPORT And : public Expression {
150149

151150
std::string ToString() const override;
152151

153-
Result<std::shared_ptr<Expression>> Negate() const override;
152+
std::shared_ptr<Expression> Negate() const override;
153+
154+
bool Equals(const Expression& other) const override;
155+
156+
private:
157+
std::shared_ptr<Expression> left_;
158+
std::shared_ptr<Expression> right_;
159+
};
160+
161+
/// \brief An Expression that represents a logical OR operation between two expressions.
162+
///
163+
/// This expression evaluates to true if at least one of its child expressions
164+
/// evaluates to true.
165+
class ICEBERG_EXPORT Or : public Expression {
166+
public:
167+
/// \brief Constructs an Or expression from two sub-expressions.
168+
///
169+
/// \param left The left operand of the OR expression
170+
/// \param right The right operand of the OR expression
171+
Or(std::shared_ptr<Expression> left, std::shared_ptr<Expression> right);
172+
173+
/// \brief Returns the left operand of the OR expression.
174+
///
175+
/// \return The left operand of the OR expression
176+
const std::shared_ptr<Expression>& left() const { return left_; }
177+
178+
/// \brief Returns the right operand of the OR expression.
179+
///
180+
/// \return The right operand of the OR expression
181+
const std::shared_ptr<Expression>& right() const { return right_; }
182+
183+
Operation op() const override { return Operation::kOr; }
184+
185+
std::string ToString() const override;
186+
187+
std::shared_ptr<Expression> Negate() const override;
154188

155189
bool Equals(const Expression& other) const override;
156190

‎test/expression_test.cc‎

Lines changed: 102 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,17 @@ TEST(TrueFalseTest, Basic) {
3030
auto false_instance = False::Instance();
3131
auto negated = false_instance->Negate();
3232

33-
EXPECT_TRUE(negated.has_value());
34-
3533
// Check that negated expression is True
36-
auto true_expr = negated.value();
37-
EXPECT_EQ(true_expr->op(), Expression::Operation::kTrue);
38-
39-
EXPECT_EQ(true_expr->ToString(), "true");
34+
EXPECT_EQ(negated->op(), Expression::Operation::kTrue);
35+
EXPECT_EQ(negated->ToString(), "true");
4036

4137
// Test negation of True returns false
4238
auto true_instance = True::Instance();
4339
negated = true_instance->Negate();
4440

45-
EXPECT_TRUE(negated.has_value());
46-
4741
// Check that negated expression is False
48-
auto false_expr = negated.value();
49-
EXPECT_EQ(false_expr->op(), Expression::Operation::kFalse);
50-
51-
EXPECT_EQ(false_expr->ToString(), "false");
42+
EXPECT_EQ(negated->op(), Expression::Operation::kFalse);
43+
EXPECT_EQ(negated->ToString(), "false");
5244
}
5345

5446
TEST(ANDTest, Basic) {
@@ -64,4 +56,102 @@ TEST(ANDTest, Basic) {
6456
EXPECT_EQ(and_expr->left()->op(), Expression::Operation::kTrue);
6557
EXPECT_EQ(and_expr->right()->op(), Expression::Operation::kTrue);
6658
}
59+
60+
TEST(ORTest, Basic) {
61+
// Create True and False expressions
62+
auto true_expr = True::Instance();
63+
auto false_expr = False::Instance();
64+
65+
// Create an OR expression
66+
auto or_expr = std::make_shared<Or>(true_expr, false_expr);
67+
68+
EXPECT_EQ(or_expr->op(), Expression::Operation::kOr);
69+
EXPECT_EQ(or_expr->ToString(), "(true or false)");
70+
EXPECT_EQ(or_expr->left()->op(), Expression::Operation::kTrue);
71+
EXPECT_EQ(or_expr->right()->op(), Expression::Operation::kFalse);
72+
}
73+
74+
TEST(ORTest, Negation) {
75+
// Test De Morgan's law: not(A or B) = (not A) and (not B)
76+
auto true_expr = True::Instance();
77+
auto false_expr = False::Instance();
78+
79+
auto or_expr = std::make_shared<Or>(true_expr, false_expr);
80+
auto negated_or = or_expr->Negate();
81+
82+
// Should become AND expression
83+
EXPECT_EQ(negated_or->op(), Expression::Operation::kAnd);
84+
EXPECT_EQ(negated_or->ToString(), "(false and true)");
85+
}
86+
87+
TEST(ORTest, Equals) {
88+
auto true_expr = True::Instance();
89+
auto false_expr = False::Instance();
90+
91+
// Test basic equality
92+
auto or_expr1 = std::make_shared<Or>(true_expr, false_expr);
93+
auto or_expr2 = std::make_shared<Or>(true_expr, false_expr);
94+
EXPECT_TRUE(or_expr1->Equals(*or_expr2));
95+
96+
// Test commutativity: (A or B) equals (B or A)
97+
auto or_expr3 = std::make_shared<Or>(false_expr, true_expr);
98+
EXPECT_TRUE(or_expr1->Equals(*or_expr3));
99+
100+
// Test inequality with different expressions
101+
auto or_expr4 = std::make_shared<Or>(true_expr, true_expr);
102+
EXPECT_FALSE(or_expr1->Equals(*or_expr4));
103+
104+
// Test inequality with different operation types
105+
auto and_expr = std::make_shared<And>(true_expr, false_expr);
106+
EXPECT_FALSE(or_expr1->Equals(*and_expr));
107+
}
108+
109+
TEST(ANDTest, Negation) {
110+
// Test De Morgan's law: not(A and B) = (not A) or (not B)
111+
auto true_expr = True::Instance();
112+
auto false_expr = False::Instance();
113+
114+
auto and_expr = std::make_shared<And>(true_expr, false_expr);
115+
auto negated_and = and_expr->Negate();
116+
117+
// Should become OR expression
118+
EXPECT_EQ(negated_and->op(), Expression::Operation::kOr);
119+
EXPECT_EQ(negated_and->ToString(), "(false or true)");
120+
}
121+
122+
TEST(ANDTest, Equals) {
123+
auto true_expr = True::Instance();
124+
auto false_expr = False::Instance();
125+
126+
// Test basic equality
127+
auto and_expr1 = std::make_shared<And>(true_expr, false_expr);
128+
auto and_expr2 = std::make_shared<And>(true_expr, false_expr);
129+
EXPECT_TRUE(and_expr1->Equals(*and_expr2));
130+
131+
// Test commutativity: (A and B) equals (B and A)
132+
auto and_expr3 = std::make_shared<And>(false_expr, true_expr);
133+
EXPECT_TRUE(and_expr1->Equals(*and_expr3));
134+
135+
// Test inequality with different expressions
136+
auto and_expr4 = std::make_shared<And>(true_expr, true_expr);
137+
EXPECT_FALSE(and_expr1->Equals(*and_expr4));
138+
139+
// Test inequality with different operation types
140+
auto or_expr = std::make_shared<Or>(true_expr, false_expr);
141+
EXPECT_FALSE(and_expr1->Equals(*or_expr));
142+
}
143+
144+
TEST(ExpressionTest, BaseClassNegateThrowsException) {
145+
// Create a mock expression that doesn't override Negate()
146+
class MockExpression : public Expression {
147+
public:
148+
Operation op() const override { return Operation::kTrue; }
149+
// Deliberately not overriding Negate() to test base class behavior
150+
};
151+
152+
auto mock_expr = std::make_shared<MockExpression>();
153+
154+
// Should throw IcebergError when calling Negate() on base class
155+
EXPECT_THROW(mock_expr->Negate(), IcebergError);
156+
}
67157
} // namespace iceberg

0 commit comments

Comments
 (0)