Skip to content

Commit e73c844

Browse files
robert3005claude
andcommitted
Remove repeated elements from IN lists
DataFusion builds the IN list of a hash join filter from the keys of the build side, which repeat. Each repeat makes the conversion and the lookup more costly. - `in_list_of` removes equal elements after it sorts them. One null keeps the SQL semantics. - The DataFusion converter converts equal literals once. - The converter reads the type and nullability of a predicate, and does not ask for its return field. DataFusion names that field by formatting the whole expression, which for a large IN list costs more than the conversion. - `contiguous_integer_bounds` checks the dtype before it views a scalar as a primitive, because a failed view builds an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 86d2845 commit e73c844

3 files changed

Lines changed: 48 additions & 3 deletions

File tree

‎vortex-array/src/expr/exprs.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,11 +1208,14 @@ pub fn in_list_of(
12081208
}
12091209

12101210
// Ordered elements make equal lists from the same set, so that equal filters deduplicate.
1211+
// An engine can build a list from the keys of a hash join, which repeat, and each repeat makes
1212+
// the lookup more costly, so equal elements are removed. One null keeps the SQL semantics.
12111213
if elements
12121214
.windows(2)
12131215
.all(|pair| pair[0].partial_cmp(&pair[1]).is_some())
12141216
{
12151217
elements.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal));
1218+
elements.dedup();
12161219
}
12171220
let list = Scalar::list(Arc::new(element_dtype), elements, Nullability::NonNullable);
12181221
Ok(in_list(needle, lit(list)))
@@ -1224,10 +1227,11 @@ fn contiguous_integer_bounds(scalars: &[Scalar]) -> Option<(&Scalar, &Scalar)> {
12241227
let mut values = scalars
12251228
.iter()
12261229
.map(|scalar| {
1227-
let primitive = scalar.as_primitive_opt()?;
1228-
if !primitive.ptype().is_int() {
1230+
// Viewing a scalar of another type as a primitive builds an error, which is costly.
1231+
if !matches!(scalar.dtype(), DType::Primitive(ptype, _) if ptype.is_int()) {
12291232
return None;
12301233
}
1234+
let primitive = scalar.as_primitive_opt()?;
12311235
let value = if primitive.ptype().is_unsigned_int() {
12321236
primitive.as_opt::<u64>()??.into()
12331237
} else {

‎vortex-datafusion/src/convert/exprs/mod.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,14 @@ impl DefaultExpressionConverter {
273273
)
274274
.map_err(|_| Unconverted::Unsupported)?;
275275
let converted = self.convert_expr(expr, schema, &input_dtype)?;
276-
let expected_dtype = self.arrow_dtype(expr.return_field(schema)?.as_ref())?;
276+
// `return_field` names the field by formatting the whole expression. For the IN list of a
277+
// hash join filter, with thousands of keys, that costs more than the conversion. A
278+
// predicate has no field metadata, so its type and nullability are enough.
279+
let expected_dtype = if expr.data_type(schema)? == DataType::Boolean {
280+
DType::Bool(expr.nullable(schema)?.into())
281+
} else {
282+
self.arrow_dtype(expr.return_field(schema)?.as_ref())?
283+
};
277284
if !converted_dtype(&converted, &input_dtype)?.eq_ignore_nullability(&expected_dtype) {
278285
return Err(Unconverted::Unsupported);
279286
}
@@ -442,9 +449,16 @@ impl DefaultExpressionConverter {
442449
}
443450
let value_dtype = converted_dtype(&value, input_dtype)?;
444451

452+
// The list of a hash join filter holds every build-side key, which repeat. Each element
453+
// converts through Arrow, so equal elements are converted once.
445454
let elements = in_list
446455
.list()
447456
.iter()
457+
.unique_by(|element| {
458+
element
459+
.downcast_ref::<df_expr::Literal>()
460+
.map(|literal| literal.value().clone())
461+
})
448462
.map(|element| {
449463
let element = self.convert_expr(element, schema, input_dtype)?;
450464
element

‎vortex-datafusion/src/convert/exprs/tests.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,33 @@ fn test_in_list_converts_to_set_lookup(#[values(false, true)] negated: bool) ->
365365
Ok(())
366366
}
367367

368+
#[test]
369+
fn test_in_list_drops_repeated_elements() -> DFResult<()> {
370+
let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
371+
let expr = Arc::new(df_expr::InListExpr::try_new(
372+
Arc::new(df_expr::Column::new("a", 0)),
373+
int_literals([Some(7), None, Some(1), Some(7), None, Some(1)]),
374+
false,
375+
&schema,
376+
)?);
377+
let element_dtype = Arc::new(DType::Primitive(PType::I32, Nullability::Nullable));
378+
let list = Scalar::list(
379+
element_dtype,
380+
vec![
381+
Scalar::null(DType::Primitive(PType::I32, Nullability::Nullable)),
382+
Scalar::from(Some(1i32)),
383+
Scalar::from(Some(7i32)),
384+
],
385+
Nullability::NonNullable,
386+
);
387+
388+
assert_eq!(
389+
convert(expr, &schema)?,
390+
vortex::expr::in_list(get_item("a", root()), lit(list))
391+
);
392+
Ok(())
393+
}
394+
368395
#[test]
369396
fn test_contiguous_in_list_converts_to_between() -> DFResult<()> {
370397
let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);

0 commit comments

Comments
 (0)