[GitHub] [arrow] pitrou commented on a diff in pull request #37467: GH-37437: [C++] Fix MakeArrayOfNull for list array with large string values type

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou commented on code in PR #37467:
URL: https://github.com/apache/arrow/pull/37467#discussion_r1311582571


##########
cpp/src/arrow/array/validate.cc:
##########
@@ -713,8 +713,10 @@ struct ValidateArrayImpl {
     }
 
     // An empty list array can have 0 offsets
-    const auto required_offsets = (data.length > 0) ? data.length + data.offset + 1 : 0;
     const auto offsets_byte_size = data.buffers[1]->size();
+    const auto required_offsets = ((data.length > 0) || (offsets_byte_size > 0))
+                                      ? data.length + data.offset + 1
+                                      : 0;
     if (offsets_byte_size / static_cast<int32_t>(sizeof(offset_type)) <
         required_offsets) {
       return Status::Invalid("Offsets buffer size (bytes): ", offsets_byte_size,

Review Comment:
   > @pitrou do you know if this "empty list array can have 0 offsets" also applies to other types that have offsets, i.e. string types? (this was added in #6230)
   
   I don't think so. In any case, the allowance for list types is IMO historical.
   
   > At the moment the offsets buffer size is not checked if the length of the array is 0, but I changed this to actually check it if the offset buffer has _some_ length (so assuming that _if_ there is an offsets buffer with a non-zero length, it should have a correct length, also if the array itself has length 0)
   
   That sounds like a good approach.



-- 
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-unsubscribe@arrow.apache.org

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