diff --git a/src/iceberg/expression/expressions.cc b/src/iceberg/expression/expressions.cc index 6839157f9..7e3db186f 100644 --- a/src/iceberg/expression/expressions.cc +++ b/src/iceberg/expression/expressions.cc @@ -104,8 +104,10 @@ std::shared_ptr> Expressions::IsNull(std::strin template std::shared_ptr> Expressions::IsNull( std::shared_ptr> expr) { - return std::make_shared>(Expression::Operation::kIsNull, - std::move(expr)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, + UnboundPredicate::Make(Expression::Operation::kIsNull, std::move(expr))); + return pred; } std::shared_ptr> Expressions::NotNull(std::string name) { @@ -115,8 +117,10 @@ std::shared_ptr> Expressions::NotNull(std::stri template std::shared_ptr> Expressions::NotNull( std::shared_ptr> expr) { - return std::make_shared>(Expression::Operation::kNotNull, - std::move(expr)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, + UnboundPredicate::Make(Expression::Operation::kNotNull, std::move(expr))); + return pred; } std::shared_ptr> Expressions::IsNaN(std::string name) { @@ -126,8 +130,9 @@ std::shared_ptr> Expressions::IsNaN(std::string template std::shared_ptr> Expressions::IsNaN( std::shared_ptr> expr) { - return std::make_shared>(Expression::Operation::kIsNan, - std::move(expr)); + ICEBERG_ASSIGN_OR_THROW(auto pred, UnboundPredicate::Make( + Expression::Operation::kIsNan, std::move(expr))); + return pred; } std::shared_ptr> Expressions::NotNaN(std::string name) { @@ -137,8 +142,10 @@ std::shared_ptr> Expressions::NotNaN(std::strin template std::shared_ptr> Expressions::NotNaN( std::shared_ptr> expr) { - return std::make_shared>(Expression::Operation::kNotNan, - std::move(expr)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, + UnboundPredicate::Make(Expression::Operation::kNotNan, std::move(expr))); + return pred; } // Template implementations for comparison predicates @@ -151,8 +158,10 @@ std::shared_ptr> Expressions::LessThan(std::str template std::shared_ptr> Expressions::LessThan( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kLt, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kLt, std::move(expr), + std::move(value))); + return pred; } std::shared_ptr> Expressions::LessThanOrEqual( @@ -163,8 +172,10 @@ std::shared_ptr> Expressions::LessThanOrEqual( template std::shared_ptr> Expressions::LessThanOrEqual( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kLtEq, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kLtEq, std::move(expr), + std::move(value))); + return pred; } std::shared_ptr> Expressions::GreaterThan( @@ -175,8 +186,10 @@ std::shared_ptr> Expressions::GreaterThan( template std::shared_ptr> Expressions::GreaterThan( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kGt, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kGt, std::move(expr), + std::move(value))); + return pred; } std::shared_ptr> Expressions::GreaterThanOrEqual( @@ -187,8 +200,10 @@ std::shared_ptr> Expressions::GreaterThanOrEqua template std::shared_ptr> Expressions::GreaterThanOrEqual( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kGtEq, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kGtEq, std::move(expr), + std::move(value))); + return pred; } std::shared_ptr> Expressions::Equal(std::string name, @@ -199,8 +214,10 @@ std::shared_ptr> Expressions::Equal(std::string template std::shared_ptr> Expressions::Equal( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kEq, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kEq, std::move(expr), + std::move(value))); + return pred; } std::shared_ptr> Expressions::NotEqual(std::string name, @@ -211,8 +228,10 @@ std::shared_ptr> Expressions::NotEqual(std::str template std::shared_ptr> Expressions::NotEqual( std::shared_ptr> expr, Literal value) { - return std::make_shared>(Expression::Operation::kNotEq, - std::move(expr), std::move(value)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kNotEq, std::move(expr), + std::move(value))); + return pred; } // String predicates @@ -225,9 +244,11 @@ std::shared_ptr> Expressions::StartsWith( template std::shared_ptr> Expressions::StartsWith( std::shared_ptr> expr, std::string value) { - return std::make_shared>(Expression::Operation::kStartsWith, - std::move(expr), - Literal::String(std::move(value))); + ICEBERG_ASSIGN_OR_THROW( + auto pred, + UnboundPredicate::Make(Expression::Operation::kStartsWith, std::move(expr), + Literal::String(std::move(value)))); + return pred; } std::shared_ptr> Expressions::NotStartsWith( @@ -238,9 +259,11 @@ std::shared_ptr> Expressions::NotStartsWith( template std::shared_ptr> Expressions::NotStartsWith( std::shared_ptr> expr, std::string value) { - return std::make_shared>(Expression::Operation::kNotStartsWith, - std::move(expr), - Literal::String(std::move(value))); + ICEBERG_ASSIGN_OR_THROW( + auto pred, + UnboundPredicate::Make(Expression::Operation::kNotStartsWith, std::move(expr), + Literal::String(std::move(value)))); + return pred; } // Template implementations for set predicates @@ -253,8 +276,10 @@ std::shared_ptr> Expressions::In( template std::shared_ptr> Expressions::In(std::shared_ptr> expr, std::vector values) { - return std::make_shared>(Expression::Operation::kIn, - std::move(expr), std::move(values)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kIn, std::move(expr), + std::move(values))); + return pred; } std::shared_ptr> Expressions::In( @@ -276,8 +301,10 @@ std::shared_ptr> Expressions::NotIn( template std::shared_ptr> Expressions::NotIn( std::shared_ptr> expr, std::vector values) { - return std::make_shared>(Expression::Operation::kNotIn, - std::move(expr), std::move(values)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(Expression::Operation::kNotIn, std::move(expr), + std::move(values))); + return pred; } std::shared_ptr> Expressions::NotIn( @@ -295,14 +322,16 @@ std::shared_ptr> Expressions::NotIn( std::shared_ptr> Expressions::Predicate( Expression::Operation op, std::string name, Literal value) { - return std::make_shared>(op, Ref(std::move(name)), - std::move(value)); + ICEBERG_ASSIGN_OR_THROW(auto pred, UnboundPredicate::Make( + op, Ref(std::move(name)), std::move(value))); + return pred; } std::shared_ptr> Expressions::Predicate( Expression::Operation op, std::string name, std::vector values) { - return std::make_shared>(op, Ref(std::move(name)), - std::move(values)); + ICEBERG_ASSIGN_OR_THROW(auto pred, UnboundPredicate::Make( + op, Ref(std::move(name)), std::move(values))); + return pred; } std::shared_ptr> Expressions::Predicate( @@ -312,14 +341,18 @@ std::shared_ptr> Expressions::Predicate( std::shared_ptr> Expressions::Predicate( Expression::Operation op, std::string name) { - return std::make_shared>(op, Ref(std::move(name))); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(op, Ref(std::move(name)))); + return pred; } template std::shared_ptr> Expressions::Predicate( Expression::Operation op, std::shared_ptr> expr, std::vector values) { - return std::make_shared>(op, std::move(expr), std::move(values)); + ICEBERG_ASSIGN_OR_THROW( + auto pred, UnboundPredicate::Make(op, std::move(expr), std::move(values))); + return pred; } template @@ -332,7 +365,8 @@ std::shared_ptr> Expressions::Predicate( template std::shared_ptr> Expressions::Predicate( Expression::Operation op, std::shared_ptr> expr) { - return std::make_shared>(op, std::move(expr)); + ICEBERG_ASSIGN_OR_THROW(auto pred, UnboundPredicate::Make(op, std::move(expr))); + return pred; } // Constants diff --git a/src/iceberg/expression/predicate.cc b/src/iceberg/expression/predicate.cc index 959443c04..c255d7584 100644 --- a/src/iceberg/expression/predicate.cc +++ b/src/iceberg/expression/predicate.cc @@ -36,27 +36,66 @@ namespace iceberg { // Predicate template implementations template Predicate::Predicate(Expression::Operation op, std::shared_ptr term) - : operation_(op), term_(std::move(term)) {} + : operation_(op), term_(std::move(term)) { + ICEBERG_DCHECK(term_ != nullptr, "Predicate cannot have null term"); +} template Predicate::~Predicate() = default; // UnboundPredicate template implementations +template +Result>> UnboundPredicate::Make( + Expression::Operation op, std::shared_ptr> term) { + if (!term) [[unlikely]] { + return InvalidExpression("UnboundPredicate cannot have null term"); + } + return std::unique_ptr>( + new UnboundPredicate(op, std::move(term))); +} + +template +Result>> UnboundPredicate::Make( + Expression::Operation op, std::shared_ptr> term, Literal value) { + if (!term) [[unlikely]] { + return InvalidExpression("UnboundPredicate cannot have null term"); + } + return std::unique_ptr>( + new UnboundPredicate(op, std::move(term), std::move(value))); +} + +template +Result>> UnboundPredicate::Make( + Expression::Operation op, std::shared_ptr> term, + std::vector values) { + if (!term) [[unlikely]] { + return InvalidExpression("UnboundPredicate cannot have null term"); + } + return std::unique_ptr>( + new UnboundPredicate(op, std::move(term), std::move(values))); +} + template UnboundPredicate::UnboundPredicate(Expression::Operation op, std::shared_ptr> term) - : BASE(op, std::move(term)) {} + : BASE(op, std::move(term)) { + ICEBERG_DCHECK(BASE::term() != nullptr, "UnboundPredicate cannot have null term"); +} template UnboundPredicate::UnboundPredicate(Expression::Operation op, std::shared_ptr> term, Literal value) - : BASE(op, std::move(term)), values_{std::move(value)} {} + : BASE(op, std::move(term)), values_{std::move(value)} { + ICEBERG_DCHECK(BASE::term() != nullptr, "UnboundPredicate cannot have null term"); +} template UnboundPredicate::UnboundPredicate(Expression::Operation op, std::shared_ptr> term, std::vector values) - : BASE(op, std::move(term)), values_(std::move(values)) {} + : BASE(op, std::move(term)), values_(std::move(values)) { + ICEBERG_DCHECK(BASE::term() != nullptr, "UnboundPredicate cannot have null term"); +} template UnboundPredicate::~UnboundPredicate() = default; @@ -117,7 +156,7 @@ std::string UnboundPredicate::ToString() const { template Result> UnboundPredicate::Negate() const { ICEBERG_ASSIGN_OR_RAISE(auto negated_op, ::iceberg::Negate(BASE::op())); - return std::make_shared(negated_op, BASE::term(), values_); + return UnboundPredicate::Make(negated_op, BASE::term(), values_); } template @@ -174,21 +213,22 @@ Result> UnboundPredicate::BindUnaryOperation( return Expressions::AlwaysFalse(); } // TODO(gangwu): deal with UnknownType - return std::make_shared(Expression::Operation::kIsNull, - std::move(bound_term)); + return BoundUnaryPredicate::Make(Expression::Operation::kIsNull, + std::move(bound_term)); case Expression::Operation::kNotNull: if (!bound_term->MayProduceNull()) { return Expressions::AlwaysTrue(); } - return std::make_shared(Expression::Operation::kNotNull, - std::move(bound_term)); + return BoundUnaryPredicate::Make(Expression::Operation::kNotNull, + std::move(bound_term)); case Expression::Operation::kIsNan: case Expression::Operation::kNotNan: if (!IsFloatingType(bound_term->type()->type_id())) { return InvalidExpression("{} cannot be used with a non-floating-point column", BASE::op()); } - return std::make_shared(BASE::op(), std::move(bound_term)); + return BoundUnaryPredicate::Make(BASE::op(), std::move(bound_term)); + default: return InvalidExpression("Operation must be IS_NULL, NOT_NULL, IS_NAN, or NOT_NAN"); } @@ -247,8 +287,8 @@ Result> UnboundPredicate::BindLiteralOperation( } // TODO(gangwu): translate truncate(col) == value to startsWith(value) - return std::make_shared(BASE::op(), std::move(bound_term), - std::move(literal)); + return BoundLiteralPredicate::Make(BASE::op(), std::move(bound_term), + std::move(literal)); } template @@ -286,24 +326,28 @@ Result> UnboundPredicate::BindInOperation( const auto& single_literal = converted_literals[0]; switch (BASE::op()) { case Expression::Operation::kIn: - return std::make_shared( - Expression::Operation::kEq, std::move(bound_term), single_literal); + return BoundLiteralPredicate::Make(Expression::Operation::kEq, + std::move(bound_term), single_literal); + case Expression::Operation::kNotIn: - return std::make_shared( - Expression::Operation::kNotEq, std::move(bound_term), single_literal); + return BoundLiteralPredicate::Make(Expression::Operation::kNotEq, + std::move(bound_term), single_literal); + default: return InvalidExpression("Operation must be IN or NOT_IN"); } } // Multiple literals - create a set predicate - return std::make_shared( - BASE::op(), std::move(bound_term), std::span(converted_literals)); + return BoundSetPredicate::Make(BASE::op(), std::move(bound_term), + std::span(converted_literals)); } // BoundPredicate implementation BoundPredicate::BoundPredicate(Expression::Operation op, std::shared_ptr term) - : Predicate(op, std::move(term)) {} + : Predicate(op, std::move(term)) { + ICEBERG_DCHECK(term_ != nullptr, "BoundPredicate cannot have null term"); +} BoundPredicate::~BoundPredicate() = default; @@ -314,9 +358,20 @@ Result BoundPredicate::Evaluate(const StructLike& data) const { } // BoundUnaryPredicate implementation +Result> BoundUnaryPredicate::Make( + Expression::Operation op, std::shared_ptr term) { + if (!term) [[unlikely]] { + return InvalidExpression("BoundUnaryPredicate cannot have null term"); + } + return std::unique_ptr( + new BoundUnaryPredicate(op, std::move(term))); +} + BoundUnaryPredicate::BoundUnaryPredicate(Expression::Operation op, std::shared_ptr term) - : BoundPredicate(op, std::move(term)) {} + : BoundPredicate(op, std::move(term)) { + ICEBERG_DCHECK(term_ != nullptr, "BoundUnaryPredicate cannot have null term"); +} BoundUnaryPredicate::~BoundUnaryPredicate() = default; @@ -337,7 +392,7 @@ Result BoundUnaryPredicate::Test(const Literal& literal) const { Result> BoundUnaryPredicate::Negate() const { ICEBERG_ASSIGN_OR_RAISE(auto negated_op, ::iceberg::Negate(op())); - return std::make_shared(negated_op, term_); + return BoundUnaryPredicate::Make(negated_op, term_); } bool BoundUnaryPredicate::Equals(const Expression& other) const { @@ -369,10 +424,21 @@ std::string BoundUnaryPredicate::ToString() const { } // BoundLiteralPredicate implementation +Result> BoundLiteralPredicate::Make( + Expression::Operation op, std::shared_ptr term, Literal literal) { + if (!term) [[unlikely]] { + return InvalidExpression("BoundLiteralPredicate cannot have null term"); + } + return std::unique_ptr( + new BoundLiteralPredicate(op, std::move(term), std::move(literal))); +} + BoundLiteralPredicate::BoundLiteralPredicate(Expression::Operation op, std::shared_ptr term, Literal literal) - : BoundPredicate(op, std::move(term)), literal_(std::move(literal)) {} + : BoundPredicate(op, std::move(term)), literal_(std::move(literal)) { + ICEBERG_DCHECK(term_ != nullptr, "BoundLiteralPredicate cannot have null term"); +} BoundLiteralPredicate::~BoundLiteralPredicate() = default; @@ -401,7 +467,7 @@ Result BoundLiteralPredicate::Test(const Literal& value) const { Result> BoundLiteralPredicate::Negate() const { ICEBERG_ASSIGN_OR_RAISE(auto negated_op, ::iceberg::Negate(op())); - return std::make_shared(negated_op, term_, literal_); + return BoundLiteralPredicate::Make(negated_op, term_, literal_); } bool BoundLiteralPredicate::Equals(const Expression& other) const { @@ -491,15 +557,38 @@ std::string BoundLiteralPredicate::ToString() const { } // BoundSetPredicate implementation +Result> BoundSetPredicate::Make( + Expression::Operation op, std::shared_ptr term, + std::span literals) { + if (!term) [[unlikely]] { + return InvalidExpression("BoundSetPredicate cannot have null term"); + } + return std::unique_ptr( + new BoundSetPredicate(op, std::move(term), literals)); +} + +Result> BoundSetPredicate::Make( + Expression::Operation op, std::shared_ptr term, LiteralSet value_set) { + if (!term) [[unlikely]] { + return InvalidExpression("BoundSetPredicate cannot have null term"); + } + return std::unique_ptr( + new BoundSetPredicate(op, std::move(term), std::move(value_set))); +} + BoundSetPredicate::BoundSetPredicate(Expression::Operation op, std::shared_ptr term, std::span literals) - : BoundPredicate(op, std::move(term)), value_set_(literals.begin(), literals.end()) {} + : BoundPredicate(op, std::move(term)), value_set_(literals.begin(), literals.end()) { + ICEBERG_DCHECK(term_ != nullptr, "BoundSetPredicate cannot have null term"); +} BoundSetPredicate::BoundSetPredicate(Expression::Operation op, std::shared_ptr term, LiteralSet value_set) - : BoundPredicate(op, std::move(term)), value_set_(std::move(value_set)) {} + : BoundPredicate(op, std::move(term)), value_set_(std::move(value_set)) { + ICEBERG_DCHECK(term_ != nullptr, "BoundSetPredicate cannot have null term"); +} BoundSetPredicate::~BoundSetPredicate() = default; @@ -516,7 +605,7 @@ Result BoundSetPredicate::Test(const Literal& value) const { Result> BoundSetPredicate::Negate() const { ICEBERG_ASSIGN_OR_RAISE(auto negated_op, ::iceberg::Negate(op())); - return std::make_shared(negated_op, term_, value_set_); + return BoundSetPredicate::Make(negated_op, term_, value_set_); } bool BoundSetPredicate::Equals(const Expression& other) const { diff --git a/src/iceberg/expression/predicate.h b/src/iceberg/expression/predicate.h index 979db1212..b35de049f 100644 --- a/src/iceberg/expression/predicate.h +++ b/src/iceberg/expression/predicate.h @@ -40,12 +40,6 @@ concept TermType = std::derived_from; template class ICEBERG_EXPORT Predicate : public Expression { public: - /// \brief Create a predicate with an operation and term. - /// - /// \param op The operation this predicate performs - /// \param term The term this predicate tests - Predicate(Expression::Operation op, std::shared_ptr term); - ~Predicate() override; Expression::Operation op() const override { return operation_; } @@ -54,6 +48,12 @@ class ICEBERG_EXPORT Predicate : public Expression { const std::shared_ptr& term() const { return term_; } protected: + /// \brief Create a predicate with an operation and term. + /// + /// \param op The operation this predicate performs + /// \param term The term this predicate tests + Predicate(Expression::Operation op, std::shared_ptr term); + Expression::Operation operation_; std::shared_ptr term_; }; @@ -68,11 +68,32 @@ class ICEBERG_EXPORT UnboundPredicate : public Predicate>, using BASE = Predicate>; public: - UnboundPredicate(Expression::Operation op, std::shared_ptr> term); - UnboundPredicate(Expression::Operation op, std::shared_ptr> term, - Literal value); - UnboundPredicate(Expression::Operation op, std::shared_ptr> term, - std::vector values); + /// \brief Create an unbound predicate (unary operation). + /// + /// \param op The operation (kIsNull, kNotNull, kIsNan, kNotNan) + /// \param term The unbound term + /// \return Result containing the unbound predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr> term); + + /// \brief Create an unbound predicate with a single value. + /// + /// \param op The operation + /// \param term The unbound term + /// \param value The literal value + /// \return Result containing the unbound predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr> term, Literal value); + + /// \brief Create an unbound predicate with multiple values. + /// + /// \param op The operation (typically kIn or kNotIn) + /// \param term The unbound term + /// \param values Vector of literal values + /// \return Result containing the unbound predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr> term, + std::vector values); ~UnboundPredicate() override; @@ -89,6 +110,12 @@ class ICEBERG_EXPORT UnboundPredicate : public Predicate>, Result> Negate() const override; private: + UnboundPredicate(Expression::Operation op, std::shared_ptr> term); + UnboundPredicate(Expression::Operation op, std::shared_ptr> term, + Literal value); + UnboundPredicate(Expression::Operation op, std::shared_ptr> term, + std::vector values); + Result> BindUnaryOperation( std::shared_ptr bound_term) const; Result> BindLiteralOperation( @@ -103,8 +130,6 @@ class ICEBERG_EXPORT UnboundPredicate : public Predicate>, /// \brief Bound predicates contain bound terms and can be evaluated. class ICEBERG_EXPORT BoundPredicate : public Predicate, public Bound { public: - BoundPredicate(Expression::Operation op, std::shared_ptr term); - ~BoundPredicate() override; using Predicate::op; @@ -132,6 +157,9 @@ class ICEBERG_EXPORT BoundPredicate : public Predicate, public Bound /// \brief Returns the kind of this bound predicate. virtual Kind kind() const = 0; + + protected: + BoundPredicate(Expression::Operation op, std::shared_ptr term); }; /// \brief Bound unary predicate (null, not-null, etc.). @@ -141,7 +169,9 @@ class ICEBERG_EXPORT BoundUnaryPredicate : public BoundPredicate { /// /// \param op The unary operation (kIsNull, kNotNull, kIsNan, kNotNan) /// \param term The bound term to test - BoundUnaryPredicate(Expression::Operation op, std::shared_ptr term); + /// \return Result containing the bound unary predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr term); ~BoundUnaryPredicate() override; @@ -154,6 +184,9 @@ class ICEBERG_EXPORT BoundUnaryPredicate : public BoundPredicate { Result> Negate() const override; bool Equals(const Expression& other) const override; + + private: + BoundUnaryPredicate(Expression::Operation op, std::shared_ptr term); }; /// \brief Bound literal predicate (comparison against a single value). @@ -164,8 +197,9 @@ class ICEBERG_EXPORT BoundLiteralPredicate : public BoundPredicate { /// \param op The comparison operation (kLt, kLtEq, kGt, kGtEq, kEq, kNotEq) /// \param term The bound term to compare /// \param literal The literal value to compare against - BoundLiteralPredicate(Expression::Operation op, std::shared_ptr term, - Literal literal); + /// \return Result containing the bound literal predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr term, Literal literal); ~BoundLiteralPredicate() override; @@ -183,6 +217,9 @@ class ICEBERG_EXPORT BoundLiteralPredicate : public BoundPredicate { bool Equals(const Expression& other) const override; private: + BoundLiteralPredicate(Expression::Operation op, std::shared_ptr term, + Literal literal); + Literal literal_; }; @@ -196,12 +233,20 @@ class ICEBERG_EXPORT BoundSetPredicate : public BoundPredicate { /// \param op The set operation (kIn, kNotIn) /// \param term The bound term to test for membership /// \param literals The set of literal values to test against - BoundSetPredicate(Expression::Operation op, std::shared_ptr term, - std::span literals); + /// \return Result containing the bound set predicate or an error + static Result> Make( + Expression::Operation op, std::shared_ptr term, + std::span literals); /// \brief Create a bound set predicate using a set of literals. - BoundSetPredicate(Expression::Operation op, std::shared_ptr term, - LiteralSet value_set); + /// + /// \param op The set operation (kIn, kNotIn) + /// \param term The bound term to test for membership + /// \param value_set The set of literal values to test against + /// \return Result containing the bound set predicate or an error + static Result> Make(Expression::Operation op, + std::shared_ptr term, + LiteralSet value_set); ~BoundSetPredicate() override; @@ -219,6 +264,12 @@ class ICEBERG_EXPORT BoundSetPredicate : public BoundPredicate { bool Equals(const Expression& other) const override; private: + BoundSetPredicate(Expression::Operation op, std::shared_ptr term, + std::span literals); + + BoundSetPredicate(Expression::Operation op, std::shared_ptr term, + LiteralSet value_set); + LiteralSet value_set_; };