zjw1111 commented on code in PR #198:
URL: https://github.com/apache/paimon-cpp/pull/198#discussion_r3795288649
##########
src/paimon/common/utils/arrow/arrow_utils.cpp:
##########
@@ -363,6 +375,25 @@ Status ArrowUtils::InnerCheckNullabilityMatch(const
std::shared_ptr<arrow::Field
auto list_array = checked_pointer_cast<arrow::ListArray>(data);
PAIMON_RETURN_NOT_OK(
InnerCheckNullabilityMatch(list_type->value_field(),
list_array->values()));
+ } else if (type->id() == arrow::Type::FIXED_SIZE_LIST) {
+ auto vector_type =
checked_pointer_cast<arrow::FixedSizeListType>(field->type());
+ auto vector_array =
checked_pointer_cast<arrow::FixedSizeListArray>(data);
+ const std::shared_ptr<arrow::Array>& values = vector_array->values();
+ if (values->null_count() != 0) {
+ int32_t vector_length = vector_type->list_size();
+ for (int64_t i = 0; i < vector_array->length(); ++i) {
+ if (vector_array->IsNull(i)) {
+ continue;
+ }
+ int64_t value_offset = (vector_array->offset() + i) *
vector_length;
+ for (int32_t j = 0; j < vector_length; ++j) {
+ if (values->IsNull(value_offset + j)) {
Review Comment:
This element scan can read out of bounds on the write path.
`AbstractFileStoreWrite::Write` builds `data` via
`arrow::ImportArray(batch->GetData(),
arrow::struct_(write_schema_->fields()))`, and Arrow's C importer does not
verify that a `FixedSizeList` child holds `length * list_size` values
(`ArrayImporter::Visit(const FixedSizeListType&)` only checks the child/buffer
counts). No `Validate()` runs before `CheckNullabilityMatch` either. So if a
caller passes an array whose child is shorter than the declared dimension,
`values->IsNull(value_offset + j)` indexes past the child's validity bitmap.
Could you add a structural check before the scan? `data->Validate()` in
`AbstractFileStoreWrite::Write` would be enough — it is O(1) for this layout
and already rejects `values->length() < (offset + length) * list_size`. It
would also turn the opposite case (child longer than declared, currently
reinterpreted silently with the wrong dimension) into a clear error instead of
corrupt data.
--
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]
