Skip to content

Commit aff96ef

Browse files
committed
Fix scan2 CI regressions
Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent a8de97b commit aff96ef

4 files changed

Lines changed: 79 additions & 12 deletions

File tree

vortex-datafusion/src/convert/stats.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,30 @@ pub(crate) fn aggregate_stats_to_df(
6464
})
6565
}
6666

67+
pub(crate) fn aggregate_stats_to_df_as(
68+
stats: &[VortexPrecision<Scalar>],
69+
target_dtype: &DType,
70+
) -> VortexResult<ColumnStatistics> {
71+
if stats.len() != BYTE_SIZE_INDEX + 1 {
72+
return Err(vortex_err!(
73+
"expected {} aggregate statistics, got {}",
74+
BYTE_SIZE_INDEX + 1,
75+
stats.len()
76+
));
77+
}
78+
79+
Ok(ColumnStatistics {
80+
null_count: scalar_u64_to_df_usize(&stats[NULL_COUNT_INDEX])?,
81+
min_value: scalar_to_df_as(&stats[MIN_INDEX], target_dtype)?,
82+
max_value: scalar_to_df_as(&stats[MAX_INDEX], target_dtype)?,
83+
// Match the V1 file-statistics path: min/max/null count/size are useful for pruning,
84+
// while sum has narrower type expectations in DataFusion and is not used for pruning.
85+
sum_value: Precision::Absent,
86+
distinct_count: Precision::Absent,
87+
byte_size: scalar_u64_to_df_usize(&stats[BYTE_SIZE_INDEX])?,
88+
})
89+
}
90+
6791
/// Convert a stats set for an array with the given dtype.
6892
#[allow(dead_code)]
6993
pub(crate) fn stats_set_to_df(
@@ -145,6 +169,22 @@ fn scalar_to_df(stat: &VortexPrecision<Scalar>) -> VortexResult<Precision<Scalar
145169
}
146170
}
147171

172+
fn scalar_to_df_as(
173+
stat: &VortexPrecision<Scalar>,
174+
target_dtype: &DType,
175+
) -> VortexResult<Precision<ScalarValue>> {
176+
let cast = |scalar: &Scalar| scalar.cast(target_dtype).and_then(|s| s.try_to_df()).ok();
177+
Ok(match stat {
178+
VortexPrecision::Exact(scalar) => cast(scalar)
179+
.map(Precision::Exact)
180+
.unwrap_or(Precision::Absent),
181+
VortexPrecision::Inexact(scalar) => cast(scalar)
182+
.map(Precision::Inexact)
183+
.unwrap_or(Precision::Absent),
184+
VortexPrecision::Absent => Precision::Absent,
185+
})
186+
}
187+
148188
fn scalar_u64_to_df_usize(stat: &VortexPrecision<Scalar>) -> VortexResult<Precision<usize>> {
149189
match stat {
150190
VortexPrecision::Exact(scalar) => Ok(Precision::Exact(scalar_u64_to_usize(scalar)?)),

vortex-datafusion/src/persistent/format.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ use super::sink::VortexSink;
7171
use super::source::VortexSource;
7272
use crate::PrecisionExt as _;
7373
use crate::convert::TryToDataFusion;
74-
use crate::convert::stats::aggregate_stats_to_df;
74+
use crate::convert::stats::aggregate_stats_to_df_as;
7575
use crate::convert::stats::column_statistics_aggregate_fns;
7676
use crate::convert::stats::is_constant_to_distinct_count;
7777

@@ -755,10 +755,22 @@ async fn infer_scan_plan_stats(table_schema: &SchemaRef, vxf: &VortexFile) -> DF
755755
let mut column_statistics = vec![ColumnStatistics::default(); table_schema.fields().len()];
756756
let mut requested_columns = Vec::new();
757757
let mut requested_exprs = Vec::new();
758+
let mut requested_dtypes = Vec::new();
758759

759760
for (idx, field) in table_schema.fields().iter().enumerate() {
760761
if struct_dtype.find(field.name()).is_some() {
762+
let target_dtype = vxf
763+
.session()
764+
.arrow()
765+
.from_arrow_field(field.as_ref())
766+
.map_err(|e| {
767+
DataFusionError::Execution(format!(
768+
"Failed to derive Vortex DType for field {}: {e}",
769+
field.name()
770+
))
771+
})?;
761772
requested_columns.push(idx);
773+
requested_dtypes.push(target_dtype);
762774
requested_exprs.push(vortex::expr::get_item(
763775
field.name().as_str(),
764776
vortex::expr::root(),
@@ -770,10 +782,15 @@ async fn infer_scan_plan_stats(table_schema: &SchemaRef, vxf: &VortexFile) -> DF
770782
.scan_plan_statistics_many(&requested_exprs, &funcs)
771783
.await
772784
.map_err(|e| DataFusionError::Execution(format!("Failed to infer scan2 stats: {e}")))?;
773-
for (column_idx, stats) in requested_columns.into_iter().zip(stats) {
774-
column_statistics[column_idx] = aggregate_stats_to_df(&stats).map_err(|e| {
775-
DataFusionError::Execution(format!("Failed to convert scan2 stats: {e}"))
776-
})?;
785+
for ((column_idx, target_dtype), stats) in requested_columns
786+
.into_iter()
787+
.zip(requested_dtypes)
788+
.zip(stats)
789+
{
790+
column_statistics[column_idx] =
791+
aggregate_stats_to_df_as(&stats, &target_dtype).map_err(|e| {
792+
DataFusionError::Execution(format!("Failed to convert scan2 stats: {e}"))
793+
})?;
777794
}
778795

779796
let total_byte_size = column_statistics

vortex-duckdb/src/exporter/sequence.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ pub(crate) fn new_exporter_with_flatten(
2424
ctx: &mut ExecutionCtx,
2525
flatten: bool,
2626
) -> VortexResult<Box<dyn ColumnExporter>> {
27-
if flatten {
27+
if flatten || array.ptype().is_unsigned_int() {
28+
// DuckDB's native SequenceVector helper does not support unsigned vectors such as the
29+
// `file_row_number` UBIGINT virtual column, so export those through the primitive path.
2830
return canonical::new_exporter(array.clone().into_array(), cache, ctx);
2931
}
3032
Ok(Box::new(SequenceExporter {

vortex-scan/src/plan/data_source.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use vortex_array::expr::Expression;
2929
use vortex_array::expr::forms::conjuncts;
3030
use vortex_array::expr::stats::Precision;
3131
use vortex_array::expr::stats::Stat;
32+
use vortex_array::expr::transform::coerce_expression;
3233
use vortex_array::extension::datetime::AnyTemporal;
3334
use vortex_array::scalar::Scalar;
3435
use vortex_array::scalar::ScalarValue;
@@ -360,7 +361,8 @@ impl ScanPlanDataSource {
360361
scan_request: DataSourceScanRequest,
361362
target_partitions: usize,
362363
) -> VortexResult<DataSourceScanRef> {
363-
let dtype = scan_request.projection.return_dtype(&self.dtype)?;
364+
let dtype = normalize_scan_expr(scan_request.projection.clone(), &self.dtype)?
365+
.return_dtype(&self.dtype)?;
364366

365367
let meta = ScanMeta {
366368
label: Some("scan2".to_string()),
@@ -483,7 +485,8 @@ impl DataSource for ScanPlanDataSource {
483485
let provider = self.session.scan_scheduler_provider();
484486
let scheduler = provider.scheduler_for_scan(&meta);
485487

486-
let dtype = scan_request.projection.return_dtype(&self.dtype)?;
488+
let dtype = normalize_scan_expr(scan_request.projection.clone(), &self.dtype)?
489+
.return_dtype(&self.dtype)?;
487490
let limit_remaining = scan_request.limit.map(AtomicU64::new).map(Arc::new);
488491

489492
Ok(Arc::new(ScanPlanDataSourceScan {
@@ -706,7 +709,8 @@ fn scan_plan_binding_stream(
706709
session: VortexSession,
707710
request: DataSourceScanRequest,
708711
) -> VortexResult<SendableArrayStream> {
709-
let output_dtype = request.projection.return_dtype(binding.root().dtype())?;
712+
let output_dtype = normalize_scan_expr(request.projection.clone(), binding.root().dtype())?
713+
.return_dtype(binding.root().dtype())?;
710714
let meta = ScanMeta {
711715
label: Some("scan2".to_string()),
712716
};
@@ -2245,12 +2249,12 @@ impl PreparedScanPlan {
22452249
request: &DataSourceScanRequest,
22462250
) -> VortexResult<Self> {
22472251
let dtype = binding.root().dtype();
2248-
let return_dtype = request.projection.return_dtype(dtype)?;
2249-
let projection = request.projection.optimize_recursive(dtype)?;
2252+
let projection = normalize_scan_expr(request.projection.clone(), dtype)?;
2253+
let return_dtype = projection.return_dtype(dtype)?;
22502254
let filter = request
22512255
.filter
22522256
.clone()
2253-
.map(|filter| filter.optimize_recursive(dtype))
2257+
.map(|filter| normalize_scan_expr(filter, dtype))
22542258
.transpose()?;
22552259

22562260
let root = binding.root();
@@ -3002,6 +3006,10 @@ fn push_expr(
30023006
.ok_or_else(|| vortex_err!("scan2 could not push expression {expr}"))
30033007
}
30043008

3009+
fn normalize_scan_expr(expr: Expression, dtype: &DType) -> VortexResult<Expression> {
3010+
coerce_expression(expr, dtype)?.optimize_recursive(dtype)
3011+
}
3012+
30053013
fn validate_temporal_comparisons(expr: &Expression, scope: &DType) -> VortexResult<()> {
30063014
for child in expr.children().iter() {
30073015
validate_temporal_comparisons(child, scope)?;

0 commit comments

Comments
 (0)