From 68298edd119c90f1cd9f53c8aa6de4d5381b5eae Mon Sep 17 00:00:00 2001 From: yingcai-cy Date: Wed, 2 Apr 2025 11:42:05 +0800 Subject: [PATCH 1/5] feat:add init expression interface. --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/error.h | 2 + src/iceberg/expression.cc | 129 ++++++++++++++++++++++++++++++ src/iceberg/expression.h | 101 ++++++++++++++++++++++++ test/CMakeLists.txt | 5 ++ test/expression_test.cc | 157 +++++++++++++++++++++++++++++++++++++ 6 files changed, 395 insertions(+) create mode 100644 src/iceberg/expression.cc create mode 100644 src/iceberg/expression.h create mode 100644 test/expression_test.cc diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index fec895240..abe78ce64 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -20,6 +20,7 @@ set(ICEBERG_INCLUDES "$" set(ICEBERG_SOURCES arrow_c_data_internal.cc demo.cc + expression.cc schema.cc schema_field.cc schema_internal.cc diff --git a/src/iceberg/error.h b/src/iceberg/error.h index a4b74a97c..165cfa9e6 100644 --- a/src/iceberg/error.h +++ b/src/iceberg/error.h @@ -38,6 +38,8 @@ enum class ErrorKind { kNotImplemented, kUnknownError, kNotSupported, + kInvalidExpression, + kInvalidOperatorType, }; /// \brief Error with a kind and a message. diff --git a/src/iceberg/expression.cc b/src/iceberg/expression.cc new file mode 100644 index 000000000..523e3940b --- /dev/null +++ b/src/iceberg/expression.cc @@ -0,0 +1,129 @@ +/* + * 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/expression.h" + +#include + +namespace iceberg { + +expected Expression::FromString( + const std::string& operation_type) { + std::string lowercase_op = operation_type; + std::transform(lowercase_op.begin(), lowercase_op.end(), lowercase_op.begin(), + [](unsigned char c) { return std::tolower(c); }); + + static const std::unordered_map op_map = { + {"true", Operation::kTrue}, + {"false", Operation::kFalse}, + {"is_null", Operation::kIsNull}, + {"not_null", Operation::kNotNull}, + {"is_nan", Operation::kIsNan}, + {"not_nan", Operation::kNotNan}, + {"lt", Operation::kLt}, + {"lt_eq", Operation::kLtEq}, + {"gt", Operation::kGt}, + {"gt_eq", Operation::kGtEq}, + {"eq", Operation::kEq}, + {"not_eq", Operation::kNotEq}, + {"in", Operation::kIn}, + {"not_in", Operation::kNotIn}, + {"not", Operation::kNot}, + {"and", Operation::kAnd}, + {"or", Operation::kOr}, + {"starts_with", Operation::kStartsWith}, + {"not_starts_with", Operation::kNotStartsWith}, + {"count", Operation::kCount}, + {"count_star", Operation::kCountStar}, + {"max", Operation::kMax}, + {"min", Operation::kMin}}; + + auto it = op_map.find(lowercase_op); + if (it == op_map.end()) { + return unexpected( + {ErrorKind::kInvalidOperatorType, "Unknown operation type: " + operation_type}); + } + return it->second; +} + +expected Expression::Negate(Operation op) { + switch (op) { + case Operation::kTrue: + return Operation::kFalse; + case Operation::kFalse: + return Operation::kTrue; + case Operation::kIsNull: + return Operation::kNotNull; + case Operation::kNotNull: + return Operation::kIsNull; + case Operation::kIsNan: + return Operation::kNotNan; + case Operation::kNotNan: + return Operation::kIsNan; + case Operation::kLt: + return Operation::kGtEq; + case Operation::kLtEq: + return Operation::kGt; + case Operation::kGt: + return Operation::kLtEq; + case Operation::kGtEq: + return Operation::kLt; + case Operation::kEq: + return Operation::kNotEq; + case Operation::kNotEq: + return Operation::kEq; + case Operation::kIn: + return Operation::kNotIn; + case Operation::kNotIn: + return Operation::kIn; + case Operation::kStartsWith: + return Operation::kNotStartsWith; + case Operation::kNotStartsWith: + return Operation::kStartsWith; + default: + return unexpected( + {ErrorKind::kInvalidOperatorType, "No negation defined for operation"}); + } +} + +expected Expression::FlipLR(Operation op) { + switch (op) { + case Operation::kLt: + return Operation::kGt; + case Operation::kLtEq: + return Operation::kGtEq; + case Operation::kGt: + return Operation::kLt; + case Operation::kGtEq: + return Operation::kLtEq; + case Operation::kEq: + return Operation::kEq; + case Operation::kNotEq: + return Operation::kNotEq; + case Operation::kAnd: + return Operation::kAnd; + case Operation::kOr: + return Operation::kOr; + default: + return unexpected( + {ErrorKind::kInvalidOperatorType, "No left-right flip for operation"}); + } +} + +} // namespace iceberg \ No newline at end of file diff --git a/src/iceberg/expression.h b/src/iceberg/expression.h new file mode 100644 index 000000000..6366e8d50 --- /dev/null +++ b/src/iceberg/expression.h @@ -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. + */ + +#pragma once + +/// \file iceberg/expression.h +/// Boolean expression tree for Iceberg table operations. + +#include + +#include "iceberg/error.h" +#include "iceberg/expected.h" +#include "iceberg/iceberg_export.h" + +namespace iceberg { + +/// \brief Represents a boolean expression tree. +class ICEBERG_EXPORT Expression { + public: + /// Operation types for expressions + enum class Operation { + kTrue, + kFalse, + kIsNull, + kNotNull, + kIsNan, + kNotNan, + kLt, + kLtEq, + kGt, + kGtEq, + kEq, + kNotEq, + kIn, + kNotIn, + kNot, + kAnd, + kOr, + kStartsWith, + kNotStartsWith, + kCount, + kCountStar, + kMax, + kMin + }; + + virtual ~Expression() = default; + + /// \brief Returns the operation for an expression node. + [[nodiscard]] virtual Operation Op() const = 0; + + /// \brief Returns the negation of this expression, equivalent to not(this). + [[nodiscard]] virtual expected, Error> Negate() const { + return unexpected( + {ErrorKind::kInvalidExpression, "This expression cannot be negated"}); + } + + /// \brief Returns whether this expression will accept the same values as another. + /// + /// If this returns true, the expressions are guaranteed to return the same evaluation + /// for the same input. However, if this returns false the expressions may return the + /// same evaluation for the same input. That is, expressions may be equivalent even if + /// this returns false. + /// + /// For best results, rewrite not and bind expressions before calling this method. + /// + /// \param other another expression + /// \return true if the expressions are equivalent + [[nodiscard]] virtual bool IsEquivalentTo(const Expression& other) const { + // only bound predicates can be equivalent + return false; + } + + /// \brief Convert operation string to enum + static expected FromString(const std::string& operation_type); + + /// \brief Returns the operation used when this is negated. + static expected Negate(Operation op); + + /// \brief Returns the equivalent operation when the left and right operands are + /// exchanged. + static expected FlipLR(Operation op); +}; + +} // namespace iceberg diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 96e319445..845b37cd9 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -39,6 +39,11 @@ target_sources(expected_test PRIVATE expected_test.cc) target_link_libraries(expected_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) add_test(NAME expected_test COMMAND expected_test) +add_executable(expression_test) +target_sources(expression_test PRIVATE expression_test.cc) +target_link_libraries(expression_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) +add_test(NAME expression_test COMMAND expression_test) + if(ICEBERG_BUILD_BUNDLE) add_executable(avro_test) target_sources(avro_test PRIVATE avro_test.cc) diff --git a/test/expression_test.cc b/test/expression_test.cc new file mode 100644 index 000000000..f94cc8c2e --- /dev/null +++ b/test/expression_test.cc @@ -0,0 +1,157 @@ +/* + * 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/expression.h" + +#include + +#include + +using iceberg::Error; +using iceberg::ErrorKind; +using iceberg::Expression; + +// A concrete implementation of Expression for testing +class TestExpression : public Expression { + public: + explicit TestExpression(Operation op) : operation_(op) {} + + Operation Op() const override { return operation_; } + + iceberg::expected, Error> Negate() const override { + auto negated_op = Expression::Negate(operation_); + if (!negated_op) { + return iceberg::unexpected(negated_op.error()); + } + return std::make_shared(negated_op.value()); + } + + bool IsEquivalentTo(const Expression& other) const override { + if (auto* test_expr = dynamic_cast(&other)) { + return operation_ == test_expr->operation_; + } + return false; + } + + private: + Operation operation_; +}; + +TEST(ExpressionTest, FromString) { + // Test valid operations + auto true_op = Expression::FromString("true"); + ASSERT_TRUE(true_op); + EXPECT_EQ(true_op.value(), Expression::Operation::kTrue); + + auto false_op = Expression::FromString("false"); + ASSERT_TRUE(false_op); + EXPECT_EQ(false_op.value(), Expression::Operation::kFalse); + + auto lt_op = Expression::FromString("lt"); + ASSERT_TRUE(lt_op); + EXPECT_EQ(lt_op.value(), Expression::Operation::kLt); + + // Test case insensitivity + auto and_op = Expression::FromString("AND"); + ASSERT_TRUE(and_op); + EXPECT_EQ(and_op.value(), Expression::Operation::kAnd); + + // Test invalid operation + auto invalid_op = Expression::FromString("invalid_operation"); + ASSERT_FALSE(invalid_op); + EXPECT_EQ(invalid_op.error().kind, ErrorKind::kInvalidOperatorType); + EXPECT_EQ(invalid_op.error().message, "Unknown operation type: invalid_operation"); +} + +TEST(ExpressionTest, NegateOperation) { + // Test valid negations + auto true_negated = Expression::Negate(Expression::Operation::kTrue); + ASSERT_TRUE(true_negated); + EXPECT_EQ(true_negated.value(), Expression::Operation::kFalse); + + auto lt_negated = Expression::Negate(Expression::Operation::kLt); + ASSERT_TRUE(lt_negated); + EXPECT_EQ(lt_negated.value(), Expression::Operation::kGtEq); + + auto is_null_negated = Expression::Negate(Expression::Operation::kIsNull); + ASSERT_TRUE(is_null_negated); + EXPECT_EQ(is_null_negated.value(), Expression::Operation::kNotNull); + + // Test invalid negation + auto not_negated = Expression::Negate(Expression::Operation::kNot); + ASSERT_FALSE(not_negated); + EXPECT_EQ(not_negated.error().kind, ErrorKind::kInvalidOperatorType); + EXPECT_EQ(not_negated.error().message, "No negation defined for operation"); +} + +TEST(ExpressionTest, FlipLR) { + // Test valid flips + auto lt_flipped = Expression::FlipLR(Expression::Operation::kLt); + ASSERT_TRUE(lt_flipped); + EXPECT_EQ(lt_flipped.value(), Expression::Operation::kGt); + + auto eq_flipped = Expression::FlipLR(Expression::Operation::kEq); + ASSERT_TRUE(eq_flipped); + EXPECT_EQ(eq_flipped.value(), Expression::Operation::kEq); + + // Test invalid flip + auto in_flipped = Expression::FlipLR(Expression::Operation::kIn); + ASSERT_FALSE(in_flipped); + EXPECT_EQ(in_flipped.error().kind, ErrorKind::kInvalidOperatorType); + EXPECT_EQ(in_flipped.error().message, "No left-right flip for operation"); +} + +TEST(ExpressionTest, TestExpressionNegate) { + // Test negatable expression + auto expr = std::make_shared(Expression::Operation::kLt); + auto negated = expr->Negate(); + ASSERT_TRUE(negated); + EXPECT_EQ(negated.value()->Op(), Expression::Operation::kGtEq); + + // Test equality between original and double-negated + auto double_negated = negated.value()->Negate(); + ASSERT_TRUE(double_negated); + EXPECT_TRUE(expr->IsEquivalentTo(*double_negated.value())); + + // Test non-negatable expression + auto non_negatable = std::make_shared(Expression::Operation::kNot); + auto negated_result = non_negatable->Negate(); + ASSERT_FALSE(negated_result); + EXPECT_EQ(negated_result.error().kind, ErrorKind::kInvalidOperatorType); + EXPECT_EQ(negated_result.error().message, "No negation defined for operation"); +} + +TEST(ExpressionTest, IsEquivalentTo) { + auto expr1 = std::make_shared(Expression::Operation::kEq); + auto expr2 = std::make_shared(Expression::Operation::kEq); + auto expr3 = std::make_shared(Expression::Operation::kNotEq); + + // Same operation should be equivalent + EXPECT_TRUE(expr1->IsEquivalentTo(*expr2)); + + // Different operations should not be equivalent + EXPECT_FALSE(expr1->IsEquivalentTo(*expr3)); + + // Test double negation equivalence + auto negated = expr1->Negate(); + ASSERT_TRUE(negated); + auto double_negated = negated.value()->Negate(); + ASSERT_TRUE(double_negated); + EXPECT_TRUE(expr1->IsEquivalentTo(*double_negated.value())); +} \ No newline at end of file From bcb7506cd6ab1d92661f1b743b6063aab06b9760 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Fri, 11 Apr 2025 15:29:17 +0800 Subject: [PATCH 2/5] feat: expression interface apply review suggestions. --- src/iceberg/CMakeLists.txt | 6 +- src/iceberg/expression.cc | 129 -------------------------- src/iceberg/expression.h | 34 ++----- src/iceberg/expressions/and.cc | 38 ++++++++ src/iceberg/expressions/and.h | 68 ++++++++++++++ src/iceberg/expressions/false.cc | 30 ++++++ src/iceberg/expressions/false.h | 55 +++++++++++ src/iceberg/expressions/true.cc | 30 ++++++ src/iceberg/expressions/true.h | 59 ++++++++++++ test/expression_test.cc | 152 +++++++------------------------ 10 files changed, 324 insertions(+), 277 deletions(-) delete mode 100644 src/iceberg/expression.cc create mode 100644 src/iceberg/expressions/and.cc create mode 100644 src/iceberg/expressions/and.h create mode 100644 src/iceberg/expressions/false.cc create mode 100644 src/iceberg/expressions/false.h create mode 100644 src/iceberg/expressions/true.cc create mode 100644 src/iceberg/expressions/true.h diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 810908cfe..d29f8ca62 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -20,7 +20,6 @@ set(ICEBERG_INCLUDES "$" set(ICEBERG_SOURCES arrow_c_data_internal.cc demo.cc - expression.cc json_internal.cc schema.cc schema_field.cc @@ -32,7 +31,10 @@ set(ICEBERG_SOURCES statistics_file.cc table_metadata.cc transform.cc - type.cc) + type.cc + expressions/true.cc + expressions/false.cc + expressions/and.cc) set(ICEBERG_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/expression.cc b/src/iceberg/expression.cc deleted file mode 100644 index 523e3940b..000000000 --- a/src/iceberg/expression.cc +++ /dev/null @@ -1,129 +0,0 @@ -/* - * 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/expression.h" - -#include - -namespace iceberg { - -expected Expression::FromString( - const std::string& operation_type) { - std::string lowercase_op = operation_type; - std::transform(lowercase_op.begin(), lowercase_op.end(), lowercase_op.begin(), - [](unsigned char c) { return std::tolower(c); }); - - static const std::unordered_map op_map = { - {"true", Operation::kTrue}, - {"false", Operation::kFalse}, - {"is_null", Operation::kIsNull}, - {"not_null", Operation::kNotNull}, - {"is_nan", Operation::kIsNan}, - {"not_nan", Operation::kNotNan}, - {"lt", Operation::kLt}, - {"lt_eq", Operation::kLtEq}, - {"gt", Operation::kGt}, - {"gt_eq", Operation::kGtEq}, - {"eq", Operation::kEq}, - {"not_eq", Operation::kNotEq}, - {"in", Operation::kIn}, - {"not_in", Operation::kNotIn}, - {"not", Operation::kNot}, - {"and", Operation::kAnd}, - {"or", Operation::kOr}, - {"starts_with", Operation::kStartsWith}, - {"not_starts_with", Operation::kNotStartsWith}, - {"count", Operation::kCount}, - {"count_star", Operation::kCountStar}, - {"max", Operation::kMax}, - {"min", Operation::kMin}}; - - auto it = op_map.find(lowercase_op); - if (it == op_map.end()) { - return unexpected( - {ErrorKind::kInvalidOperatorType, "Unknown operation type: " + operation_type}); - } - return it->second; -} - -expected Expression::Negate(Operation op) { - switch (op) { - case Operation::kTrue: - return Operation::kFalse; - case Operation::kFalse: - return Operation::kTrue; - case Operation::kIsNull: - return Operation::kNotNull; - case Operation::kNotNull: - return Operation::kIsNull; - case Operation::kIsNan: - return Operation::kNotNan; - case Operation::kNotNan: - return Operation::kIsNan; - case Operation::kLt: - return Operation::kGtEq; - case Operation::kLtEq: - return Operation::kGt; - case Operation::kGt: - return Operation::kLtEq; - case Operation::kGtEq: - return Operation::kLt; - case Operation::kEq: - return Operation::kNotEq; - case Operation::kNotEq: - return Operation::kEq; - case Operation::kIn: - return Operation::kNotIn; - case Operation::kNotIn: - return Operation::kIn; - case Operation::kStartsWith: - return Operation::kNotStartsWith; - case Operation::kNotStartsWith: - return Operation::kStartsWith; - default: - return unexpected( - {ErrorKind::kInvalidOperatorType, "No negation defined for operation"}); - } -} - -expected Expression::FlipLR(Operation op) { - switch (op) { - case Operation::kLt: - return Operation::kGt; - case Operation::kLtEq: - return Operation::kGtEq; - case Operation::kGt: - return Operation::kLt; - case Operation::kGtEq: - return Operation::kLtEq; - case Operation::kEq: - return Operation::kEq; - case Operation::kNotEq: - return Operation::kNotEq; - case Operation::kAnd: - return Operation::kAnd; - case Operation::kOr: - return Operation::kOr; - default: - return unexpected( - {ErrorKind::kInvalidOperatorType, "No left-right flip for operation"}); - } -} - -} // namespace iceberg \ No newline at end of file diff --git a/src/iceberg/expression.h b/src/iceberg/expression.h index 6366e8d50..0a6b8f3ef 100644 --- a/src/iceberg/expression.h +++ b/src/iceberg/expression.h @@ -20,13 +20,11 @@ #pragma once /// \file iceberg/expression.h -/// Boolean expression tree for Iceberg table operations. +/// Expression interface for Iceberg table operations. -#include - -#include "iceberg/error.h" #include "iceberg/expected.h" #include "iceberg/iceberg_export.h" +#include "iceberg/result.h" namespace iceberg { @@ -63,39 +61,23 @@ class ICEBERG_EXPORT Expression { virtual ~Expression() = default; /// \brief Returns the operation for an expression node. - [[nodiscard]] virtual Operation Op() const = 0; + virtual Operation Op() const = 0; /// \brief Returns the negation of this expression, equivalent to not(this). - [[nodiscard]] virtual expected, Error> Negate() const { - return unexpected( - {ErrorKind::kInvalidExpression, "This expression cannot be negated"}); + virtual Result> Negate() const { + return unexpected( + Error(ErrorKind::kInvalidExpression, "Expression cannot be negated")); } /// \brief Returns whether this expression will accept the same values as another. - /// - /// If this returns true, the expressions are guaranteed to return the same evaluation - /// for the same input. However, if this returns false the expressions may return the - /// same evaluation for the same input. That is, expressions may be equivalent even if - /// this returns false. - /// - /// For best results, rewrite not and bind expressions before calling this method. - /// /// \param other another expression /// \return true if the expressions are equivalent - [[nodiscard]] virtual bool IsEquivalentTo(const Expression& other) const { + virtual bool IsEquivalentTo(const Expression& other) const { // only bound predicates can be equivalent return false; } - /// \brief Convert operation string to enum - static expected FromString(const std::string& operation_type); - - /// \brief Returns the operation used when this is negated. - static expected Negate(Operation op); - - /// \brief Returns the equivalent operation when the left and right operands are - /// exchanged. - static expected FlipLR(Operation op); + virtual std::string ToString() const { return "Expression"; } }; } // namespace iceberg diff --git a/src/iceberg/expressions/and.cc b/src/iceberg/expressions/and.cc new file mode 100644 index 000000000..a768266c9 --- /dev/null +++ b/src/iceberg/expressions/and.cc @@ -0,0 +1,38 @@ +/* + * 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 "and.h" + +namespace iceberg { + +And::And(std::shared_ptr left, std::shared_ptr right) + : left_(std::move(left)), right_(std::move(right)) {} + +bool And::IsEquivalentTo(const Expression& expr) const { + if (expr.Op() == Operation::kAnd) { + const auto& other = static_cast(expr); + return (left_->IsEquivalentTo(*other.left()) && + right_->IsEquivalentTo(*other.right())) || + (left_->IsEquivalentTo(*other.right()) && + right_->IsEquivalentTo(*other.left())); + } + return false; +} + +} // namespace iceberg diff --git a/src/iceberg/expressions/and.h b/src/iceberg/expressions/and.h new file mode 100644 index 000000000..d4a88488a --- /dev/null +++ b/src/iceberg/expressions/and.h @@ -0,0 +1,68 @@ +/* + * 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 "iceberg/expression.h" + +namespace iceberg { + +/// \brief An Expression that represents a logical AND operation between two expressions. +/// +/// This expression evaluates to true if and only if both of its child expressions +/// evaluate to true. +class ICEBERG_EXPORT And : public Expression { + public: + /// \brief Constructs an And expression from two sub-expressions. + /// + /// @param left The left operand of the AND expression + /// @param right The right operand of the AND expression + And(std::shared_ptr left, std::shared_ptr right); + + /// \brief Returns the left operand of the AND expression. + /// + /// @return The left operand of the AND expression + const std::shared_ptr& left() const { return left_; } + + /// \brief Returns the right operand of the AND expression. + /// + /// @return The right operand of the AND expression + const std::shared_ptr& right() const { return right_; } + + Operation Op() const override { return Operation::kAnd; } + + std::string ToString() const override { + return std::format("({} and {})", left_->ToString(), right_->ToString()); + } + + // implement Negate later + // expected, Error> Negate() const override; + + bool IsEquivalentTo(const Expression& other) const override; + + private: + std::shared_ptr left_; + std::shared_ptr right_; +}; + +} // namespace iceberg diff --git a/src/iceberg/expressions/false.cc b/src/iceberg/expressions/false.cc new file mode 100644 index 000000000..e886e235c --- /dev/null +++ b/src/iceberg/expressions/false.cc @@ -0,0 +1,30 @@ +/* + * 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 "false.h" + +#include "true.h" + +namespace iceberg { + +expected, Error> False::Negate() const { + return True::shared_instance(); +} + +} // namespace iceberg diff --git a/src/iceberg/expressions/false.h b/src/iceberg/expressions/false.h new file mode 100644 index 000000000..eb87a3b61 --- /dev/null +++ b/src/iceberg/expressions/false.h @@ -0,0 +1,55 @@ +/* + * 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 "iceberg/expression.h" + +namespace iceberg { + +/// \brief An expression that is always false. +class ICEBERG_EXPORT False : public Expression { + public: + static const False& instance() { + static False instance; + return instance; + } + + static const std::shared_ptr& shared_instance() { + static std::shared_ptr instance = std::shared_ptr( + const_cast(&False::instance()), [](Expression*) {}); + return instance; + } + + Operation Op() const override { return Operation::kFalse; } + + std::string ToString() const override { return "false"; } + + Result> Negate() const override; + + bool IsEquivalentTo(const Expression& other) const override { + return other.Op() == Operation::kFalse; + } + + private: + constexpr False() = default; +}; +} // namespace iceberg diff --git a/src/iceberg/expressions/true.cc b/src/iceberg/expressions/true.cc new file mode 100644 index 000000000..83b92e8a3 --- /dev/null +++ b/src/iceberg/expressions/true.cc @@ -0,0 +1,30 @@ +/* + * 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 "true.h" + +#include "false.h" + +namespace iceberg { + +Result> True::Negate() const { + return False::shared_instance(); +} + +} // namespace iceberg diff --git a/src/iceberg/expressions/true.h b/src/iceberg/expressions/true.h new file mode 100644 index 000000000..a0be4dc5a --- /dev/null +++ b/src/iceberg/expressions/true.h @@ -0,0 +1,59 @@ +/* + * 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 "iceberg/expression.h" + +namespace iceberg { + +/// \brief An Expression that is always true. +/// +/// Represents a boolean predicate that always evaluates to true. +class ICEBERG_EXPORT True : public Expression { + public: + static const True& instance() { + static True instance; + return instance; + } + + static const std::shared_ptr& shared_instance() { + static std::shared_ptr instance = std::shared_ptr( + const_cast(&True::instance()), [](Expression*) {}); + return instance; + } + + Operation Op() const override { return Operation::kTrue; } + + std::string ToString() const override { return "true"; } + + Result> Negate() const override; + + bool IsEquivalentTo(const Expression& other) const override { + return other.Op() == Operation::kTrue; + } + + private: + constexpr True() = default; +}; + +} // namespace iceberg diff --git a/test/expression_test.cc b/test/expression_test.cc index f94cc8c2e..4cf8c7bf8 100644 --- a/test/expression_test.cc +++ b/test/expression_test.cc @@ -17,141 +17,53 @@ * under the License. */ -#include "iceberg/expression.h" - #include #include -using iceberg::Error; -using iceberg::ErrorKind; -using iceberg::Expression; - -// A concrete implementation of Expression for testing -class TestExpression : public Expression { - public: - explicit TestExpression(Operation op) : operation_(op) {} - - Operation Op() const override { return operation_; } - - iceberg::expected, Error> Negate() const override { - auto negated_op = Expression::Negate(operation_); - if (!negated_op) { - return iceberg::unexpected(negated_op.error()); - } - return std::make_shared(negated_op.value()); - } +#include "iceberg/expressions/and.h" +#include "iceberg/expressions/false.h" +#include "iceberg/expressions/true.h" - bool IsEquivalentTo(const Expression& other) const override { - if (auto* test_expr = dynamic_cast(&other)) { - return operation_ == test_expr->operation_; - } - return false; - } +namespace iceberg { - private: - Operation operation_; -}; +TEST(TrueFalseTest, Basi) { + // Test negation of False returns True + const False& false_instance = False::instance(); + auto negated = false_instance.Negate(); -TEST(ExpressionTest, FromString) { - // Test valid operations - auto true_op = Expression::FromString("true"); - ASSERT_TRUE(true_op); - EXPECT_EQ(true_op.value(), Expression::Operation::kTrue); + EXPECT_TRUE(negated.has_value()); - auto false_op = Expression::FromString("false"); - ASSERT_TRUE(false_op); - EXPECT_EQ(false_op.value(), Expression::Operation::kFalse); + // Check that negated expression is True + std::shared_ptr true_expr = negated.value(); + EXPECT_EQ(true_expr->Op(), Expression::Operation::kTrue); - auto lt_op = Expression::FromString("lt"); - ASSERT_TRUE(lt_op); - EXPECT_EQ(lt_op.value(), Expression::Operation::kLt); + EXPECT_EQ(true_expr->ToString(), "true"); - // Test case insensitivity - auto and_op = Expression::FromString("AND"); - ASSERT_TRUE(and_op); - EXPECT_EQ(and_op.value(), Expression::Operation::kAnd); + // Test negation of True returns false + const True& true_instance = True::instance(); + negated = true_instance.Negate(); - // Test invalid operation - auto invalid_op = Expression::FromString("invalid_operation"); - ASSERT_FALSE(invalid_op); - EXPECT_EQ(invalid_op.error().kind, ErrorKind::kInvalidOperatorType); - EXPECT_EQ(invalid_op.error().message, "Unknown operation type: invalid_operation"); -} - -TEST(ExpressionTest, NegateOperation) { - // Test valid negations - auto true_negated = Expression::Negate(Expression::Operation::kTrue); - ASSERT_TRUE(true_negated); - EXPECT_EQ(true_negated.value(), Expression::Operation::kFalse); + EXPECT_TRUE(negated.has_value()); - auto lt_negated = Expression::Negate(Expression::Operation::kLt); - ASSERT_TRUE(lt_negated); - EXPECT_EQ(lt_negated.value(), Expression::Operation::kGtEq); + // Check that negated expression is True + std::shared_ptr false_expr = negated.value(); + EXPECT_EQ(false_expr->Op(), Expression::Operation::kFalse); - auto is_null_negated = Expression::Negate(Expression::Operation::kIsNull); - ASSERT_TRUE(is_null_negated); - EXPECT_EQ(is_null_negated.value(), Expression::Operation::kNotNull); - - // Test invalid negation - auto not_negated = Expression::Negate(Expression::Operation::kNot); - ASSERT_FALSE(not_negated); - EXPECT_EQ(not_negated.error().kind, ErrorKind::kInvalidOperatorType); - EXPECT_EQ(not_negated.error().message, "No negation defined for operation"); + EXPECT_EQ(false_expr->ToString(), "false"); } -TEST(ExpressionTest, FlipLR) { - // Test valid flips - auto lt_flipped = Expression::FlipLR(Expression::Operation::kLt); - ASSERT_TRUE(lt_flipped); - EXPECT_EQ(lt_flipped.value(), Expression::Operation::kGt); +TEST(ANDTest, Basic) { + // Create two True expressions + auto true_expr1 = True::shared_instance(); + auto true_expr2 = True::shared_instance(); - auto eq_flipped = Expression::FlipLR(Expression::Operation::kEq); - ASSERT_TRUE(eq_flipped); - EXPECT_EQ(eq_flipped.value(), Expression::Operation::kEq); + // Create an AND expression + auto and_expr = std::make_shared(true_expr1, true_expr2); - // Test invalid flip - auto in_flipped = Expression::FlipLR(Expression::Operation::kIn); - ASSERT_FALSE(in_flipped); - EXPECT_EQ(in_flipped.error().kind, ErrorKind::kInvalidOperatorType); - EXPECT_EQ(in_flipped.error().message, "No left-right flip for operation"); + EXPECT_EQ(and_expr->Op(), Expression::Operation::kAnd); + EXPECT_EQ(and_expr->ToString(), "(true and true)"); + EXPECT_EQ(and_expr->left()->Op(), Expression::Operation::kTrue); + EXPECT_EQ(and_expr->right()->Op(), Expression::Operation::kTrue); } - -TEST(ExpressionTest, TestExpressionNegate) { - // Test negatable expression - auto expr = std::make_shared(Expression::Operation::kLt); - auto negated = expr->Negate(); - ASSERT_TRUE(negated); - EXPECT_EQ(negated.value()->Op(), Expression::Operation::kGtEq); - - // Test equality between original and double-negated - auto double_negated = negated.value()->Negate(); - ASSERT_TRUE(double_negated); - EXPECT_TRUE(expr->IsEquivalentTo(*double_negated.value())); - - // Test non-negatable expression - auto non_negatable = std::make_shared(Expression::Operation::kNot); - auto negated_result = non_negatable->Negate(); - ASSERT_FALSE(negated_result); - EXPECT_EQ(negated_result.error().kind, ErrorKind::kInvalidOperatorType); - EXPECT_EQ(negated_result.error().message, "No negation defined for operation"); -} - -TEST(ExpressionTest, IsEquivalentTo) { - auto expr1 = std::make_shared(Expression::Operation::kEq); - auto expr2 = std::make_shared(Expression::Operation::kEq); - auto expr3 = std::make_shared(Expression::Operation::kNotEq); - - // Same operation should be equivalent - EXPECT_TRUE(expr1->IsEquivalentTo(*expr2)); - - // Different operations should not be equivalent - EXPECT_FALSE(expr1->IsEquivalentTo(*expr3)); - - // Test double negation equivalence - auto negated = expr1->Negate(); - ASSERT_TRUE(negated); - auto double_negated = negated.value()->Negate(); - ASSERT_TRUE(double_negated); - EXPECT_TRUE(expr1->IsEquivalentTo(*double_negated.value())); -} \ No newline at end of file +} // namespace iceberg From 48be4c9571c7e0035e005592beb9282f9cb833bd Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Sat, 12 Apr 2025 14:59:50 +0800 Subject: [PATCH 3/5] feat:expressin apply review comments. --- src/iceberg/CMakeLists.txt | 4 +- src/iceberg/expression.cc | 65 +++++++++++++++++++++++ src/iceberg/expression.h | 90 ++++++++++++++++++++++++++++++-- src/iceberg/expressions/and.cc | 38 -------------- src/iceberg/expressions/and.h | 68 ------------------------ src/iceberg/expressions/false.cc | 30 ----------- src/iceberg/expressions/false.h | 55 ------------------- src/iceberg/expressions/true.cc | 30 ----------- src/iceberg/expressions/true.h | 59 --------------------- src/iceberg/result.h | 1 - test/expression_test.cc | 28 +++++----- 11 files changed, 166 insertions(+), 302 deletions(-) create mode 100644 src/iceberg/expression.cc delete mode 100644 src/iceberg/expressions/and.cc delete mode 100644 src/iceberg/expressions/and.h delete mode 100644 src/iceberg/expressions/false.cc delete mode 100644 src/iceberg/expressions/false.h delete mode 100644 src/iceberg/expressions/true.cc delete mode 100644 src/iceberg/expressions/true.h diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index d29f8ca62..2b802b367 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -32,9 +32,7 @@ set(ICEBERG_SOURCES table_metadata.cc transform.cc type.cc - expressions/true.cc - expressions/false.cc - expressions/and.cc) + expression.cc) set(ICEBERG_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/expression.cc b/src/iceberg/expression.cc new file mode 100644 index 000000000..a8b8d0472 --- /dev/null +++ b/src/iceberg/expression.cc @@ -0,0 +1,65 @@ +/* + * 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 "expression.h" + +#include + +namespace iceberg { + +// True implementation +const std::shared_ptr& True::Instance() { + static const std::shared_ptr instance = std::shared_ptr(new True()); + return instance; +} + +Result True::Negate() const { return False::Instance(); } + +// False implementation +const std::shared_ptr& False::Instance() { + static const std::shared_ptr instance = std::shared_ptr(new False()); + return instance; +} + +Result False::Negate() const { return True::Instance(); } + +// And implementation +And::And(ExpressionPtr left, ExpressionPtr right) + : left_(std::move(left)), right_(std::move(right)) {} + +std::string And::ToString() const { + return std::format("({} and {})", left_->ToString(), right_->ToString()); +} + +Result And::Negate() const { + // TODO(yingcai-cy): Implement Or expression + return unexpected( + Error(ErrorKind::kInvalidExpression, "And negation not yet implemented")); +} + +bool And::Equals(const Expression& expr) const { + if (expr.op() == Operation::kAnd) { + const auto& other = static_cast(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.h b/src/iceberg/expression.h index 0a6b8f3ef..a8824552b 100644 --- a/src/iceberg/expression.h +++ b/src/iceberg/expression.h @@ -22,12 +22,19 @@ /// \file iceberg/expression.h /// Expression interface for Iceberg table operations. +#include +#include + #include "iceberg/expected.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" namespace iceberg { +/// \brief Type alias for shared pointer to Expression +class Expression; +using ExpressionPtr = std::shared_ptr; + /// \brief Represents a boolean expression tree. class ICEBERG_EXPORT Expression { public: @@ -61,10 +68,10 @@ class ICEBERG_EXPORT Expression { virtual ~Expression() = default; /// \brief Returns the operation for an expression node. - virtual Operation Op() const = 0; + virtual Operation op() const = 0; /// \brief Returns the negation of this expression, equivalent to not(this). - virtual Result> Negate() const { + virtual Result Negate() const { return unexpected( Error(ErrorKind::kInvalidExpression, "Expression cannot be negated")); } @@ -72,7 +79,7 @@ class ICEBERG_EXPORT Expression { /// \brief Returns whether this expression will accept the same values as another. /// \param other another expression /// \return true if the expressions are equivalent - virtual bool IsEquivalentTo(const Expression& other) const { + virtual bool Equals(const Expression& other) const { // only bound predicates can be equivalent return false; } @@ -80,4 +87,81 @@ class ICEBERG_EXPORT Expression { virtual std::string ToString() const { return "Expression"; } }; +/// \brief An Expression that is always true. +/// +/// Represents a boolean predicate that always evaluates to true. +class ICEBERG_EXPORT True : public Expression { + public: + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Operation op() const override { return Operation::kTrue; } + + std::string ToString() const override { return "true"; } + + Result Negate() const override; + + bool Equals(const Expression& other) const override { + return other.op() == Operation::kTrue; + } + + private: + constexpr True() = default; +}; + +/// \brief An expression that is always false. +class ICEBERG_EXPORT False : public Expression { + public: + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Operation op() const override { return Operation::kFalse; } + + std::string ToString() const override { return "false"; } + + Result Negate() const override; + + bool Equals(const Expression& other) const override { + return other.op() == Operation::kFalse; + } + + private: + constexpr False() = default; +}; + +/// \brief An Expression that represents a logical AND operation between two expressions. +/// +/// This expression evaluates to true if and only if both of its child expressions +/// evaluate to true. +class ICEBERG_EXPORT And : public Expression { + public: + /// \brief Constructs an And expression from two sub-expressions. + /// + /// @param left The left operand of the AND expression + /// @param right The right operand of the AND expression + And(ExpressionPtr left, ExpressionPtr right); + + /// \brief Returns the left operand of the AND expression. + /// + /// @return The left operand of the AND expression + const ExpressionPtr& left() const { return left_; } + + /// \brief Returns the right operand of the AND expression. + /// + /// @return The right operand of the AND expression + const ExpressionPtr& right() const { return right_; } + + Operation op() const override { return Operation::kAnd; } + + std::string ToString() const override; + + Result Negate() const override; + + bool Equals(const Expression& other) const override; + + private: + ExpressionPtr left_; + ExpressionPtr right_; +}; + } // namespace iceberg diff --git a/src/iceberg/expressions/and.cc b/src/iceberg/expressions/and.cc deleted file mode 100644 index a768266c9..000000000 --- a/src/iceberg/expressions/and.cc +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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 "and.h" - -namespace iceberg { - -And::And(std::shared_ptr left, std::shared_ptr right) - : left_(std::move(left)), right_(std::move(right)) {} - -bool And::IsEquivalentTo(const Expression& expr) const { - if (expr.Op() == Operation::kAnd) { - const auto& other = static_cast(expr); - return (left_->IsEquivalentTo(*other.left()) && - right_->IsEquivalentTo(*other.right())) || - (left_->IsEquivalentTo(*other.right()) && - right_->IsEquivalentTo(*other.left())); - } - return false; -} - -} // namespace iceberg diff --git a/src/iceberg/expressions/and.h b/src/iceberg/expressions/and.h deleted file mode 100644 index d4a88488a..000000000 --- a/src/iceberg/expressions/and.h +++ /dev/null @@ -1,68 +0,0 @@ -/* - * 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 "iceberg/expression.h" - -namespace iceberg { - -/// \brief An Expression that represents a logical AND operation between two expressions. -/// -/// This expression evaluates to true if and only if both of its child expressions -/// evaluate to true. -class ICEBERG_EXPORT And : public Expression { - public: - /// \brief Constructs an And expression from two sub-expressions. - /// - /// @param left The left operand of the AND expression - /// @param right The right operand of the AND expression - And(std::shared_ptr left, std::shared_ptr right); - - /// \brief Returns the left operand of the AND expression. - /// - /// @return The left operand of the AND expression - const std::shared_ptr& left() const { return left_; } - - /// \brief Returns the right operand of the AND expression. - /// - /// @return The right operand of the AND expression - const std::shared_ptr& right() const { return right_; } - - Operation Op() const override { return Operation::kAnd; } - - std::string ToString() const override { - return std::format("({} and {})", left_->ToString(), right_->ToString()); - } - - // implement Negate later - // expected, Error> Negate() const override; - - bool IsEquivalentTo(const Expression& other) const override; - - private: - std::shared_ptr left_; - std::shared_ptr right_; -}; - -} // namespace iceberg diff --git a/src/iceberg/expressions/false.cc b/src/iceberg/expressions/false.cc deleted file mode 100644 index e886e235c..000000000 --- a/src/iceberg/expressions/false.cc +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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 "false.h" - -#include "true.h" - -namespace iceberg { - -expected, Error> False::Negate() const { - return True::shared_instance(); -} - -} // namespace iceberg diff --git a/src/iceberg/expressions/false.h b/src/iceberg/expressions/false.h deleted file mode 100644 index eb87a3b61..000000000 --- a/src/iceberg/expressions/false.h +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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 "iceberg/expression.h" - -namespace iceberg { - -/// \brief An expression that is always false. -class ICEBERG_EXPORT False : public Expression { - public: - static const False& instance() { - static False instance; - return instance; - } - - static const std::shared_ptr& shared_instance() { - static std::shared_ptr instance = std::shared_ptr( - const_cast(&False::instance()), [](Expression*) {}); - return instance; - } - - Operation Op() const override { return Operation::kFalse; } - - std::string ToString() const override { return "false"; } - - Result> Negate() const override; - - bool IsEquivalentTo(const Expression& other) const override { - return other.Op() == Operation::kFalse; - } - - private: - constexpr False() = default; -}; -} // namespace iceberg diff --git a/src/iceberg/expressions/true.cc b/src/iceberg/expressions/true.cc deleted file mode 100644 index 83b92e8a3..000000000 --- a/src/iceberg/expressions/true.cc +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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 "true.h" - -#include "false.h" - -namespace iceberg { - -Result> True::Negate() const { - return False::shared_instance(); -} - -} // namespace iceberg diff --git a/src/iceberg/expressions/true.h b/src/iceberg/expressions/true.h deleted file mode 100644 index a0be4dc5a..000000000 --- a/src/iceberg/expressions/true.h +++ /dev/null @@ -1,59 +0,0 @@ -/* - * 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 "iceberg/expression.h" - -namespace iceberg { - -/// \brief An Expression that is always true. -/// -/// Represents a boolean predicate that always evaluates to true. -class ICEBERG_EXPORT True : public Expression { - public: - static const True& instance() { - static True instance; - return instance; - } - - static const std::shared_ptr& shared_instance() { - static std::shared_ptr instance = std::shared_ptr( - const_cast(&True::instance()), [](Expression*) {}); - return instance; - } - - Operation Op() const override { return Operation::kTrue; } - - std::string ToString() const override { return "true"; } - - Result> Negate() const override; - - bool IsEquivalentTo(const Expression& other) const override { - return other.Op() == Operation::kTrue; - } - - private: - constexpr True() = default; -}; - -} // namespace iceberg diff --git a/src/iceberg/result.h b/src/iceberg/result.h index 34c167fe4..a70383d43 100644 --- a/src/iceberg/result.h +++ b/src/iceberg/result.h @@ -40,7 +40,6 @@ enum class ErrorKind { kUnknownError, kNotSupported, kInvalidExpression, - kInvalidOperatorType, kJsonParseError, }; diff --git a/test/expression_test.cc b/test/expression_test.cc index 4cf8c7bf8..a29a58b6b 100644 --- a/test/expression_test.cc +++ b/test/expression_test.cc @@ -17,53 +17,51 @@ * under the License. */ +#include "iceberg/expression.h" + #include #include -#include "iceberg/expressions/and.h" -#include "iceberg/expressions/false.h" -#include "iceberg/expressions/true.h" - namespace iceberg { TEST(TrueFalseTest, Basi) { // Test negation of False returns True - const False& false_instance = False::instance(); - auto negated = false_instance.Negate(); + const std::shared_ptr false_instance = False::Instance(); + auto negated = false_instance->Negate(); EXPECT_TRUE(negated.has_value()); // Check that negated expression is True std::shared_ptr true_expr = negated.value(); - EXPECT_EQ(true_expr->Op(), Expression::Operation::kTrue); + EXPECT_EQ(true_expr->op(), Expression::Operation::kTrue); EXPECT_EQ(true_expr->ToString(), "true"); // Test negation of True returns false - const True& true_instance = True::instance(); - negated = true_instance.Negate(); + const std::shared_ptr true_instance = True::Instance(); + negated = true_instance->Negate(); EXPECT_TRUE(negated.has_value()); // Check that negated expression is True std::shared_ptr false_expr = negated.value(); - EXPECT_EQ(false_expr->Op(), Expression::Operation::kFalse); + EXPECT_EQ(false_expr->op(), Expression::Operation::kFalse); EXPECT_EQ(false_expr->ToString(), "false"); } TEST(ANDTest, Basic) { // Create two True expressions - auto true_expr1 = True::shared_instance(); - auto true_expr2 = True::shared_instance(); + auto true_expr1 = True::Instance(); + auto true_expr2 = True::Instance(); // Create an AND expression auto and_expr = std::make_shared(true_expr1, true_expr2); - EXPECT_EQ(and_expr->Op(), Expression::Operation::kAnd); + EXPECT_EQ(and_expr->op(), Expression::Operation::kAnd); EXPECT_EQ(and_expr->ToString(), "(true and true)"); - EXPECT_EQ(and_expr->left()->Op(), Expression::Operation::kTrue); - EXPECT_EQ(and_expr->right()->Op(), Expression::Operation::kTrue); + EXPECT_EQ(and_expr->left()->op(), Expression::Operation::kTrue); + EXPECT_EQ(and_expr->right()->op(), Expression::Operation::kTrue); } } // namespace iceberg From afd3935d9c75ee191a0197755cd74d35855f965d Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Tue, 15 Apr 2025 17:10:01 +0800 Subject: [PATCH 4/5] feat: expression interface init commit, apply review suggestions. --- src/iceberg/expression.cc | 10 +++++----- src/iceberg/expression.h | 30 +++++++++++++----------------- test/expression_test.cc | 12 ++++++------ 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/iceberg/expression.cc b/src/iceberg/expression.cc index a8b8d0472..d53cef3cc 100644 --- a/src/iceberg/expression.cc +++ b/src/iceberg/expression.cc @@ -25,11 +25,11 @@ namespace iceberg { // True implementation const std::shared_ptr& True::Instance() { - static const std::shared_ptr instance = std::shared_ptr(new True()); + static const std::shared_ptr instance{new True()}; return instance; } -Result True::Negate() const { return False::Instance(); } +Result> True::Negate() const { return False::Instance(); } // False implementation const std::shared_ptr& False::Instance() { @@ -37,17 +37,17 @@ const std::shared_ptr& False::Instance() { return instance; } -Result False::Negate() const { return True::Instance(); } +Result> False::Negate() const { return True::Instance(); } // And implementation -And::And(ExpressionPtr left, ExpressionPtr right) +And::And(std::shared_ptr left, std::shared_ptr right) : left_(std::move(left)), right_(std::move(right)) {} std::string And::ToString() const { return std::format("({} and {})", left_->ToString(), right_->ToString()); } -Result And::Negate() const { +Result> And::Negate() const { // TODO(yingcai-cy): Implement Or expression return unexpected( Error(ErrorKind::kInvalidExpression, "And negation not yet implemented")); diff --git a/src/iceberg/expression.h b/src/iceberg/expression.h index a8824552b..3113fb8bd 100644 --- a/src/iceberg/expression.h +++ b/src/iceberg/expression.h @@ -31,10 +31,6 @@ namespace iceberg { -/// \brief Type alias for shared pointer to Expression -class Expression; -using ExpressionPtr = std::shared_ptr; - /// \brief Represents a boolean expression tree. class ICEBERG_EXPORT Expression { public: @@ -71,7 +67,7 @@ class ICEBERG_EXPORT Expression { virtual Operation op() const = 0; /// \brief Returns the negation of this expression, equivalent to not(this). - virtual Result Negate() const { + virtual Result> Negate() const { return unexpected( Error(ErrorKind::kInvalidExpression, "Expression cannot be negated")); } @@ -99,7 +95,7 @@ class ICEBERG_EXPORT True : public Expression { std::string ToString() const override { return "true"; } - Result Negate() const override; + Result> Negate() const override; bool Equals(const Expression& other) const override { return other.op() == Operation::kTrue; @@ -119,7 +115,7 @@ class ICEBERG_EXPORT False : public Expression { std::string ToString() const override { return "false"; } - Result Negate() const override; + Result> Negate() const override; bool Equals(const Expression& other) const override { return other.op() == Operation::kFalse; @@ -137,31 +133,31 @@ class ICEBERG_EXPORT And : public Expression { public: /// \brief Constructs an And expression from two sub-expressions. /// - /// @param left The left operand of the AND expression - /// @param right The right operand of the AND expression - And(ExpressionPtr left, ExpressionPtr right); + /// \param left The left operand of the AND expression + /// \param right The right operand of the AND expression + And(std::shared_ptr left, std::shared_ptr right); /// \brief Returns the left operand of the AND expression. /// - /// @return The left operand of the AND expression - const ExpressionPtr& left() const { return left_; } + /// \return The left operand of the AND expression + const std::shared_ptr& left() const { return left_; } /// \brief Returns the right operand of the AND expression. /// - /// @return The right operand of the AND expression - const ExpressionPtr& right() const { return right_; } + /// \return The right operand of the AND expression + const std::shared_ptr& right() const { return right_; } Operation op() const override { return Operation::kAnd; } std::string ToString() const override; - Result Negate() const override; + Result> Negate() const override; bool Equals(const Expression& other) const override; private: - ExpressionPtr left_; - ExpressionPtr right_; + std::shared_ptr left_; + std::shared_ptr right_; }; } // namespace iceberg diff --git a/test/expression_test.cc b/test/expression_test.cc index a29a58b6b..1ca50cb5f 100644 --- a/test/expression_test.cc +++ b/test/expression_test.cc @@ -25,27 +25,27 @@ namespace iceberg { -TEST(TrueFalseTest, Basi) { +TEST(TrueFalseTest, Basic) { // Test negation of False returns True - const std::shared_ptr false_instance = False::Instance(); + auto false_instance = False::Instance(); auto negated = false_instance->Negate(); EXPECT_TRUE(negated.has_value()); // Check that negated expression is True - std::shared_ptr true_expr = negated.value(); + auto true_expr = negated.value(); EXPECT_EQ(true_expr->op(), Expression::Operation::kTrue); EXPECT_EQ(true_expr->ToString(), "true"); // Test negation of True returns false - const std::shared_ptr true_instance = True::Instance(); + auto true_instance = True::Instance(); negated = true_instance->Negate(); EXPECT_TRUE(negated.has_value()); - // Check that negated expression is True - std::shared_ptr false_expr = negated.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"); From b3f4a43339cd64883efc1c086156848fd1e6e0a7 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Wed, 16 Apr 2025 11:04:51 +0800 Subject: [PATCH 5/5] Fix cmake file format. --- test/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 41329e4f8..b9dc6c2a7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -46,7 +46,8 @@ add_test(NAME expected_test COMMAND expected_test) add_executable(expression_test) target_sources(expression_test PRIVATE expression_test.cc) -target_link_libraries(expression_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) +target_link_libraries(expression_test PRIVATE iceberg_static GTest::gtest_main + GTest::gmock) add_test(NAME expression_test COMMAND expression_test) if(ICEBERG_BUILD_BUNDLE)