Skip to content

Commit 90e7fc8

Browse files
committed
fix review
1 parent 5251e97 commit 90e7fc8

File tree

2 files changed

+14
-9
lines changed

2 files changed

+14
-9
lines changed

src/iceberg/expression/residual_evaluator.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ Result<std::shared_ptr<Expression>> ResidualVisitor::Predicate(
235235
// Not associated with a partition field, can't be evaluated
236236
return pred;
237237
}
238+
auto schema = partition_type_->ToSchema();
238239

239240
for (const auto& part : parts) {
240241
// Check the strict projection
@@ -243,9 +244,8 @@ Result<std::shared_ptr<Expression>> ResidualVisitor::Predicate(
243244
std::shared_ptr<Expression> strict_result = nullptr;
244245

245246
if (strict_projection != nullptr) {
246-
ICEBERG_ASSIGN_OR_RAISE(
247-
auto bound_strict,
248-
strict_projection->Bind(*partition_type_->ToSchema(), case_sensitive_));
247+
ICEBERG_ASSIGN_OR_RAISE(auto bound_strict,
248+
strict_projection->Bind(*schema, case_sensitive_));
249249
if (bound_strict->is_bound_predicate()) {
250250
ICEBERG_ASSIGN_OR_RAISE(
251251
strict_result, BoundVisitor::Predicate(
@@ -268,9 +268,8 @@ Result<std::shared_ptr<Expression>> ResidualVisitor::Predicate(
268268
std::shared_ptr<Expression> inclusive_result = nullptr;
269269

270270
if (inclusive_projection != nullptr) {
271-
ICEBERG_ASSIGN_OR_RAISE(
272-
auto bound_inclusive,
273-
inclusive_projection->Bind(*partition_type_->ToSchema(), case_sensitive_));
271+
ICEBERG_ASSIGN_OR_RAISE(auto bound_inclusive,
272+
inclusive_projection->Bind(*schema, case_sensitive_));
274273

275274
if (bound_inclusive->is_bound_predicate()) {
276275
ICEBERG_ASSIGN_OR_RAISE(

src/iceberg/schema.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,28 @@ class ICEBERG_EXPORT Schema : public StructType {
5959

6060
std::string ToString() const override;
6161

62-
/// \brief Recursively Find the SchemaField by field name.
62+
/// \brief Recursively find the SchemaField by field name.
6363
///
6464
/// Short names for maps and lists are included for any name that does not conflict with
6565
/// a canonical name. For example, a list, 'l', of structs with field 'x' will produce
66-
/// short name 'l.x' in addition to canonical name 'l.element.x'.
66+
/// short name 'l.x' in addition to canonical name 'l.element.x'. A map 'm', if its
67+
/// value include a structs with field 'x' wil produce short name 'm.x' in addition to
68+
/// canonical name 'm.value.x'.
6769
/// FIXME: Currently only handles ASCII lowercase conversion; extend to support
6870
/// non-ASCII characters (e.g., using std::towlower or ICU)
6971
Result<std::optional<std::reference_wrapper<const SchemaField>>> FindFieldByName(
7072
std::string_view name, bool case_sensitive = true) const;
7173

72-
/// \brief Recursively Find the SchemaField by field id.
74+
/// \brief Recursively find the SchemaField by field id.
75+
///
76+
/// \param field_id The id of the field to get the accessor for.
77+
/// \return The field with the given id, or std::nullopt if not found.
7378
Result<std::optional<std::reference_wrapper<const SchemaField>>> FindFieldById(
7479
int32_t field_id) const;
7580

7681
/// \brief Get the accessor to access the field by field id.
7782
///
83+
/// \param field_id The id of the field to get the accessor for.
7884
/// \return The accessor to access the field, or NotFound if the field is not found.
7985
Result<std::unique_ptr<StructLikeAccessor>> GetAccessorById(int32_t field_id) const;
8086

0 commit comments

Comments
 (0)