wgtmac commented on code in PR #800:
URL: https://github.com/apache/iceberg-cpp/pull/800#discussion_r3695115977
##########
src/iceberg/avro/avro_data_util.cc:
##########
@@ -497,6 +507,135 @@ 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 PrepareStructDefaultScalars(std::span<FieldProjection> projections,
+ ::arrow::ArrayBuilder* builder);
+
+// Recurse into whatever nested builder this projection describes, so a
default cached for
+// a struct field is found at any nesting depth (e.g.
`list<list<struct<...>>>`) instead
+// of only when the collection's child is an immediate struct.
+Status PrepareNestedDefaultScalars(FieldProjection& projection,
+ ::arrow::ArrayBuilder* builder) {
+ if (projection.kind != FieldProjection::Kind::kProjected ||
+ projection.children.empty()) {
+ return {};
+ }
+
+ switch (builder->type()->id()) {
+ case ::arrow::Type::STRUCT:
+ return PrepareStructDefaultScalars(projection.children, builder);
+ case ::arrow::Type::LIST: {
+ // List projections store a single child for the element.
+ auto* list_builder =
internal::checked_cast<::arrow::ListBuilder*>(builder);
+ return PrepareNestedDefaultScalars(projection.children[0],
+ list_builder->value_builder());
+ }
+ case ::arrow::Type::LARGE_LIST: {
+ auto* list_builder =
internal::checked_cast<::arrow::LargeListBuilder*>(builder);
+ return PrepareNestedDefaultScalars(projection.children[0],
+ list_builder->value_builder());
+ }
+ case ::arrow::Type::MAP: {
+ auto* map_builder =
internal::checked_cast<::arrow::MapBuilder*>(builder);
+ if (projection.children.size() >= 1) {
+ ICEBERG_RETURN_UNEXPECTED(PrepareNestedDefaultScalars(
+ projection.children[0], map_builder->key_builder()));
+ }
+ if (projection.children.size() >= 2) {
+ ICEBERG_RETURN_UNEXPECTED(PrepareNestedDefaultScalars(
+ projection.children[1], map_builder->item_builder()));
+ }
+ return {};
+ }
+ default:
+ return {};
+ }
+}
+
+Status PrepareStructDefaultScalars(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 (field_projection.attributes != nullptr) {
Review Comment:
`attributes != nullptr` does not guarantee that `default_scalar` is prepared
now that this container may hold other Avro attributes. If another attribute
creates the container first, this skips preparation and the append path falls
back to rebuilding the scalar per row. Please reuse/cast the existing
`AvroExtraAttributes`, check `default_scalar` instead, and add a test with a
pre-existing empty container.
--
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]
