Skip to content

Commit 172d931

Browse files
authored
test: allow external_access_plan run on windows (#13531)
* test: allow external_access_plan run on windows * fix: avoid '%' in path for external_access_plan.rs and remove temp file * minor: remove singleton TEST_DATA for external_access_plan
1 parent 7553b3b commit 172d931

File tree

3 files changed

+45
-38
lines changed

3 files changed

+45
-38
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ datafusion/sqllogictests/test_files/tpch/data/*
6464
# Scratch temp dir for sqllogictests
6565
datafusion/sqllogictest/test_files/scratch*
6666

67+
# temp file for core
68+
datafusion/core/*.parquet
69+
6770
# rat
6871
filtered_rat.txt
6972
rat.txt

datafusion/core/tests/parquet/external_access_plan.rs

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ use datafusion_physical_plan::ExecutionPlan;
3333
use parquet::arrow::arrow_reader::{RowSelection, RowSelector};
3434
use parquet::arrow::ArrowWriter;
3535
use parquet::file::properties::WriterProperties;
36-
use std::sync::{Arc, OnceLock};
36+
use std::path::Path;
37+
use std::sync::Arc;
3738
use tempfile::NamedTempFile;
3839

3940
#[tokio::test]
@@ -314,12 +315,19 @@ impl TestFull {
314315

315316
let TestData {
316317
_temp_file: _,
317-
schema,
318-
file_name,
319-
file_size,
318+
ref schema,
319+
ref file_name,
320+
ref file_size,
320321
} = get_test_data();
321322

322-
let mut partitioned_file = PartitionedFile::new(file_name, *file_size);
323+
let new_file_name = if cfg!(target_os = "windows") {
324+
// Windows path separator is different from Unix
325+
file_name.replace("\\", "/")
326+
} else {
327+
file_name.clone()
328+
};
329+
330+
let mut partitioned_file = PartitionedFile::new(new_file_name, *file_size);
323331

324332
// add the access plan, if any, as an extension
325333
if let Some(access_plan) = access_plan {
@@ -355,6 +363,8 @@ impl TestFull {
355363
pretty_format_batches(&results).unwrap()
356364
);
357365

366+
std::fs::remove_file(file_name).unwrap();
367+
358368
Ok(MetricsFinder::find_metrics(plan.as_ref()).unwrap())
359369
}
360370
}
@@ -369,45 +379,41 @@ struct TestData {
369379
file_size: u64,
370380
}
371381

372-
static TEST_DATA: OnceLock<TestData> = OnceLock::new();
373-
374382
/// Return a parquet file with 2 row groups each with 5 rows
375-
fn get_test_data() -> &'static TestData {
376-
TEST_DATA.get_or_init(|| {
377-
let scenario = Scenario::UTF8;
378-
let row_per_group = 5;
383+
fn get_test_data() -> TestData {
384+
let scenario = Scenario::UTF8;
385+
let row_per_group = 5;
379386

380-
let mut temp_file = tempfile::Builder::new()
381-
.prefix("user_access_plan")
382-
.suffix(".parquet")
383-
.tempfile()
384-
.expect("tempfile creation");
387+
let mut temp_file = tempfile::Builder::new()
388+
.prefix("user_access_plan")
389+
.suffix(".parquet")
390+
.tempfile_in(Path::new(""))
391+
.expect("tempfile creation");
385392

386-
let props = WriterProperties::builder()
387-
.set_max_row_group_size(row_per_group)
388-
.build();
393+
let props = WriterProperties::builder()
394+
.set_max_row_group_size(row_per_group)
395+
.build();
389396

390-
let batches = create_data_batch(scenario);
391-
let schema = batches[0].schema();
397+
let batches = create_data_batch(scenario);
398+
let schema = batches[0].schema();
392399

393-
let mut writer =
394-
ArrowWriter::try_new(&mut temp_file, schema.clone(), Some(props)).unwrap();
400+
let mut writer =
401+
ArrowWriter::try_new(&mut temp_file, schema.clone(), Some(props)).unwrap();
395402

396-
for batch in batches {
397-
writer.write(&batch).expect("writing batch");
398-
}
399-
writer.close().unwrap();
403+
for batch in batches {
404+
writer.write(&batch).expect("writing batch");
405+
}
406+
writer.close().unwrap();
400407

401-
let file_name = temp_file.path().to_string_lossy().to_string();
402-
let file_size = temp_file.path().metadata().unwrap().len();
408+
let file_name = temp_file.path().to_string_lossy().to_string();
409+
let file_size = temp_file.path().metadata().unwrap().len();
403410

404-
TestData {
405-
_temp_file: temp_file,
406-
schema,
407-
file_name,
408-
file_size,
409-
}
410-
})
411+
TestData {
412+
_temp_file: temp_file,
413+
schema,
414+
file_name,
415+
file_size,
416+
}
411417
}
412418

413419
/// Return the total value of the specified metric name

datafusion/core/tests/parquet/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ use std::sync::Arc;
4343
use tempfile::NamedTempFile;
4444

4545
mod custom_reader;
46-
// Don't run on windows as tempfiles don't seem to work the same
47-
#[cfg(not(target_os = "windows"))]
4846
mod external_access_plan;
4947
mod file_statistics;
5048
#[cfg(not(target_family = "windows"))]

0 commit comments

Comments
 (0)