HuaHuaY commented on code in PR #403: URL: https://github.com/apache/iceberg-cpp/pull/403#discussion_r2597593308
########## src/iceberg/expression/manifest_evaluator.h: ########## @@ -0,0 +1,84 @@ +/* + * 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/manifest_evaluator.h +/// +/// Evaluates an Expression on a ManifestFile to test whether the file contains +/// matching partitions. +/// +/// For row expressions, evaluation is inclusive: it returns true if a file +/// may match and false if it cannot match. +/// +/// Files are passed to #eval(ManifestFile), which returns true if the manifest may +/// contain data files that match the partition expression. Manifest files may be +/// skipped if and only if the return value of eval is false. +/// + +#include <memory> + +#include "iceberg/expression/expression.h" +#include "iceberg/iceberg_export.h" +#include "iceberg/partition_spec.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Evaluates an Expression against manifest. +/// \note: The evaluator is thread-safe. +class ICEBERG_EXPORT ManifestEvaluator { + public: + /// \brief Make a manifest evaluator for RowFilter + /// + /// \param expr The expression to evaluate + /// \param spec The partition spec + /// \param schema The schema of the table + /// \param case_sensitive Whether field name matching is case-sensitive + static Result<std::unique_ptr<ManifestEvaluator>> MakeRowFilter( + std::shared_ptr<Expression> expr, const std::shared_ptr<PartitionSpec> spec, + const Schema& schema, bool case_sensitive = true); + + /// \brief Make a manifest evaluator for PartitionFilter + /// + /// \param expr The expression to evaluate + /// \param spec The partition spec + /// \param schema The schema of the table + /// \param case_sensitive Whether field name matching is case-sensitive + static Result<std::unique_ptr<ManifestEvaluator>> MakePartitionFilter( + std::shared_ptr<Expression> expr, const std::shared_ptr<PartitionSpec> spec, Review Comment: Why is the signature `const shared_ptr` instead of `shared_ptr` or `const shared_ptr&`? ########## src/iceberg/expression/manifest_evaluator.h: ########## @@ -0,0 +1,84 @@ +/* + * 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/manifest_evaluator.h +/// +/// Evaluates an Expression on a ManifestFile to test whether the file contains +/// matching partitions. +/// +/// For row expressions, evaluation is inclusive: it returns true if a file +/// may match and false if it cannot match. +/// +/// Files are passed to #eval(ManifestFile), which returns true if the manifest may +/// contain data files that match the partition expression. Manifest files may be +/// skipped if and only if the return value of eval is false. +/// + +#include <memory> + +#include "iceberg/expression/expression.h" +#include "iceberg/iceberg_export.h" +#include "iceberg/partition_spec.h" Review Comment: ```suggestion #include "iceberg/iceberg_export.h" ``` We can use forward declaration and move the include into source files. ########## src/iceberg/expression/manifest_evaluator.cc: ########## @@ -0,0 +1,397 @@ +/* + * 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/manifest_evaluator.h" + +#include "iceberg/expression/binder.h" +#include "iceberg/expression/expression_visitor.h" +#include "iceberg/expression/rewrite_not.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/row/struct_like.h" +#include "iceberg/schema.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { +constexpr bool kRowsMightMatch = true; +constexpr bool kRowCannotMatch = false; +constexpr int32_t kInPredicateLimit = 200; +} // namespace + +class ManifestEvalVisitor : public BoundVisitor<bool> { + public: + explicit ManifestEvalVisitor(const ManifestFile& manifest) + : stats_(manifest.partitions) {} + + Result<bool> AlwaysTrue() override { return true; } + + Result<bool> AlwaysFalse() override { return false; } + + Result<bool> Not(bool child_result) override { return !child_result; } + + Result<bool> And(bool left_result, bool right_result) override { + return left_result && right_result; + } + + Result<bool> Or(bool left_result, bool right_result) override { + return left_result || right_result; + } + + Result<bool> IsNull(const std::shared_ptr<Bound>& expr) override { + // no need to check whether the field is required because binding evaluates that case + // if the column has no null values, the expression cannot match + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (!stats_.at(pos).contains_null) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotNull(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (AllValuesAreNull(stats_.at(pos), ref->type()->type_id())) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> IsNaN(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (stats_.at(pos).contains_nan.has_value() && !stats_.at(pos).contains_nan.value()) { + return kRowCannotMatch; + } + if (AllValuesAreNull(stats_.at(pos), ref->type()->type_id())) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotNaN(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + // if containsNaN is true, containsNull is false and lowerBound is null, all values + // are NaN + if (summary.contains_nan.has_value() && summary.contains_nan.value() && + !summary.contains_null && !summary.lower_bound.has_value()) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> Lt(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower >= lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> LtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower > lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> Gt(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper <= lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> GtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper < lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> Eq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value() || !summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null and literal cannot contain null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower > lit) { + return kRowCannotMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper < lit) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + // because the bounds are not necessarily a min or max value, this cannot be answered + // using them. notEq(col, X) with (X, Y) doesn't guarantee that X is a value in col. + return kRowsMightMatch; + } + + Result<bool> In(const std::shared_ptr<Bound>& expr, + const BoundSetPredicate::LiteralSet& literal_set) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value() || !summary.upper_bound.has_value()) { + // values are all null and literalSet cannot contain null. + return kRowCannotMatch; + } + if (literal_set.size() > kInPredicateLimit) { + // skip evaluating the predicate if the number of values is too big + return kRowsMightMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + + auto literals_view = literal_set | std::views::filter([&](const Literal& lit) { + return lower <= lit; + }); + + if (literals_view.empty()) { + // if all values are less than lower bound, rows cannot match. + return kRowCannotMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + auto filtered_view = literals_view | std::views::filter([&](const Literal& lit) { + return upper >= lit; + }); + if (filtered_view.empty()) { + // if all remaining values are greater than upper bound, rows cannot match. + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> NotIn(const std::shared_ptr<Bound>& expr, + const BoundSetPredicate::LiteralSet& literal_set) override { + // because the bounds are not necessarily a min or max value, this cannot be answered + // using them. notIn(col, {X, ...}) with (X, Y) doesn't guarantee that X is a value in + // col. + return kRowsMightMatch; + } + + Result<bool> StartsWith(const std::shared_ptr<Bound>& expr, + const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value() || !summary.upper_bound.has_value()) { + return kRowCannotMatch; + } + if (lit.type()->type_id() != TypeId::kString) { + return kRowCannotMatch; + } + const auto& prefix = get<std::string>(lit.value()); Review Comment: I'm a little confused why the namespace prefix is not needed here. ########## src/iceberg/expression/manifest_evaluator.cc: ########## @@ -0,0 +1,397 @@ +/* + * 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/manifest_evaluator.h" + +#include "iceberg/expression/binder.h" +#include "iceberg/expression/expression_visitor.h" +#include "iceberg/expression/rewrite_not.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/row/struct_like.h" +#include "iceberg/schema.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { +constexpr bool kRowsMightMatch = true; +constexpr bool kRowCannotMatch = false; +constexpr int32_t kInPredicateLimit = 200; +} // namespace + +class ManifestEvalVisitor : public BoundVisitor<bool> { + public: + explicit ManifestEvalVisitor(const ManifestFile& manifest) + : stats_(manifest.partitions) {} + + Result<bool> AlwaysTrue() override { return true; } + + Result<bool> AlwaysFalse() override { return false; } + + Result<bool> Not(bool child_result) override { return !child_result; } + + Result<bool> And(bool left_result, bool right_result) override { + return left_result && right_result; + } + + Result<bool> Or(bool left_result, bool right_result) override { + return left_result || right_result; + } + + Result<bool> IsNull(const std::shared_ptr<Bound>& expr) override { + // no need to check whether the field is required because binding evaluates that case + // if the column has no null values, the expression cannot match + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (!stats_.at(pos).contains_null) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotNull(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (AllValuesAreNull(stats_.at(pos), ref->type()->type_id())) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> IsNaN(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + if (stats_.at(pos).contains_nan.has_value() && !stats_.at(pos).contains_nan.value()) { + return kRowCannotMatch; + } + if (AllValuesAreNull(stats_.at(pos), ref->type()->type_id())) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotNaN(const std::shared_ptr<Bound>& expr) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + // if containsNaN is true, containsNull is false and lowerBound is null, all values + // are NaN + if (summary.contains_nan.has_value() && summary.contains_nan.value() && + !summary.contains_null && !summary.lower_bound.has_value()) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> Lt(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower >= lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> LtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower > lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> Gt(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper <= lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> GtEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null + } + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper < lit) { + return kRowCannotMatch; + } + return kRowsMightMatch; + } + + Result<bool> Eq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value() || !summary.upper_bound.has_value()) { + return kRowCannotMatch; // values are all null and literal cannot contain null + } + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + if (lower > lit) { + return kRowCannotMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + if (upper < lit) { + return kRowCannotMatch; + } + + return kRowsMightMatch; + } + + Result<bool> NotEq(const std::shared_ptr<Bound>& expr, const Literal& lit) override { + // because the bounds are not necessarily a min or max value, this cannot be answered + // using them. notEq(col, X) with (X, Y) doesn't guarantee that X is a value in col. + return kRowsMightMatch; + } + + Result<bool> In(const std::shared_ptr<Bound>& expr, + const BoundSetPredicate::LiteralSet& literal_set) override { + ICEBERG_ASSIGN_OR_RAISE(auto ref, ParseBoundReference(expr)); + ICEBERG_ASSIGN_OR_RAISE(auto pos, GetPosition(*ref)); + const auto& summary = stats_.at(pos); + if (!summary.lower_bound.has_value() || !summary.upper_bound.has_value()) { + // values are all null and literalSet cannot contain null. + return kRowCannotMatch; + } + if (literal_set.size() > kInPredicateLimit) { + // skip evaluating the predicate if the number of values is too big + return kRowsMightMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto lower, DeserializeBoundLiteral(summary.lower_bound.value(), ref->type())); + + auto literals_view = literal_set | std::views::filter([&](const Literal& lit) { + return lower <= lit; + }); + + if (literals_view.empty()) { + // if all values are less than lower bound, rows cannot match. + return kRowCannotMatch; + } + + ICEBERG_ASSIGN_OR_RAISE( + auto upper, DeserializeBoundLiteral(summary.upper_bound.value(), ref->type())); + auto filtered_view = literals_view | std::views::filter([&](const Literal& lit) { Review Comment: How about merging two `std::views::filter`? ```cpp auto filtered_view = literals_view | std::views::filter([&lower, &upper](const Literal& lit) { return lower <= lit && lit <= upper; }); ``` ########## src/iceberg/expression/manifest_evaluator.cc: ########## @@ -0,0 +1,397 @@ +/* + * 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/manifest_evaluator.h" + +#include "iceberg/expression/binder.h" +#include "iceberg/expression/expression_visitor.h" +#include "iceberg/expression/rewrite_not.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/row/struct_like.h" +#include "iceberg/schema.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { +constexpr bool kRowsMightMatch = true; +constexpr bool kRowCannotMatch = false; +constexpr int32_t kInPredicateLimit = 200; +} // namespace + +class ManifestEvalVisitor : public BoundVisitor<bool> { + public: + explicit ManifestEvalVisitor(const ManifestFile& manifest) + : stats_(manifest.partitions) {} + + Result<bool> AlwaysTrue() override { return true; } Review Comment: @wgtmac Too many subclasses override these methods but it seems meaningless. Should we open another PR to merge the implementations? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
