felipecrv commented on code in PR #41894:
URL: https://github.com/apache/arrow/pull/41894#discussion_r1635688147


##########
cpp/src/arrow/util/formatting.h:
##########
@@ -652,5 +654,82 @@ class StringFormatter<MonthDayNanoIntervalType> {
   }
 };
 
+template <>
+class StringFormatter<BaseListScalar> {
+ public:
+  explicit StringFormatter(const DataType* type) : type_(type) {}
+
+  using value_type = BaseListScalar;
+
+  template <typename Appender>
+  Return<Appender> operator()(const BaseListScalar& value, Appender&& append) {
+    std::stringstream ss;
+    ss << type_->ToString() << "[";
+    for (int64_t i = 0; i < value.value->length(); i++) {
+      if (i > 0) ss << ", ";
+      auto item = value.value->GetScalar(i);

Review Comment:
   We shouldn't allocate one scalar per entry of an array in a loop. The 
virtual-dispatching string formatters should have an initialization phase and 
then you ask it (via a virtual method) to convert value at position `i` and 
append it into `std::string *out_appender`.
   
   I will sketch this out and open a draft PR, so you can checkout my branch 
and implement it for all the nested types. Is that cool with you?



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to