wgtmac commented on code in PR #800:
URL: https://github.com/apache/iceberg-cpp/pull/800#discussion_r3649740719
##########
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:
Please recurse through nested list and map builders, not just an immediate
struct. For example, a default under `list<list<struct<...>>>` is not cached
and is rebuilt for every element. Please add a nested-collection test as well.
--
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]
