HuaHuaY commented on code in PR #50271:
URL: https://github.com/apache/arrow/pull/50271#discussion_r3621258284
##########
cpp/src/parquet/arrow/reader.cc:
##########
@@ -725,13 +725,59 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public
ListReader<int32_t> {
DCHECK_EQ(data->buffers.size(), 2);
DCHECK_EQ(field()->type()->id(), ::arrow::Type::FIXED_SIZE_LIST);
const auto& type =
checked_cast<::arrow::FixedSizeListType&>(*field()->type());
- const int32_t* offsets = reinterpret_cast<const
int32_t*>(data->buffers[1]->data());
- for (int x = 1; x <= data->length; x++) {
- int32_t size = offsets[x] - offsets[x - 1];
- if (size != type.list_size()) {
- return Status::Invalid("Expected all lists to be of size=",
type.list_size(),
- " but index ", x, " had size=", size);
+ const auto* offsets = reinterpret_cast<const
int32_t*>(data->buffers[1]->data());
+ const int32_t list_size = type.list_size();
+ auto validate_offsets = [&](int64_t start, int64_t length, bool valid) ->
Status {
+ const int32_t expected_size = valid ? list_size : 0;
+ std::span<const int32_t> run_offsets(offsets + start,
+ static_cast<size_t>(length + 1));
+ const auto first_invalid_offset = std::ranges::adjacent_find(
+ run_offsets,
+ [&](int32_t left, int32_t right) { return right - left !=
expected_size; });
+ if (first_invalid_offset != run_offsets.end()) {
+ const int64_t x =
+ start + std::ranges::distance(run_offsets.begin(),
first_invalid_offset);
+ const int32_t size = offsets[x + 1] - offsets[x];
+ if (valid) {
+ return Status::Invalid("Expected all lists to be of size=",
list_size,
+ " but index ", x + 1, " had size=", size);
+ }
+ return Status::Invalid("Expected null fixed-size list at index ", x +
1,
+ " to have no child values but had size=", size);
}
+ return Status::OK();
+ };
+ if (data->GetNullCount() != 0) {
+ // Rebuild the child array run-by-run so null fixed-size list slots still
+ // contribute list_size child values in the final layout.
+ ::arrow::ArrayVector child_arrays;
+
+ auto visit_run = [&](int64_t start, int64_t length, bool valid) ->
Status {
+ RETURN_NOT_OK(validate_offsets(start, length, valid));
+
+ const int64_t child_length = length * list_size;
+ // Valid runs reuse the decoded child slice; null runs materialize null
+ // children to preserve the fixed-size list shape.
+ if (!valid) {
+ ARROW_ASSIGN_OR_RAISE(
+ auto null_array,
+ ::arrow::MakeArrayOfNull(type.value_type(), child_length,
ctx_->pool));
+ child_arrays.push_back(std::move(null_array));
+ return Status::OK();
+ }
+ child_arrays.push_back(
+ ::arrow::MakeArray(data->child_data[0]->Slice(offsets[start],
child_length)));
Review Comment:
I don't think this is a good idea. First, the array is created via `Slice`,
and the actual data isn't copied. Second, `Concatenate` handles different array
types automatically, whereas doing it manually would be too much of a hassle.
--
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]
