diff --git a/src/iceberg/expression/binder.cc b/src/iceberg/expression/binder.cc index 9474151d7..9f1b44fbd 100644 --- a/src/iceberg/expression/binder.cc +++ b/src/iceberg/expression/binder.cc @@ -161,7 +161,10 @@ Result ReferenceVisitor::Predicate( Result ReferenceVisitor::Aggregate( const std::shared_ptr& aggregate) { - referenced_field_ids_.insert(aggregate->reference()->field_id()); + // Aggregates without a term (e.g. COUNT(*)) reference no field ids. + if (const auto& reference = aggregate->reference()) { + referenced_field_ids_.insert(reference->field_id()); + } return referenced_field_ids_; } diff --git a/src/iceberg/test/expression_visitor_test.cc b/src/iceberg/test/expression_visitor_test.cc index a3b2c4cad..ca2e039e0 100644 --- a/src/iceberg/test/expression_visitor_test.cc +++ b/src/iceberg/test/expression_visitor_test.cc @@ -805,6 +805,25 @@ TEST_F(ReferenceVisitorTest, Constants) { EXPECT_TRUE(refs_false.empty()); } +TEST_F(ReferenceVisitorTest, CountStar) { + // COUNT(*) has no referenced field; the aggregate's reference() is null, so + // GetReferencedFieldIds must return an empty set instead of dereferencing it. + ICEBERG_UNWRAP_OR_FAIL(auto bound_count_star, Bind(Expressions::CountStar())); + + ICEBERG_UNWRAP_OR_FAIL(auto refs, + ReferenceVisitor::GetReferencedFieldIds(bound_count_star)); + EXPECT_TRUE(refs.empty()); +} + +TEST_F(ReferenceVisitorTest, AggregateWithTerm) { + // An aggregate with a term (e.g. MAX(age)) still references its field, so the + // null guard must not drop the non-null case. age has field id 3. + ICEBERG_UNWRAP_OR_FAIL(auto bound_max, Bind(Expressions::Max("age"))); + + ICEBERG_UNWRAP_OR_FAIL(auto refs, ReferenceVisitor::GetReferencedFieldIds(bound_max)); + EXPECT_THAT(refs, ::testing::UnorderedElementsAre(3)); +} + TEST_F(ReferenceVisitorTest, UnboundPredicate) { auto unbound_pred = Expressions::Equal("name", Literal::String("Alice")); auto result = ReferenceVisitor::GetReferencedFieldIds(unbound_pred);