huan233usc commented on code in PR #800:
URL: https://github.com/apache/iceberg-cpp/pull/800#discussion_r3653197117
##########
src/iceberg/avro/avro_data_util.cc:
##########
@@ -497,6 +507,128 @@ Status AppendFieldToBuilder(const ::avro::NodePtr&
avro_node,
} // namespace
+namespace {
+
+Result<std::shared_ptr<::arrow::Scalar>> MakeDefaultScalar(
+ const Literal& literal, const std::shared_ptr<::arrow::DataType>&
builder_type) {
+ // The builder's own memory pool is not exposed, so the small scalar buffer
uses the
+ // default pool.
+ ICEBERG_ASSIGN_OR_RAISE(std::shared_ptr<::arrow::Scalar> scalar,
+ arrow::ToArrowScalar(literal,
::arrow::default_memory_pool()));
+
+ // For an extension builder (e.g. `arrow.uuid`) target its storage type:
ToArrowScalar
+ // yields the storage scalar (fixed_size_binary(16) for uuid) and
Scalar::CastTo has no
+ // kernel that targets an extension type. This mirrors MakeDefaultArray's
extension
+ // handling.
+ std::shared_ptr<::arrow::DataType> target_type = builder_type;
+ if (target_type->id() == ::arrow::Type::EXTENSION) {
+ target_type = internal::checked_cast<const
::arrow::ExtensionType&>(*target_type)
+ .storage_type();
+ }
+
+ if (!scalar->type->Equals(*target_type)) {
+ ICEBERG_ARROW_ASSIGN_OR_RETURN(scalar, scalar->CastTo(target_type));
+ }
+ return scalar;
+}
+
+Status PrepareDefaultScalars(std::span<FieldProjection> projections,
+ ::arrow::ArrayBuilder* builder) {
+ auto* struct_builder =
internal::checked_cast<::arrow::StructBuilder*>(builder);
+ if (static_cast<size_t>(struct_builder->num_fields()) != projections.size())
{
+ return InvalidArgument(
+ "Inconsistent number of struct builder fields ({}) and projections
({})",
+ struct_builder->num_fields(), projections.size());
+ }
+
+ for (size_t i = 0; i < projections.size(); ++i) {
+ auto& field_projection = projections[i];
+ auto* field_builder = struct_builder->field_builder(static_cast<int>(i));
+
+ if (field_projection.kind == FieldProjection::Kind::kDefault) {
+ if (dynamic_cast<const
AvroDefaultAttributes*>(field_projection.attributes.get()) !=
+ nullptr) {
+ continue;
+ }
+ auto attrs = std::make_shared<AvroDefaultAttributes>();
+ ICEBERG_ASSIGN_OR_RAISE(attrs->scalar,
+
MakeDefaultScalar(std::get<Literal>(field_projection.from),
+ field_builder->type()));
+ field_projection.attributes = std::move(attrs);
+ continue;
+ }
+
+ if (field_projection.kind != FieldProjection::Kind::kProjected ||
+ field_projection.children.empty()) {
+ continue;
+ }
+
+ switch (field_builder->type()->id()) {
+ case ::arrow::Type::STRUCT:
+ ICEBERG_RETURN_UNEXPECTED(
+ PrepareDefaultScalars(field_projection.children, field_builder));
+ break;
+ case ::arrow::Type::LIST: {
+ // List projections store a single child for the element. Defaults
only appear
+ // on nested struct fields of that element.
+ auto* list_builder =
internal::checked_cast<::arrow::ListBuilder*>(field_builder);
+ auto& element_projection = field_projection.children[0];
+ if (element_projection.kind == FieldProjection::Kind::kProjected &&
Review Comment:
Done — the recursion now dispatches on the builder type
(struct/list/large_list/map) instead of only descending when the collection
child is an immediate struct, so a default under `list<list<struct<...>>>` gets
its scalar prepared once at any depth. Added a nested-collection test asserting
the scalar is prepared, plus an end-to-end `list<list<struct>>` decode test
(the former fails without the change).
--
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]
