Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parquet: clear metadata and project fields of ParquetRecordBatchStream::schema #5135

Merged
merged 8 commits into from
Dec 5, 2023
Merged

Parquet: clear metadata and project fields of ParquetRecordBatchStream::schema #5135

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
84be336
Parquet: clear metadata of ParquetRecordBatchStream::schema
Jefffrey Nov 28, 2023
fd662ad
Revert "Parquet: clear metadata of ParquetRecordBatchStream::schema"
Jefffrey Nov 28, 2023
ef9601e
Document expected behaviour
Jefffrey Nov 28, 2023
8e6489d
Revert "Document expected behaviour"
Jefffrey Nov 28, 2023
91f3780
Reapply "Parquet: clear metadata of ParquetRecordBatchStream::schema"
Jefffrey Nov 28, 2023
af4f2d1
ParquetRecordBatchStream should strip schema metadata and respect pro…
Jefffrey Nov 29, 2023
1d96883
Merge branch 'master' into clear_parquet_record_batch_stream_schema_m…
Jefffrey Nov 30, 2023
7ea1e1b
Fix projection of nested fields
Jefffrey Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion parquet/src/arrow/async_reader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,15 @@ impl<T> std::fmt::Debug for ParquetRecordBatchStream<T> {
}

impl<T> ParquetRecordBatchStream<T> {
/// Returns the [`SchemaRef`] for this parquet file
/// Returns the [`SchemaRef`] for this parquet file.
///
/// Note that unlike its synchronous counterpart [`ParquetRecordBatchReader`], the [`SchemaRef`]
/// returned here will contain the original metadata, whereas [`ParquetRecordBatchReader`]
/// strips this metadata.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about this, they use the same logic? The difference is that the returned RecordBatch lack the metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see how schema is created for ParquetRecordBatchReader here:

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

Line 609 in 093a10e

schema: Arc::new(Schema::new(levels.fields.clone())),

And here:

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

Lines 622 to 625 in 093a10e

let schema = match array_reader.get_data_type() {
ArrowType::Struct(ref fields) => Schema::new(fields.clone()),
_ => unreachable!("Struct array reader's data type is not struct!"),
};

They only account for the fields, ignoring the metadata.

///
/// As such, this schema is the same as [`ParquetRecordBatchStreamBuilder::schema`] which also
/// contains the metadata, but differs from the [`RecordBatch`]es produced which have the metadata
/// in their [`RecordBatch::schema`] stripped.
pub fn schema(&self) -> &SchemaRef {
&self.schema
}
Expand Down
Loading