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.

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

Fix test feature selection so all feature combinations work as expected #6626

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 8 additions & 9 deletions parquet/src/file/metadata/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::schema::types;
use crate::schema::types::SchemaDescriptor;
use crate::thrift::{TCompactSliceInputProtocol, TSerializable};

#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, nice catch! It seems I relied on the default feature set when I added this.

use crate::arrow::async_reader::MetadataFetch;

/// Reads the [`ParquetMetaData`] from a byte stream.
Expand Down Expand Up @@ -321,7 +321,7 @@ impl ParquetMetaDataReader {
///
/// See [`Self::with_prefetch_hint`] for a discussion of how to reduce the number of fetches
/// performed by this function.
#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
pub async fn load_and_finish<F: MetadataFetch>(
mut self,
fetch: F,
Expand All @@ -336,7 +336,7 @@ impl ParquetMetaDataReader {
///
/// See [`Self::with_prefetch_hint`] for a discussion of how to reduce the number of fetches
/// performed by this function.
#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
pub async fn try_load<F: MetadataFetch>(
&mut self,
mut fetch: F,
Expand All @@ -357,12 +357,12 @@ impl ParquetMetaDataReader {

/// Asynchronously fetch the page index structures when a [`ParquetMetaData`] has already
/// been obtained. See [`Self::new_with_metadata()`].
#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
pub async fn load_page_index<F: MetadataFetch>(&mut self, fetch: F) -> Result<()> {
self.load_page_index_with_remainder(fetch, None).await
}

#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
async fn load_page_index_with_remainder<F: MetadataFetch>(
&mut self,
mut fetch: F,
Expand Down Expand Up @@ -513,7 +513,7 @@ impl ParquetMetaDataReader {
/// Return the number of bytes to read in the initial pass. If `prefetch_size` has
/// been provided, then return that value if it is larger than the size of the Parquet
/// file footer (8 bytes). Otherwise returns `8`.
#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
fn get_prefetch_size(&self) -> usize {
if let Some(prefetch) = self.prefetch_hint {
if prefetch > FOOTER_SIZE {
Expand All @@ -523,7 +523,7 @@ impl ParquetMetaDataReader {
FOOTER_SIZE
}

#[cfg(feature = "async")]
#[cfg(all(feature = "async", feature = "arrow"))]
async fn load_metadata<F: MetadataFetch>(
fetch: &mut F,
file_size: usize,
Expand Down Expand Up @@ -851,8 +851,7 @@ mod tests {
}
}

#[cfg(feature = "async")]
#[cfg(test)]
#[cfg(all(feature = "async", feature = "arrow", test))]
mod async_tests {
use super::*;
use bytes::Bytes;
Expand Down
1 change: 1 addition & 0 deletions parquet/tests/arrow_reader/bad_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ fn test_arrow_rs_gh_6229_dict_header() {
}

#[test]
#[cfg(feature = "snap")]
fn test_arrow_rs_gh_6229_dict_levels() {
let err = read_file("ARROW-RS-GH-6229-LEVELS.parquet").unwrap_err();
assert_eq!(
Expand Down
6 changes: 4 additions & 2 deletions parquet/tests/arrow_reader/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ use parquet::arrow::arrow_reader::ArrowReaderBuilder;
fn test_datapage_v1_corrupt_checksum() {
let errors = read_file_batch_errors("datapage_v1-corrupt-checksum.parquet");
assert_eq!(errors, [
Err("Parquet argument error: Parquet error: Page CRC checksum mismatch".to_string()),
Err("Parquet argument error: Parquet error: Page CRC checksum mismatch".to_string()),
Ok(()),
Ok(()),
Err("Parquet argument error: Parquet error: Page CRC checksum mismatch".to_string()),
Err("Parquet argument error: Parquet error: Page CRC checksum mismatch".to_string()),
Err("Parquet argument error: Parquet error: Not all children array length are the same!".to_string())
]);
}
Expand All @@ -41,6 +41,7 @@ fn test_datapage_v1_uncompressed_checksum() {
}

#[test]
#[cfg(feature = "snap")]
fn test_datapage_v1_snappy_compressed_checksum() {
let errors = read_file_batch_errors("datapage_v1-snappy-compressed-checksum.parquet");
assert_eq!(errors, [Ok(()), Ok(()), Ok(()), Ok(()), Ok(())]);
Expand All @@ -52,6 +53,7 @@ fn test_plain_dict_uncompressed_checksum() {
assert_eq!(errors, [Ok(())]);
}
#[test]
#[cfg(feature = "snap")]
fn test_rle_dict_snappy_checksum() {
let errors = read_file_batch_errors("rle-dict-snappy-checksum.parquet");
assert_eq!(errors, [Ok(())]);
Expand Down
Loading