Skip to content

Commit c8bc4cf

Browse files
committed
feat(analytics): Sort multi-value fields by min
Signed-off-by: Varun Bansal <bansvaru@amazon.com>
1 parent b62087d commit c8bc4cf

11 files changed

Lines changed: 459 additions & 19 deletions

File tree

sandbox/plugins/analytics-backend-datafusion/rust/src/helper.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,6 @@ pub async fn register_listing_table(
115115
let mut listing_options = ListingOptions::new(Arc::new(ParquetFormat::new()))
116116
.with_file_extension(".parquet")
117117
.with_collect_stat(true);
118-
if let Some(sort_exprs) = build_file_sort_order(sort_fields, sort_orders) {
119-
listing_options = listing_options.with_file_sort_order(vec![sort_exprs]);
120-
}
121-
122118
let resolved_schema = listing_options
123119
.infer_schema(&ctx.state(), &table_path)
124120
.await
@@ -127,6 +123,11 @@ pub async fn register_listing_table(
127123
e
128124
})?;
129125
let resolved_schema = coerce_inferred_schema(resolved_schema);
126+
if let Some(sort_exprs) =
127+
build_file_sort_order(sort_fields, sort_orders, resolved_schema.as_ref())
128+
{
129+
listing_options = listing_options.with_file_sort_order(vec![sort_exprs]);
130+
}
130131

131132
let table_config = ListingTableConfig::new(table_path)
132133
.with_listing_options(listing_options)

sandbox/plugins/analytics-backend-datafusion/rust/src/indexed_executor.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -975,11 +975,17 @@ async unsafe fn execute_indexed_with_context_inner(
975975
// interpretable by `api::fetch_by_row_ids` (which builds its own segments from
976976
// `ShardView.object_metas` in catalog order).
977977
let mut segments = segments;
978-
if should_reverse_segments(
979-
analyze_top_sort(&logical_plan).as_ref(),
980-
&sort_fields,
981-
&sort_orders,
982-
) {
978+
let lead_sort_is_list = sort_fields
979+
.first()
980+
.and_then(|field| schema.field_with_name(field).ok())
981+
.is_some_and(|field| matches!(field.data_type(), arrow::datatypes::DataType::List(_)));
982+
if !lead_sort_is_list
983+
&& should_reverse_segments(
984+
analyze_top_sort(&logical_plan).as_ref(),
985+
&sort_fields,
986+
&sort_orders,
987+
)
988+
{
983989
log_debug!(
984990
"indexed_executor: reversing segment iteration (catalog leading sort={:?} {:?}, query opposite)",
985991
sort_fields.first(),

sandbox/plugins/analytics-backend-datafusion/rust/src/indexed_table/segment_info.rs

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,10 @@ fn compute_segment_sort_bounds(
208208
file_schema: &arrow::datatypes::SchemaRef,
209209
pq_meta: &ParquetMetaData,
210210
) -> (Option<ScalarValue>, Option<ScalarValue>) {
211-
if file_schema.index_of(lead_field).is_err() {
211+
let Ok(field) = file_schema.field_with_name(lead_field) else {
212+
return (None, None);
213+
};
214+
if matches!(field.data_type(), arrow::datatypes::DataType::List(_)) {
212215
return (None, None);
213216
}
214217

@@ -526,6 +529,61 @@ mod tests {
526529
);
527530
}
528531

532+
#[tokio::test]
533+
async fn scalar_and_list_field_promote_to_list_schema() {
534+
let dir = tempdir().unwrap();
535+
let scalar_schema = Arc::new(Schema::new(vec![Field::new("tags", DataType::Utf8, true)]));
536+
let list_child = Arc::new(Field::new("element", DataType::Utf8, true));
537+
let list_schema = Arc::new(Schema::new(vec![Field::new(
538+
"tags",
539+
DataType::List(Arc::clone(&list_child)),
540+
true,
541+
)]));
542+
let scalar_path = write_parquet(
543+
dir.path(),
544+
"a.parquet",
545+
scalar_schema,
546+
vec![Arc::new(StringArray::from(vec![Some("prod")]))],
547+
);
548+
let list_path = write_parquet(
549+
dir.path(),
550+
"b.parquet",
551+
list_schema,
552+
vec![Arc::new(ListArray::new(
553+
list_child,
554+
OffsetBuffer::new(vec![0_i32, 2].into()),
555+
Arc::new(StringArray::from(vec!["prod", "error"])),
556+
None,
557+
))],
558+
);
559+
560+
let store: Arc<dyn ObjectStore> = Arc::new(LocalFileSystem::new());
561+
let metas = object_metas(store.as_ref(), &[scalar_path, list_path]).await;
562+
let ctx = SessionContext::new();
563+
let generations: Vec<i64> = (0..metas.len() as i64).collect();
564+
let (segments, schema) = build_segments(
565+
&ctx.state(),
566+
Arc::clone(&store),
567+
&metas,
568+
&generations,
569+
default_metadata_cache(),
570+
&["tags".to_string()],
571+
)
572+
.await
573+
.unwrap();
574+
575+
assert!(
576+
segments[1].sort_min.is_none() && segments[1].sort_max.is_none(),
577+
"LIST child statistics must not be used as per-row list_min bounds"
578+
);
579+
580+
assert!(matches!(
581+
schema.field_with_name("tags").unwrap().data_type(),
582+
DataType::List(child)
583+
if matches!(child.data_type(), DataType::Utf8 | DataType::Utf8View)
584+
));
585+
}
586+
529587
/// Incompatible types (Int32 vs Int64 on the same field name) is
530588
/// a fail-fast: the Arrow `Schema::try_merge` rejects it, and we
531589
/// bubble the error up. Catches accidental type widening at the

sandbox/plugins/analytics-backend-datafusion/rust/src/indexed_table/table_provider.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,18 @@ fn build_projected_lex_ordering(
150150
let mut exprs: Vec<PhysicalSortExpr> = Vec::with_capacity(sort_fields.len());
151151
for (i, field) in sort_fields.iter().enumerate() {
152152
let phys = match physical_col(field, projected_schema) {
153-
Ok(e) => e,
153+
Ok(expr) => match projected_schema
154+
.field_with_name(field)
155+
.map(|field| field.data_type())
156+
{
157+
Ok(DataType::List(_)) => {
158+
match crate::udf::list_min::physical_expr(expr, projected_schema.as_ref()) {
159+
Ok(expr) => expr,
160+
Err(_) => break,
161+
}
162+
}
163+
_ => expr,
164+
},
154165
Err(_) => break,
155166
};
156167
let descending = sort_orders
@@ -866,6 +877,21 @@ mod tests {
866877
}
867878
}
868879

880+
#[test]
881+
fn list_sort_key_advertises_list_min_physical_ordering() {
882+
let child = Arc::new(Field::new("element", DataType::Utf8View, true));
883+
let schema = Arc::new(Schema::new(vec![Field::new(
884+
"tags",
885+
DataType::List(child),
886+
true,
887+
)]));
888+
let ordering =
889+
build_projected_lex_ordering(&schema, &["tags".into()], &["desc".into()]).unwrap();
890+
assert!(format!("{}", ordering[0].expr).contains("list_min"));
891+
assert!(ordering[0].options.descending);
892+
assert!(!ordering[0].options.nulls_first);
893+
}
894+
869895
// QueryShardExec holds an ExecutionPlanMetricsSet (not Clone). We only
870896
// need to inspect `.predicate`, so read through a reference.
871897
async fn scan_predicate(

sandbox/plugins/analytics-backend-datafusion/rust/src/session_context.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -306,12 +306,6 @@ pub async unsafe fn create_session_context(
306306
.with_collect_stat(true)
307307
.with_target_partitions(effective_partitions);
308308

309-
if let Some(sort_exprs) =
310-
build_file_sort_order(&shard_view.sort_fields, &shard_view.sort_orders)
311-
{
312-
listing_options = listing_options.with_file_sort_order(vec![sort_exprs]);
313-
}
314-
315309
// Register under the planner's logical table name (alias / index pattern / index), shipped
316310
// explicitly as logicalTableName on the shard-scan instruction node. See
317311
// resolve_register_name for why we do NOT reverse-engineer this from the plan bytes. The
@@ -368,11 +362,18 @@ pub async unsafe fn create_session_context(
368362
// failing with "Cannot merge statistics with different number of columns". Non-widened
369363
// (single-index) scans keep full stats.
370364
// TODO: re-enable once DataFusion's Statistics::try_merge tolerates a column-count delta.
371-
let listing_options = if resolved_schema.fields().len() != inferred_field_count {
365+
let mut listing_options = if resolved_schema.fields().len() != inferred_field_count {
372366
listing_options.with_collect_stat(false)
373367
} else {
374368
listing_options
375369
};
370+
if let Some(sort_exprs) = build_file_sort_order(
371+
&shard_view.sort_fields,
372+
&shard_view.sort_orders,
373+
resolved_schema.as_ref(),
374+
) {
375+
listing_options = listing_options.with_file_sort_order(vec![sort_exprs]);
376+
}
376377

377378
let table_config = ListingTableConfig::new(shard_view.table_path.clone())
378379
.with_listing_options(listing_options)
@@ -737,6 +738,7 @@ fn try_acquire_budget(
737738
pub(crate) fn build_file_sort_order(
738739
sort_fields: &[String],
739740
sort_orders: &[String],
741+
schema: &arrow::datatypes::Schema,
740742
) -> Option<Vec<datafusion::logical_expr::SortExpr>> {
741743
if sort_fields.is_empty() {
742744
return None;
@@ -749,7 +751,12 @@ pub(crate) fn build_file_sort_order(
749751
.map(|(name, order)| {
750752
let ascending = order.eq_ignore_ascii_case("asc");
751753
let nulls_first = ascending;
752-
Expr::Column(Column::from_name(name.clone())).sort(ascending, nulls_first)
754+
let column = Expr::Column(Column::from_name(name.clone()));
755+
let key = match schema.field_with_name(name).map(|field| field.data_type()) {
756+
Ok(arrow::datatypes::DataType::List(_)) => crate::udf::list_min::expr(column),
757+
_ => column,
758+
};
759+
key.sort(ascending, nulls_first)
753760
})
754761
.collect();
755762
Some(sort_exprs)
@@ -772,6 +779,26 @@ mod tests {
772779
use crate::agg_mode::Mode;
773780
use crate::query_tracker::QueryTrackingContext;
774781

782+
#[test]
783+
fn file_sort_order_uses_fixed_list_min_for_both_directions() {
784+
let child = Arc::new(Field::new("element", DataType::Utf8View, true));
785+
let schema = Schema::new(vec![
786+
Field::new("tags", DataType::List(child), true),
787+
Field::new("id", DataType::Int64, true),
788+
]);
789+
790+
for (order, ascending) in [("asc", true), ("desc", false)] {
791+
let ordering =
792+
build_file_sort_order(&["tags".into()], &[order.into()], &schema).unwrap();
793+
assert!(format!("{}", ordering[0].expr).contains("list_min"));
794+
assert_eq!(ordering[0].asc, ascending);
795+
assert_eq!(ordering[0].nulls_first, ascending);
796+
}
797+
798+
let scalar = build_file_sort_order(&["id".into()], &["asc".into()], &schema).unwrap();
799+
assert!(!format!("{}", scalar[0].expr).contains("list_min"));
800+
}
801+
775802
#[tokio::test]
776803
async fn test_widen_schema_noop_when_plan_empty() {
777804
let ctx = SessionContext::new();

0 commit comments

Comments
 (0)