refactor(avro): address review feedback

Place the mutable builder parameter last and express debug-only array validation with assert.

Author: gripleaf

src/paimon/format/avro/avro_direct_decoder.cpp

@@ -57,7 +57,7 @@ Status DecodeFieldToBuilder(const ::avro::NodePtr& avro_node,
                             ::avro::Decoder* decoder, arrow::ArrayBuilder* array_builder,
                             AvroDirectDecoder::DecodeContext* ctx);
 
-Status ReserveBuilderCapacityImpl(arrow::ArrayBuilder* array_builder, int64_t capacity) {
+Status ReserveBuilderCapacityImpl(int64_t capacity, arrow::ArrayBuilder* array_builder) {
     PAIMON_RETURN_NOT_OK_FROM_ARROW(array_builder->Reserve(capacity));
     if (array_builder->type()->id() != arrow::Type::STRUCT) {
         return Status::OK();
@@ -66,7 +66,7 @@ Status ReserveBuilderCapacityImpl(arrow::ArrayBuilder* array_builder, int64_t ca
     auto* struct_builder = checked_cast<arrow::StructBuilder*>(array_builder);
     for (int32_t i = 0; i < struct_builder->num_fields(); ++i) {
         PAIMON_RETURN_NOT_OK(
-            ReserveBuilderCapacityImpl(struct_builder->field_builder(i), capacity));
+            ReserveBuilderCapacityImpl(capacity, struct_builder->field_builder(i)));
     }
     return Status::OK();
 }
@@ -207,7 +207,7 @@ Status DecodeListToBuilder(const ::avro::NodePtr& avro_node, ::avro::Decoder* de
     // Read array block count
     int64_t block_count = decoder->arrayStart();
     while (block_count != 0) {
-        PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(value_builder, block_count));
+        PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(block_count, value_builder));
         for (int64_t i = 0; i < block_count; ++i) {
             PAIMON_RETURN_NOT_OK(DecodeFieldToBuilder(element_node, /*projection=*/std::nullopt,
                                                       decoder, value_builder, ctx));
@@ -236,8 +236,8 @@ Status DecodeMapToBuilder(const ::avro::NodePtr& avro_node, ::avro::Decoder* dec
         // Read map block count
         int64_t block_count = decoder->mapStart();
         while (block_count != 0) {
-            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(key_builder, block_count));
-            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(item_builder, block_count));
+            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(block_count, key_builder));
+            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(block_count, item_builder));
             for (int64_t i = 0; i < block_count; ++i) {
                 PAIMON_RETURN_NOT_OK(DecodeFieldToBuilder(key_node, /*projection=*/std::nullopt,
                                                           decoder, key_builder, ctx));
@@ -265,8 +265,8 @@ Status DecodeMapToBuilder(const ::avro::NodePtr& avro_node, ::avro::Decoder* dec
         // Read array block count
         int64_t block_count = decoder->arrayStart();
         while (block_count != 0) {
-            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(key_builder, block_count));
-            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(item_builder, block_count));
+            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(block_count, key_builder));
+            PAIMON_RETURN_NOT_OK(ReserveBuilderCapacityImpl(block_count, item_builder));
             for (int64_t i = 0; i < block_count; ++i) {
                 PAIMON_RETURN_NOT_OK(DecodeFieldToBuilder(key_node, /*projection=*/std::nullopt,
                                                           decoder, key_builder, ctx));
@@ -466,9 +466,9 @@ Status AvroDirectDecoder::DecodeAvroToBuilder(const ::avro::NodePtr& avro_node,
     return DecodeFieldToBuilder(avro_node, projection, decoder, array_builder, ctx);
 }
 
-Status AvroDirectDecoder::ReserveBuilderCapacity(arrow::ArrayBuilder* array_builder,
-                                                 int64_t capacity) {
-    return ReserveBuilderCapacityImpl(array_builder, capacity);
+Status AvroDirectDecoder::ReserveBuilderCapacity(int64_t capacity,
+                                                 arrow::ArrayBuilder* array_builder) {
+    return ReserveBuilderCapacityImpl(capacity, array_builder);
 }
 
 }  // namespace paimon::avro

src/paimon/format/avro/avro_direct_decoder.h

@@ -83,10 +83,10 @@ class AvroDirectDecoder {
                                       DecodeContext* ctx);
 
     /// Reserve slots for a builder and any struct children with the same cardinality.
-    /// @param array_builder Builder to reserve.
     /// @param capacity Number of additional values to append.
+    /// @param array_builder Builder to reserve.
     /// @return Status::OK if all reservations succeed.
-    static Status ReserveBuilderCapacity(arrow::ArrayBuilder* array_builder, int64_t capacity);
+    static Status ReserveBuilderCapacity(int64_t capacity, arrow::ArrayBuilder* array_builder);
 };
 
 }  // namespace paimon::avro

src/paimon/format/avro/avro_direct_encoder_decoder_test.cpp

@@ -373,7 +373,7 @@ TEST_F(AvroDirectEncoderDecoderTest, TestReserveBuilderCapacity) {
     std::unique_ptr<arrow::ArrayBuilder> builder = std::move(builder_result).ValueOrDie();
 
     constexpr int64_t capacity = 1024;
-    ASSERT_OK(AvroDirectDecoder::ReserveBuilderCapacity(builder.get(), capacity));
+    ASSERT_OK(AvroDirectDecoder::ReserveBuilderCapacity(capacity, builder.get()));
 
     auto* root_builder = checked_cast<arrow::StructBuilder*>(builder.get());
     ASSERT_GE(root_builder->capacity(), capacity);

src/paimon/format/avro/avro_file_batch_reader.cpp

@@ -18,6 +18,7 @@
 
 #include "paimon/format/avro/avro_file_batch_reader.h"
 
+#include <cassert>
 #include <limits>
 #include <memory>
 #include <utility>
@@ -112,7 +113,7 @@ Result<BatchReader::ReadBatch> AvroFileBatchReader::NextBatch() {
             }
             if (array_builder_->length() == 0) {
                 PAIMON_RETURN_NOT_OK(
-                    AvroDirectDecoder::ReserveBuilderCapacity(array_builder_.get(), batch_size_));
+                    AvroDirectDecoder::ReserveBuilderCapacity(batch_size_, array_builder_.get()));
             }
             reader_->decr();
             PAIMON_RETURN_NOT_OK(AvroDirectDecoder::DecodeAvroToBuilder(
@@ -127,9 +128,7 @@ Result<BatchReader::ReadBatch> AvroFileBatchReader::NextBatch() {
         }
         PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> array,
                                           array_builder_->Finish());
-#ifndef NDEBUG
-        PAIMON_RETURN_NOT_OK_FROM_ARROW(array->Validate());
-#endif
+        assert(array->Validate().ok());
         std::unique_ptr<ArrowArray> c_array = std::make_unique<ArrowArray>();
         std::unique_ptr<ArrowSchema> c_schema = std::make_unique<ArrowSchema>();
         PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*array, c_array.get(), c_schema.get()));