Skip to content

Commit 2d720ca

Browse files
committed
replace slt with deterministic integration test
1 parent 31201a6 commit 2d720ca

2 files changed

Lines changed: 90 additions & 95 deletions

File tree

‎datafusion/core/tests/parquet/filter_pushdown.rs‎

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@
2626
//! select * from data limit 10;
2727
//! ```
2828
29-
use arrow::array::{ArrayRef, Int32Array, StringArray};
29+
use arrow::array::{ArrayRef, Int32Array, Int64Array, StringArray};
3030
use arrow::compute::concat_batches;
3131
use arrow::error::ArrowError;
3232
use arrow::record_batch::RecordBatch;
33+
use datafusion::execution::SessionStateBuilder;
3334
use datafusion::physical_plan::metrics::{MetricValue, MetricsSet};
3435
use datafusion::physical_plan::{collect, displayable};
3536
use datafusion::prelude::{
@@ -809,3 +810,91 @@ async fn pushed_down_predicate_reports_the_original_error() {
809810
"expected the original cast error, got {root:?}"
810811
);
811812
}
813+
814+
/// A MIN/MAX dynamic filter must survive a file that lacks the aggregated
815+
/// column. That file's Partial aggregate evaluates to a typed null
816+
/// (`Int64(NULL)`), which must not replace the real MIN published later.
817+
///
818+
/// With one partition the files are read in order `01_missing`, `02_high`,
819+
/// `03_low`, so the bad interleaving happens on every run. Without the fix,
820+
/// the MIN bound stays null, the filter becomes `latency_ms > 204`, and
821+
/// `03_low` is pruned, returning MIN = 200.
822+
#[tokio::test]
823+
async fn aggregate_dynamic_filter_ignores_typed_null_bound() {
824+
let tempdir = TempDir::new_in(Path::new(".")).unwrap();
825+
let dir = tempdir.path();
826+
827+
let write = |name: &str, col_name: &str, array: ArrayRef| {
828+
let batch = RecordBatch::try_from_iter([(col_name, array)]).unwrap();
829+
let file = File::create(dir.join(name)).unwrap();
830+
let mut writer = ArrowWriter::try_new(file, batch.schema(), None).unwrap();
831+
writer.write(&batch).unwrap();
832+
writer.close().unwrap();
833+
};
834+
write(
835+
"01_missing.parquet",
836+
"host",
837+
Arc::new(StringArray::from(vec!["h1"; 5])),
838+
);
839+
write(
840+
"02_high.parquet",
841+
"latency_ms",
842+
Arc::new(Int64Array::from(vec![200, 201, 202, 203, 204])),
843+
);
844+
write(
845+
"03_low.parquet",
846+
"latency_ms",
847+
Arc::new(Int64Array::from(vec![100, 101, 102, 103, 104])),
848+
);
849+
850+
// One partition keeps the file order fixed. Drop the rule that would
851+
// otherwise fold Partial/Final into a Single aggregate, which does not
852+
// create a dynamic filter.
853+
let default_state = SessionStateBuilder::new().with_default_features().build();
854+
let rules = default_state
855+
.physical_optimizers()
856+
.iter()
857+
.filter(|rule| rule.name() != "CombinePartialFinalAggregate")
858+
.cloned()
859+
.collect();
860+
let config = SessionConfig::new().with_target_partitions(1);
861+
let state = SessionStateBuilder::new()
862+
.with_config(config)
863+
.with_default_features()
864+
.with_physical_optimizer_rules(rules)
865+
.build();
866+
let ctx = SessionContext::new_with_state(state);
867+
868+
ctx.sql(&format!(
869+
"CREATE EXTERNAL TABLE t (latency_ms BIGINT, host VARCHAR) \
870+
STORED AS PARQUET LOCATION '{}/'",
871+
dir.display()
872+
))
873+
.await
874+
.unwrap();
875+
876+
let df = ctx
877+
.sql("SELECT min(latency_ms), max(latency_ms) FROM t")
878+
.await
879+
.unwrap();
880+
let plan = df.create_physical_plan().await.unwrap();
881+
let plan_str = displayable(plan.as_ref()).indent(false).to_string();
882+
assert!(
883+
plan_str.contains("mode=Partial") && plan_str.contains("DynamicFilter"),
884+
"expected a Partial aggregate with a dynamic filter:\n{plan_str}"
885+
);
886+
887+
let batches = collect(plan, ctx.task_ctx()).await.unwrap();
888+
let batch = concat_batches(&batches[0].schema(), &batches).unwrap();
889+
let min = batch
890+
.column(0)
891+
.as_any()
892+
.downcast_ref::<Int64Array>()
893+
.unwrap();
894+
let max = batch
895+
.column(1)
896+
.as_any()
897+
.downcast_ref::<Int64Array>()
898+
.unwrap();
899+
assert_eq!((min.value(0), max.value(0)), (100, 204), "\n{plan_str}");
900+
}

‎datafusion/sqllogictest/test_files/push_down_filter_regression.slt‎

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -571,100 +571,6 @@ reset datafusion.optimizer.max_passes;
571571
statement ok
572572
drop table agg_filter_pushdown;
573573

574-
########
575-
# MIN/MAX dynamic filter over a schema-evolved dataset.
576-
#
577-
# One of the files does not contain the aggregated column at all, so that
578-
# partition's Partial aggregate evaluates to a typed null (Int64(NULL)) rather
579-
# than ScalarValue::Null. Merging that bound into the shared dynamic filter
580-
# bound must leave any real MIN/MAX from other partitions untouched. If the
581-
# typed null were compared as a value, it would win the MIN comparison, the
582-
# filter would collapse to `latency_ms > 204`, and the file holding the true
583-
# minimum would be pruned, returning 200 instead of 100.
584-
#
585-
# The wrong answer needs the file holding the minimum to be opened after the
586-
# other two partitions have published their bounds, so that file is named to
587-
# sort last. Use as many partitions as files so every file is read by its own
588-
# partition. The outcome still depends on scheduling, so the query is repeated
589-
# a few times; `scalar_min_max_ignore_typed_nulls` in
590-
# datafusion/physical-plan/src/aggregates/aggregate_stream.rs covers the
591-
# merge deterministically.
592-
593-
statement ok
594-
set datafusion.execution.target_partitions = 8;
595-
596-
statement ok
597-
COPY (
598-
SELECT * FROM (VALUES ('h1'), ('h1'), ('h1'), ('h1'), ('h1')) AS t(host)
599-
) TO 'test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/01_missing.parquet'
600-
STORED AS PARQUET;
601-
602-
statement ok
603-
COPY (
604-
SELECT * FROM (VALUES (200), (201), (202), (203), (204)) AS t(latency_ms)
605-
) TO 'test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/02_high.parquet'
606-
STORED AS PARQUET;
607-
608-
statement ok
609-
COPY (
610-
SELECT * FROM (VALUES (100), (101), (102), (103), (104)) AS t(latency_ms)
611-
) TO 'test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/03_low.parquet'
612-
STORED AS PARQUET;
613-
614-
statement ok
615-
CREATE EXTERNAL TABLE agg_dyn_schema_evolution (latency_ms BIGINT, host VARCHAR)
616-
STORED AS PARQUET
617-
LOCATION 'test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/';
618-
619-
# Sanity check that the plan uses a Partial/Final aggregate with a dynamic
620-
# filter pushed into the scan, and one partition per file.
621-
query TT
622-
explain select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
623-
----
624-
physical_plan
625-
01)AggregateExec: mode=Final, gby=[], aggr=[min(agg_dyn_schema_evolution.latency_ms), max(agg_dyn_schema_evolution.latency_ms)]
626-
02)--CoalescePartitionsExec
627-
03)----AggregateExec: mode=Partial, gby=[], aggr=[min(agg_dyn_schema_evolution.latency_ms), max(agg_dyn_schema_evolution.latency_ms)]
628-
04)------DataSourceExec: file_groups={3 groups: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/01_missing.parquet], [WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/02_high.parquet], [WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_regression/agg_dyn_schema_evolution/03_low.parquet]]}, projection=[latency_ms], file_type=parquet, predicate=DynamicFilter [ empty ], dynamic_rg_pruning=eligible
629-
630-
query II
631-
select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
632-
----
633-
100 204
634-
635-
query II
636-
select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
637-
----
638-
100 204
639-
640-
query II
641-
select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
642-
----
643-
100 204
644-
645-
query II
646-
select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
647-
----
648-
100 204
649-
650-
query II
651-
select min(latency_ms), max(latency_ms) from agg_dyn_schema_evolution;
652-
----
653-
100 204
654-
655-
query I
656-
select min(latency_ms) from agg_dyn_schema_evolution;
657-
----
658-
100
659-
660-
query I
661-
select max(latency_ms) from agg_dyn_schema_evolution;
662-
----
663-
204
664-
665-
statement ok
666-
drop table agg_dyn_schema_evolution;
667-
668574
# Config reset
669575

670576
# The SLT runner sets `target_partitions` to 4 instead of using the default, so

0 commit comments

Comments
 (0)