[NSE-265] Reserve enough memory before UnsafeAppend in builder

[JkSelf]

What changes were proposed in this pull request?

Related to #265

fixes: #241

shuffle builder use UnsafeAppend API for better performance. it
tries to reserve enough space based on results of last recordbatch,
this maybe not buggy if there's a dense recordbatch after a sparse one.

this patch adds below fixes:

• adds Reset() after Finish() in builder
• reserve length for offset_builder in binary builder

A further clean up on the reservation logic should be needed.

How was this patch tested?

locally verified

[github-actions]

#265

[JkSelf]

@zhouyuan You can have a try with this PR.

[zhouyuan]

@JkSelf
thanks a lot for digging into this bug! this seems to be one hidden bug from codes committed while ago
I made some quick tests on my env, it looks like we are missing one call to reserve size in the offsets_builder_. below patch can make it work also. we may need to check Reserve() the right size in somewhere.

diff --git a/native-sql-engine/cpp/src/shuffle/splitter.cc b/native-sql-engine/cpp/src/shuffle/splitter.cc
index 2eebb9ae..31856e36 100644
--- a/native-sql-engine/cpp/src/shuffle/splitter.cc
+++ b/native-sql-engine/cpp/src/shuffle/splitter.cc
@@ -1047,6 +1047,7 @@ arrow::Status Splitter::AppendBinary(
       offset_type length;
       auto value = src_arr->GetValue(row, &length);
       const auto& builder = dst_builders[partition_id_[row]];
+      RETURN_NOT_OK(builder->Reserve(1));
       RETURN_NOT_OK(builder->ReserveData(length));
       builder->UnsafeAppend(value, length);
     }
@@ -1056,6 +1057,7 @@ arrow::Status Splitter::AppendBinary(
         offset_type length;
         auto value = src_arr->GetValue(row, &length);
         const auto& builder = dst_builders[partition_id_[row]];
+        RETURN_NOT_OK(builder->Reserve(1));
         RETURN_NOT_OK(builder->ReserveData(length));
         builder->UnsafeAppend(value, length);
       } else {

[rui-mo]

verified q67 on my env.