(iceberg-cpp) branch main updated: fix(expression): treat COUNT(*) as referencing no fields in GetReferencedFieldIds (#981)

[gangwu]

This is an automated email from the ASF dual-hosted git repository.

wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git


The following commit(s) were added to refs/heads/main by this push:
     new de7c9ce0 fix(expression): treat COUNT(*) as referencing no fields in 
GetReferencedFieldIds (#981)
de7c9ce0 is described below

commit de7c9ce0715c5846ab5ce3cbcaa6d90c199a5ba1
Author: YangJie <yangji...@baidu.com>
AuthorDate: Tue Oct 13 03:16:52 2026 -0400

    fix(expression): treat COUNT(*) as referencing no fields in 
GetReferencedFieldIds (#981)
    
    ReferenceVisitor::Aggregate unconditionally called
    aggregate->reference()->field_id(), but BoundAggregate::reference()
    returns nullptr for aggregates without a term such as COUNT(*), so
    GetReferencedFieldIds on a bound count(*) dereferenced null and
    crashed. Skip the insert when there is no reference; the aggregate
    then correctly contributes no field ids, matching the Java reference.
---
 src/iceberg/expression/binder.cc            |  5 ++++-
 src/iceberg/test/expression_visitor_test.cc | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/iceberg/expression/binder.cc b/src/iceberg/expression/binder.cc
index ecc1f1f2..52dede27 100644
--- a/src/iceberg/expression/binder.cc
+++ b/src/iceberg/expression/binder.cc
@@ -180,7 +180,10 @@ Result<FieldIdsSetRef> ReferenceVisitor::Predicate(
 
 Result<FieldIdsSetRef> ReferenceVisitor::Aggregate(
     const std::shared_ptr<BoundAggregate>& 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 e58902dc..d24142cf 100644
--- a/src/iceberg/test/expression_visitor_test.cc
+++ b/src/iceberg/test/expression_visitor_test.cc
@@ -839,6 +839,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);