Copilot commented on code in PR #55348:
URL: https://github.com/apache/arrow/pull/55348#discussion_r4429053687
##########
cpp/src/parquet/arrow/schema.cc:
##########
@@ -100,6 +100,24 @@ Status ListToNode(const
std::shared_ptr<::arrow::BaseListType>& type,
const std::string& name, bool nullable, int field_id,
const WriterProperties& properties,
const ArrowWriterProperties& arrow_properties, NodePtr* out)
{
+ if (type->id() == ArrowTypeId::FIXED_SIZE_LIST &&
+ arrow_properties.vector_logical_type() &&
+ type->value_type()->id() != ArrowTypeId::DICTIONARY) {
Review Comment:
The eligibility probe must exclude unsupported element storage types before
calling `FieldToNode`: `FixedSizeList<null>` fails with `NullType Arrow field
must be nullable`, while nested FixedSizeLists repeatedly convert the same
subtree and cause exponential schema-conversion work. An extension backed by
`dictionary<int32, float>` also bypasses the current guard, allowing referenced
NaN/Inf values into a VECTOR file without validation. Unwrap extension storage
and exclude null, dictionary, and nested types before the probe so they use the
ordinary LIST path.
##########
cpp/src/parquet/arrow/schema.cc:
##########
@@ -842,11 +860,53 @@ Status ListToSchemaField(const GroupNode& group,
LevelInfo current_levels,
return Status::OK();
}
+Status VectorToSchemaField(const GroupNode& group, LevelInfo current_levels,
+ SchemaTreeContext* ctx, const SchemaField* parent,
+ SchemaField* out) {
+ // The canonical structure is validated when the GroupNode is created:
+ //
+ // required/optional group name=whatever (VECTOR(N)) {
+ // repeated group name=list {
+ // required TYPE element;
+ // }
+ // }
+ //
+ // yields fixed_size_list<element: TYPE not null>[N] ?nullable
+ const auto& vector_type = checked_cast<const
VectorLogicalType&>(*group.logical_type());
+
+ if (group.is_optional()) {
+ current_levels.IncrementOptional();
+ }
+
+ out->children.resize(1);
+ SchemaField* child_field = &out->children[0];
+
+ ctx->LinkParent(out, parent);
+ ctx->LinkParent(child_field, out);
+
+ int16_t repeated_ancestor_def_level = current_levels.IncrementRepeated();
+ const auto& list_group = checked_cast<const GroupNode&>(*group.field(0));
+ RETURN_NOT_OK(
+ NodeToSchemaField(*list_group.field(0), current_levels, ctx, out,
child_field));
+
+ out->field = ::arrow::field(
+ group.name(),
+ ::arrow::fixed_size_list(child_field->field, vector_type.num_elements()),
+ group.is_optional(), FieldIdMetadata(group.field_id()));
Review Comment:
A required VECTOR inside a nullable struct fails to read when the struct is
null, including `struct_with_required_vector` in the new Python test.
`ListReader` creates a zero-length slot without a validity bitmap, so
`FixedSizeListReader` treats it as a present vector and rejects its length.
Update the reader to pad slots masked by null ancestors while continuing to
reject wrong-sized present vectors.
--
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]
