[C++][Parquet] OpenTelemetry possibly reporting wrong column in ColumnReaderImpl::ReadColumn

[EnricoMi]

Describe the bug, including details regarding any error messages, version, and platform.

ColumnReaderImpl::ReadColumn reports information on the column being read in

arrow/cpp/src/parquet/arrow/reader.cc

Lines 282 to 292 in 0bd8def

	#ifdef ARROW_WITH_OPENTELEMETRY
	std::string column_name = reader_->metadata()->schema()->Column(i)->name();
	std::string phys_type =
	TypeToString(reader_->metadata()->schema()->Column(i)->physical_type());
	::arrow::util::tracing::Span span;
	START_SPAN(span, "parquet::arrow::read_column",
	{{"parquet.arrow.columnindex", i},
	{"parquet.arrow.columnname", column_name},
	{"parquet.arrow.physicaltype", phys_type},
	{"parquet.arrow.records_to_read", records_to_read}});
	#endif

However, FileReaderImpl::DecodeRowGroups uses column index i that refers to the reader index, not the column index:

arrow/cpp/src/parquet/arrow/reader.cc

Line 1386 in 0bd8def

RETURN_NOT_OK(ReadColumn(static_cast<int>(i), row_groups, reader.get(), &column));

So the reported column information is wrong when only a subset of columns is read, or when nested schemas exist.

Could use manifest_.GetFieldIndices(column_indices)[i], but that does not work with nested schemas. Cannot use column_indices[i] here, as it does not align with reader indices either.

Note: Use of i earlier in this method is fixed in #51362:

arrow/cpp/src/parquet/arrow/reader.cc

Lines 279 to 280 in 0bd8def

	records_to_read +=
	reader_->metadata()->RowGroup(row_group)->ColumnChunk(i)->num_values();

Component(s)

C++

[adamreeve]

I think we can map i to the top-level column column index with manifest_.GetFieldIndices as you'd done originally in #51362. But we'd need to get the name from the top-level schema node instead of Column(i). And phys_type could be set if the column is a primitive, otherwise we leave it empty?

[gitedmond]

I reproduced this and have a fix ready. It follows @adamreeve’s suggestion: map each selected Parquet leaf index to its top-level field, use that field’s index and name in the tracing attributes, and leave the physical type empty for nested fields. I applied the same mapping to direct row-group column reads.

I added OpenTelemetry tests for flat, nested, reordered, full-schema, and direct reads. All five fail before the fix and pass afterward, and the existing Parquet tests still pass.

The change is currently based on #51362. Once that merges, I can rebase onto main and open the PR. Does this approach sound right?

[adamreeve]

Sounds good thanks @gitedmond, I've merged #51362 so you can rebase now.

[gitedmond]

Thanks! I've rebased onto main and opened #51401.