fix(parquet): keep virtual columns in the schema reported with a schema hint

[bharadwaj-pendyala]

Which issue does this PR close?

• Closes Providing schema hint to ArrowReaderMetadata causes resulting schema to omit virtual fields #11046.

Rationale for this change

ArrowReaderMetadata::with_supplied_schema already treats virtual columns as extra fields on top of the hint. It passes them to parquet_to_arrow_field_levels_with_virtual, and its own length check reads supplied_schema.fields().len() + virtual_columns.len(). Then it returns schema: supplied_schema, which doesn't have them.

So the two branches of try_new disagree. Without a hint you get a schema with the virtual fields in it; add a hint and they vanish, even though the reader still decodes them. On the file from the issue that means metadata.schema() reports one field while every batch that metadata produces has two:

metadata.schema().fields()  [value]
batch.schema().fields()     [value, row_number]

@limenilbuz asked for either of two behaviours: include the virtual columns in the reported schema, or stop erroring when the hint itself contains them. This does the first. The second is a change to what with_schema accepts, and the length check here already assumes virtual fields live outside the hint, so the first is the one that makes the function agree with itself.

What changes are included in this PR?

The returned schema is now the supplied fields followed by the virtual fields, keeping the supplied schema's key/value metadata. When no virtual columns are requested the supplied schema is returned untouched, so nothing changes for that path.

parquet_to_arrow_field_levels_with_virtual appends virtual columns to the root in the order given and clones them unchanged (parquet/src/arrow/schema/mod.rs:223), so appending them here in the same order lines the reported schema up with field_levels.

Are these changes tested?

Yes. test_supplied_schema_keeps_virtual_columns builds metadata from a hint plus two virtual fields and checks the field order, that the hint's schema metadata survives, and that metadata.schema() agrees with the fields of the batch the reader emits. It fails on f9e02ba with:

left: [Field { name: "value", data_type: Int64 }]
right: [Field { name: "value", .. }, Field { name: "row_number", .. }, Field { name: "row_group_index", .. }]

cargo test -p parquet --lib is 1381 passed, 0 failed. cargo fmt --all -- --check and cargo clippy -p parquet --all-targets are both clean.

Are there any user-facing changes?

ArrowReaderMetadata::schema(), and the builder schema derived from it, gain the virtual fields when a hint and virtual columns are combined. That's the fix, but it is a field-count change on a public accessor, so it's worth calling out. Physical column indices are unaffected and no crate in the tree combines those two options.

One thing I left alone: with an explicit projection the async reader's schema() drops virtual fields while the batches still carry them. That reproduces with and without a schema hint, so it's a separate bug from this one and I didn't touch it here.

[Jefffrey]

i notice here we construct via field_levels:

arrow-rs/parquet/src/arrow/schema/mod.rs

Lines 75 to 99 in 0a59795

	pub(crate) fn parquet_to_arrow_schema_and_fields(
	parquet_schema: &SchemaDescriptor,
	mask: ProjectionMask,
	key_value_metadata: Option<&Vec<KeyValue>>,
	virtual_columns: &[FieldRef],
	) -> Result<(Schema, Option<ParquetField>)> {
	let mut metadata = parse_key_value_metadata(key_value_metadata).unwrap_or_default();
	let maybe_schema = metadata
	.remove(super::ARROW_SCHEMA_META_KEY)
	.map(|value| get_arrow_schema_from_metadata(&value))
	.transpose()?;
	// Add the Arrow metadata to the Parquet metadata skipping keys that collide
	if let Some(arrow_schema) = &maybe_schema {
	arrow_schema.metadata().iter().for_each(|(k, v)| {
	metadata.entry(k.clone()).or_insert_with(|| v.clone());
	});
	}
	let hint = maybe_schema.as_ref().map(|s| s.fields());
	let field_levels =
	parquet_to_arrow_field_levels_with_virtual(parquet_schema, mask, hint, virtual_columns)?;
	let schema = Schema::new_with_metadata(field_levels.fields, metadata);
	Ok((schema, field_levels.levels))
	}

• line 97

is this equivalent?

[bharadwaj-pendyala]

Yes, with one exception. parquet_to_arrow_field_levels_with_virtual rejects a name mismatch at complex.rs:325, and the loop just above compares data type, nullability and metadata, so the top-level fields agree. Switched to it in 53f7c9f.

The exception is the deprecated dict_id/dict_is_ordered on a nested field. With a legacy REPEATED BINARY element list and a dictionary element hint carrying 42/true, field_levels.fields reports 0/false, because into_list rebuilds the element with Field::new at complex.rs:69. Field equality ignores both, so the two still compare equal, and I don't think it's worth keeping the old chaining over.