fix: respect nullability in nested struct accessors

src/iceberg/row/struct_like.cc

@@ -76,39 +76,73 @@ Result<Scalar> LiteralToScalar(const Literal& literal) {
   }
 }
 
+namespace {
+
+Result<Scalar> GetCheckedField(const StructLike& row, size_t pos, bool optional) {
+  ICEBERG_ASSIGN_OR_RAISE(auto field, row.GetField(pos));
+  if (!optional && std::holds_alternative<std::monostate>(field)) {
+    return InvalidArgument("Required field at position {} is null", pos);
+  }
+  return field;
+}
+
+Result<std::shared_ptr<StructLike>> GetParentStruct(const StructLike& row, size_t pos,
+                                                    bool optional) {
+  ICEBERG_ASSIGN_OR_RAISE(auto field, GetCheckedField(row, pos, optional));
+  if (std::holds_alternative<std::monostate>(field)) return std::shared_ptr<StructLike>{};
+  if (!std::holds_alternative<std::shared_ptr<StructLike>>(field)) {
+    return InvalidSchema("Encountered non-struct at position {}", pos);
+  }
+  auto parent = std::get<std::shared_ptr<StructLike>>(std::move(field));
+  if (!parent && !optional) {
+    return InvalidArgument("Required field at position {} is null", pos);
+  }
+  return parent;
+}
+
+}  // namespace
+
 StructLikeAccessor::StructLikeAccessor(std::shared_ptr<Type> type,
-                                       std::span<const size_t> position_path)
-    : type_(std::move(type)), position_path_(position_path.begin(), position_path.end()) {
-  if (position_path.size() == 1) {
-    accessor_ = [pos =
-                     position_path[0]](const StructLike& struct_like) -> Result<Scalar> {
-      return struct_like.GetField(pos);
+                                       std::span<const size_t> position_path,
+                                       std::vector<bool> is_optional)
+    : type_(std::move(type)),
+      position_path_(position_path.begin(), position_path.end()),
+      is_optional_(std::move(is_optional)) {
+  if (position_path_.size() != is_optional_.size()) {
+    accessor_ = [](const StructLike&) -> Result<Scalar> {
+      return InvalidArgument("Optionality count does not match position path");
     };
-  } else if (position_path.size() == 2) {
-    accessor_ = [pos0 = position_path[0], pos1 = position_path[1]](
+    return;
+  }
+  if (position_path_.size() == 1) {
+    accessor_ = [pos = position_path_[0], optional = static_cast<bool>(is_optional_[0])](
                     const StructLike& struct_like) -> Result<Scalar> {
-      ICEBERG_ASSIGN_OR_RAISE(auto first_level_field, struct_like.GetField(pos0));
-      if (!std::holds_alternative<std::shared_ptr<StructLike>>(first_level_field)) {
-        return InvalidSchema("Encountered non-struct in the position path [{},{}]", pos0,
-                             pos1);
-      }
-      return std::get<std::shared_ptr<StructLike>>(first_level_field)->GetField(pos1);
+      return GetCheckedField(struct_like, pos, optional);
+    };
+  } else if (position_path_.size() == 2) {
+    accessor_ = [pos0 = position_path_[0], pos1 = position_path_[1],
+                 optional = static_cast<bool>(is_optional_[0]),
+                 leaf_optional = static_cast<bool>(is_optional_[1])](
+                    const StructLike& struct_like) -> Result<Scalar> {
+      ICEBERG_ASSIGN_OR_RAISE(auto nested, GetParentStruct(struct_like, pos0, optional));
+      if (!nested) return Scalar{std::monostate{}};
+      return GetCheckedField(*nested, pos1, leaf_optional);
     };
-  } else if (!position_path.empty()) {
+  } else if (!position_path_.empty()) {
     accessor_ = [this](const StructLike& struct_like) -> Result<Scalar> {
       std::vector<std::shared_ptr<StructLike>> backups;
+      backups.reserve(position_path_.size() - 1);
       const StructLike* current_struct_like = &struct_like;
       for (size_t i = 0; i < position_path_.size() - 1; ++i) {
-        ICEBERG_ASSIGN_OR_RAISE(auto field,
-                                current_struct_like->GetField(position_path_[i]));
-        if (!std::holds_alternative<std::shared_ptr<StructLike>>(field)) {
-          return InvalidSchema("Encountered non-struct in the position path [{}]",
-                               position_path_);
-        }
-        backups.push_back(std::get<std::shared_ptr<StructLike>>(field));
+        ICEBERG_ASSIGN_OR_RAISE(
+            auto parent,
+            GetParentStruct(*current_struct_like, position_path_[i], is_optional_[i]));
+        if (!parent) return Scalar{std::monostate{}};
+        backups.push_back(std::move(parent));
         current_struct_like = backups.back().get();
       }
-      return current_struct_like->GetField(position_path_.back());
+      return GetCheckedField(*current_struct_like, position_path_.back(),
+                             is_optional_.back());
     };
   } else {
     accessor_ = [](const StructLike&) -> Result<Scalar> {

src/iceberg/row/struct_like.h

@@ -31,6 +31,7 @@
 #include <span>
 #include <string_view>
 #include <variant>
+#include <vector>
 
 #include "iceberg/expression/literal.h"
 #include "iceberg/result.h"
@@ -105,9 +106,11 @@ class ICEBERG_EXPORT MapLike {
 class ICEBERG_EXPORT StructLikeAccessor {
  public:
   explicit StructLikeAccessor(std::shared_ptr<Type> type,
-                              std::span<const size_t> position_path);
+                              std::span<const size_t> position_path,
+                              std::vector<bool> is_optional);
 
   /// \brief Get the scalar value at the given position.
+  /// A null optional parent returns monostate; a null required field is an error.
   Result<Scalar> Get(const StructLike& struct_like) const {
     return accessor_(struct_like);
   }
@@ -121,13 +124,15 @@ class ICEBERG_EXPORT StructLikeAccessor {
   /// \brief Get the type of the value that this accessor is bound to.
   const Type& type() const { return *type_; }
 
-  /// \brief Get the position path of the value that this accessor bounded to.
+  /// \brief Get the position path of the value that this accessor is bound to.
   const std::vector<size_t>& position_path() const { return position_path_; }
 
  private:
   std::shared_ptr<Type> type_;
   std::function<Result<Scalar>(const StructLike&)> accessor_;
   std::vector<size_t> position_path_;
+  // One entry per position, including the leaf field.
+  std::vector<bool> is_optional_;
 };
 
 }  // namespace iceberg

src/iceberg/schema.cc

@@ -357,7 +357,18 @@ Result<std::unique_ptr<StructLikeAccessor>> Schema::GetAccessorById(
     if (!field.has_value()) {
       return NotFound("Cannot get accessor for field id: {}", field_id);
     }
-    return std::make_unique<StructLikeAccessor>(field.value().get().type(), it->second);
+    std::vector<bool> is_optional;
+    is_optional.reserve(it->second.size());
+    const StructType* current = this;
+    for (size_t i = 0; i < it->second.size(); ++i) {
+      const auto& field_at_position = current->fields()[it->second[i]];
+      is_optional.push_back(field_at_position.optional());
+      if (i + 1 < it->second.size()) {
+        current = static_cast<const StructType*>(field_at_position.type().get());
+      }
+    }
+    return std::make_unique<StructLikeAccessor>(field.value().get().type(), it->second,
+                                                std::move(is_optional));
   }
   return NotFound("Cannot get accessor for field id: {}", field_id);
 }

src/iceberg/test/evaluator_test.cc

@@ -330,7 +330,7 @@ TEST_F(EvaluatorTest, NotEqual) {
 
 TEST_F(EvaluatorTest, StartsWith) {
   auto string_schema = std::make_unique<Schema>(
-      std::vector<SchemaField>{SchemaField::MakeRequired(24, "s", string())});
+      std::vector<SchemaField>{SchemaField::MakeOptional(24, "s", string())});
   ICEBERG_UNWRAP_OR_FAIL(
       auto evaluator,
       Evaluator::Make(*string_schema, Expressions::StartsWith("s", "abc")));

src/iceberg/test/struct_like_test.cc

@@ -20,7 +20,10 @@
 #include "iceberg/row/struct_like.h"
 
 #include <array>
+#include <memory>
+#include <optional>
 #include <utility>
+#include <vector>
 
 #include <arrow/c/bridge.h>
 #include <arrow/json/from_string.h>
@@ -71,6 +74,7 @@ namespace {
 class SingleFieldStructLike : public StructLike {
  public:
   explicit SingleFieldStructLike(Scalar value) : value_(std::move(value)) {}
+  explicit SingleFieldStructLike(Result<Scalar> value) : value_(std::move(value)) {}
 
   Result<Scalar> GetField(size_t pos) const override {
     if (pos != 0) {
@@ -82,9 +86,35 @@ class SingleFieldStructLike : public StructLike {
   size_t num_fields() const override { return 1; }
 
  private:
-  Scalar value_;
+  Result<Scalar> value_;
 };
 
+std::shared_ptr<StructLike> WrapSingleFieldRow(Result<Scalar> field, size_t depth) {
+  std::shared_ptr<StructLike> row =
+      std::make_shared<SingleFieldStructLike>(std::move(field));
+  for (size_t i = 1; i < depth; ++i) {
+    row = std::make_shared<SingleFieldStructLike>(Scalar{row});
+  }
+  return row;
+}
+
+std::unique_ptr<Schema> MakeNestedSchema(
+    size_t depth, std::optional<size_t> optional_parent = std::nullopt,
+    bool optional_leaf = false) {
+  auto field =
+      optional_leaf
+          ? SchemaField::MakeOptional(static_cast<int32_t>(depth), "value", int32())
+          : SchemaField::MakeRequired(static_cast<int32_t>(depth), "value", int32());
+  for (size_t level = depth - 1; level > 0; --level) {
+    auto nested = struct_({field});
+    field =
+        optional_parent == level - 1
+            ? SchemaField::MakeOptional(static_cast<int32_t>(level), "nested", nested)
+            : SchemaField::MakeRequired(static_cast<int32_t>(level), "nested", nested);
+  }
+  return std::make_unique<Schema>(std::vector<SchemaField>{std::move(field)});
+}
+
 }  // namespace
 
 TEST(ManifestFileStructLike, BasicFields) {
@@ -152,7 +182,7 @@ TEST(StructLikeAccessorTest, GetLiteralUuid) {
   std::string_view uuid_data(reinterpret_cast<const char*>(bytes.data()), bytes.size());
   SingleFieldStructLike row(Scalar{uuid_data});
   std::array<size_t, 1> path = {0};
-  StructLikeAccessor accessor(iceberg::uuid(), path);
+  StructLikeAccessor accessor(iceberg::uuid(), path, {false});
 
   ICEBERG_UNWRAP_OR_FAIL(auto literal, accessor.GetLiteral(row));
   EXPECT_EQ(literal.type()->type_id(), TypeId::kUuid);
@@ -163,13 +193,138 @@ TEST(StructLikeAccessorTest, GetLiteralUuid) {
 TEST(StructLikeAccessorTest, GetLiteralUuidRejectsWrongLength) {
   SingleFieldStructLike row(Scalar{std::string_view("not-a-uuid")});
   std::array<size_t, 1> path = {0};
-  StructLikeAccessor accessor(iceberg::uuid(), path);
+  StructLikeAccessor accessor(iceberg::uuid(), path, {false});
 
   auto result = accessor.GetLiteral(row);
   EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument));
   EXPECT_THAT(result, HasErrorMessage("UUID byte array must be exactly 16 bytes"));
 }
 
+TEST(StructLikeAccessorTest, RequiredAndOptionalLeaf) {
+  SingleFieldStructLike row(Scalar{std::monostate{}});
+  for (bool optional : {false, true}) {
+    auto schema = MakeNestedSchema(1, std::nullopt, optional);
+    ICEBERG_UNWRAP_OR_FAIL(auto accessor, schema->GetAccessorById(1));
+    if (optional) {
+      EXPECT_SCALAR_NULL(accessor->Get(row));
+      ICEBERG_UNWRAP_OR_FAIL(auto literal, accessor->GetLiteral(row));
+      EXPECT_TRUE(literal.IsNull());
+    } else {
+      EXPECT_THAT(accessor->Get(row), IsError(ErrorKind::kInvalidArgument));
+      EXPECT_THAT(accessor->GetLiteral(row), IsError(ErrorKind::kInvalidArgument));
+    }
+  }
+}
+
+class NestedStructLikeAccessorTest : public ::testing::TestWithParam<size_t> {};
+
+TEST_P(NestedStructLikeAccessorTest, NullParents) {
+  const std::vector<size_t> path(GetParam(), 0);
+  StructLikeAccessor accessor(int32(), path, std::vector<bool>(path.size(), true));
+  for (const Scalar null :
+       {Scalar{std::monostate{}}, Scalar{std::shared_ptr<StructLike>{}}}) {
+    for (size_t parent = 0; parent + 1 < path.size(); ++parent) {
+      SCOPED_TRACE(parent);
+      auto row = WrapSingleFieldRow(null, parent + 1);
+      ICEBERG_UNWRAP_OR_FAIL(auto scalar, accessor.Get(*row));
+      EXPECT_TRUE(std::holds_alternative<std::monostate>(scalar));
+      ICEBERG_UNWRAP_OR_FAIL(auto literal, accessor.GetLiteral(*row));
+      EXPECT_TRUE(literal.IsNull());
+      EXPECT_EQ(literal.type()->type_id(), TypeId::kInt);
+    }
+  }
+}
+
+TEST_P(NestedStructLikeAccessorTest, SchemaRequiredAndOptionalParents) {
+  const auto depth = GetParam();
+  auto required_schema = MakeNestedSchema(depth);
+  ICEBERG_UNWRAP_OR_FAIL(auto required_accessor,
+                         required_schema->GetAccessorById(static_cast<int32_t>(depth)));
+  EXPECT_EQ(required_accessor->position_path(), std::vector<size_t>(depth, 0));
+
+  for (const Scalar null :
+       {Scalar{std::monostate{}}, Scalar{std::shared_ptr<StructLike>{}}}) {
+    for (size_t parent = 0; parent + 1 < depth; ++parent) {
+      SCOPED_TRACE(parent);
+      auto row = WrapSingleFieldRow(null, parent + 1);
+      auto schema = MakeNestedSchema(depth, parent);
+      ICEBERG_UNWRAP_OR_FAIL(auto accessor,
+                             schema->GetAccessorById(static_cast<int32_t>(depth)));
+      EXPECT_SCALAR_NULL(accessor->Get(*row));
+      ICEBERG_UNWRAP_OR_FAIL(auto literal, accessor->GetLiteral(*row));
+      EXPECT_TRUE(literal.IsNull());
+      EXPECT_EQ(literal.type()->type_id(), TypeId::kInt);
+
+      EXPECT_THAT(required_accessor->Get(*row), IsError(ErrorKind::kInvalidArgument));
+      EXPECT_THAT(required_accessor->GetLiteral(*row),
+                  IsError(ErrorKind::kInvalidArgument));
+    }
+  }
+}
+
+TEST_P(NestedStructLikeAccessorTest, ValueAndNullLeaf) {
+  const std::vector<size_t> path(GetParam(), 0);
+  StructLikeAccessor accessor(int32(), path, std::vector<bool>(path.size(), true));
+  auto row = WrapSingleFieldRow(Scalar{int32_t{42}}, path.size());
+  ICEBERG_UNWRAP_OR_FAIL(auto value, accessor.GetLiteral(*row));
+  EXPECT_EQ(value, Literal::Int(42));
+
+  auto null_row = WrapSingleFieldRow(Scalar{std::monostate{}}, path.size());
+  ICEBERG_UNWRAP_OR_FAIL(auto null, accessor.GetLiteral(*null_row));
+  EXPECT_TRUE(null.IsNull());
+  EXPECT_EQ(null.type()->type_id(), TypeId::kInt);
+}
+
+TEST_P(NestedStructLikeAccessorTest, RequiredAndOptionalLeaf) {
+  const auto depth = GetParam();
+  auto row = WrapSingleFieldRow(Scalar{std::monostate{}}, depth);
+  for (bool optional : {false, true}) {
+    auto schema = MakeNestedSchema(depth, std::nullopt, optional);
+    ICEBERG_UNWRAP_OR_FAIL(auto accessor,
+                           schema->GetAccessorById(static_cast<int32_t>(depth)));
+    if (optional) {
+      EXPECT_SCALAR_NULL(accessor->Get(*row));
+      ICEBERG_UNWRAP_OR_FAIL(auto literal, accessor->GetLiteral(*row));
+      EXPECT_TRUE(literal.IsNull());
+    } else {
+      EXPECT_THAT(accessor->Get(*row), IsError(ErrorKind::kInvalidArgument));
+      EXPECT_THAT(accessor->GetLiteral(*row), IsError(ErrorKind::kInvalidArgument));
+    }
+  }
+
+  auto schema = MakeNestedSchema(depth, 0);
+  ICEBERG_UNWRAP_OR_FAIL(auto accessor,
+                         schema->GetAccessorById(static_cast<int32_t>(depth)));
+  EXPECT_THAT(accessor->Get(*row), IsError(ErrorKind::kInvalidArgument));
+  auto absent_parent = WrapSingleFieldRow(Scalar{std::monostate{}}, 1);
+  EXPECT_SCALAR_NULL(accessor->Get(*absent_parent));
+}
+
+TEST_P(NestedStructLikeAccessorTest, RejectsNonStructParents) {
+  const std::vector<size_t> path(GetParam(), 0);
+  StructLikeAccessor accessor(int32(), path, std::vector<bool>(path.size(), true));
+  for (size_t parent = 0; parent + 1 < path.size(); ++parent) {
+    SCOPED_TRACE(parent);
+    auto row = WrapSingleFieldRow(Scalar{int32_t{42}}, parent + 1);
+    EXPECT_THAT(accessor.Get(*row), IsError(ErrorKind::kInvalidSchema));
+  }
+}
+
+TEST_P(NestedStructLikeAccessorTest, PreservesFieldErrors) {
+  const std::vector<size_t> path(GetParam(), 0);
+  StructLikeAccessor accessor(int32(), path, std::vector<bool>(path.size(), true));
+  for (size_t level = 0; level < path.size(); ++level) {
+    SCOPED_TRACE(level);
+    auto row = WrapSingleFieldRow(IOError("Cannot read field"), level + 1);
+    auto result = accessor.GetLiteral(*row);
+    EXPECT_THAT(result, IsError(ErrorKind::kIOError));
+    EXPECT_THAT(result, HasErrorMessage("Cannot read field"));
+  }
+}
+
+INSTANTIATE_TEST_SUITE_P(NestedPaths, NestedStructLikeAccessorTest,
+                         ::testing::Values(size_t{2}, size_t{3}, size_t{4}));
+
 TEST(ManifestFileStructLike, OptionalFields) {
   ManifestFile manifest_file{.manifest_path = "/path/to/manifest2.avro",
                              .manifest_length = 54321,