Skip to content

Commit dcb9bb2

Browse files
authored
feat: Add support for kernel default expression evaluation (delta-io#979)
## What changes are proposed in this pull request? Pathfinding on delta-io#686 exposed a gap in kernel's default predicate evaluator API: It cannot evaluate expressions, even tho expressions can legitimately be input to predicates, e.g. ```sql WHERE x < y + z ``` Add that missing support, with arithmetic on integer types as the exemplar. Support for floating point types is left as future work, because it requires extra care to decide how to handle Inf and NaN values: Should they propagate as-is? Or one/both be detected and converted to `Scalar::Null`? NOTE: This PR is _not_ adding arbitrary expression evaluation support to the `KernelPredicateEvaluator` API. That would require a much bigger design, and it's not clear that it's even desirable -- normal expression evaluation should be handled by the engine, potentially using an `ExpressionEvaluationHandler` if the connector relies on the default engine for expression evaluation. ## How was this change tested? New unit tests
1 parent e01fd3b commit dcb9bb2

3 files changed

Lines changed: 203 additions & 2 deletions

File tree

kernel/src/expressions/scalars.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,58 @@ impl Scalar {
299299
};
300300
Ok(Self::Timestamp(timestamp.timestamp_micros()))
301301
}
302+
303+
/// Attempts to add two scalars, returning None if they were incompatible.
304+
pub fn try_add(&self, other: &Scalar) -> Option<Scalar> {
305+
use Scalar::*;
306+
let result = match (self, other) {
307+
(Integer(a), Integer(b)) => Integer(a.checked_add(*b)?),
308+
(Long(a), Long(b)) => Long(a.checked_add(*b)?),
309+
(Short(a), Short(b)) => Short(a.checked_add(*b)?),
310+
(Byte(a), Byte(b)) => Byte(a.checked_add(*b)?),
311+
_ => return None,
312+
};
313+
Some(result)
314+
}
315+
316+
/// Attempts to subtract two scalars, returning None if they were incompatible.
317+
pub fn try_sub(&self, other: &Scalar) -> Option<Scalar> {
318+
use Scalar::*;
319+
let result = match (self, other) {
320+
(Integer(a), Integer(b)) => Integer(a.checked_sub(*b)?),
321+
(Long(a), Long(b)) => Long(a.checked_sub(*b)?),
322+
(Short(a), Short(b)) => Short(a.checked_sub(*b)?),
323+
(Byte(a), Byte(b)) => Byte(a.checked_sub(*b)?),
324+
_ => return None,
325+
};
326+
Some(result)
327+
}
328+
329+
/// Attempts to multiply two scalars, returning None if they were incompatible.
330+
pub fn try_mul(&self, other: &Scalar) -> Option<Scalar> {
331+
use Scalar::*;
332+
let result = match (self, other) {
333+
(Integer(a), Integer(b)) => Integer(a.checked_mul(*b)?),
334+
(Long(a), Long(b)) => Long(a.checked_mul(*b)?),
335+
(Short(a), Short(b)) => Short(a.checked_mul(*b)?),
336+
(Byte(a), Byte(b)) => Byte(a.checked_mul(*b)?),
337+
_ => return None,
338+
};
339+
Some(result)
340+
}
341+
342+
/// Attempts to divide two scalars, returning None if they were incompatible.
343+
pub fn try_div(&self, other: &Scalar) -> Option<Scalar> {
344+
use Scalar::*;
345+
let result = match (self, other) {
346+
(Integer(a), Integer(b)) => Integer(a.checked_div(*b)?),
347+
(Long(a), Long(b)) => Long(a.checked_div(*b)?),
348+
(Short(a), Short(b)) => Short(a.checked_div(*b)?),
349+
(Byte(a), Byte(b)) => Byte(a.checked_div(*b)?),
350+
_ => return None,
351+
};
352+
Some(result)
353+
}
302354
}
303355

304356
impl Display for Scalar {

kernel/src/kernel_predicates/mod.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
//! but data skipping "evaluation" actually produces a transformed predicate that replaces column
55
//! references with stats column references, which log replay will instruct the engine to evaluate.
66
use crate::expressions::{
7-
BinaryPredicate, BinaryPredicateOp, ColumnName, Expression as Expr, JunctionPredicate,
8-
JunctionPredicateOp, Predicate as Pred, Scalar, UnaryPredicate, UnaryPredicateOp,
7+
BinaryExpression, BinaryExpressionOp, BinaryPredicate, BinaryPredicateOp, ColumnName,
8+
Expression as Expr, JunctionPredicate, JunctionPredicateOp, Predicate as Pred, Scalar,
9+
UnaryPredicate, UnaryPredicateOp,
910
};
1011
use crate::schema::DataType;
1112

@@ -571,6 +572,25 @@ impl<R: ResolveColumnAsScalar> DefaultKernelPredicateEvaluator<R> {
571572
fn resolve_column(&self, col: &ColumnName) -> Option<Scalar> {
572573
self.resolver.resolve_column(col)
573574
}
575+
576+
#[allow(unused)] // TODO: remove this annotation once opaque predicates start using it
577+
pub(crate) fn eval_expr(&self, expr: &Expr) -> Option<Scalar> {
578+
match expr {
579+
Expr::Literal(value) => Some(value.clone()),
580+
Expr::Column(name) => self.resolve_column(name),
581+
Expr::Predicate(pred) => self.eval_pred(pred, false).map(Scalar::from),
582+
Expr::Struct(_) => None, // TODO
583+
Expr::Binary(BinaryExpression { op, left, right }) => {
584+
let op_fn = match op {
585+
BinaryExpressionOp::Plus => Scalar::try_add,
586+
BinaryExpressionOp::Minus => Scalar::try_sub,
587+
BinaryExpressionOp::Multiply => Scalar::try_mul,
588+
BinaryExpressionOp::Divide => Scalar::try_div,
589+
};
590+
op_fn(&self.eval_expr(left)?, &self.eval_expr(right)?)
591+
}
592+
}
593+
}
574594
}
575595

576596
impl<R: ResolveColumnAsScalar + 'static> From<R> for DefaultKernelPredicateEvaluator<R> {

kernel/src/kernel_predicates/tests.rs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,135 @@ fn test_default_partial_cmp_scalars() {
181181
}
182182
}
183183

184+
#[test]
185+
fn test_default_scalar_arithmetic() {
186+
use Scalar::*;
187+
let left = &[Byte(2), Short(200), Integer(20000), Long(2000000)];
188+
let right = &[Byte(3), Short(30), Integer(3000), Long(300000)];
189+
let expected = [
190+
(Byte(5), Byte(-1), Byte(6), Byte(0)),
191+
(Short(230), Short(170), Short(6000), Short(6)),
192+
(
193+
Integer(23000),
194+
Integer(17000),
195+
Integer(60000000),
196+
Integer(6),
197+
),
198+
(Long(2300000), Long(1700000), Long(600000000000), Long(6)),
199+
];
200+
201+
let filter = DefaultKernelPredicateEvaluator::from(Scalar::from(1));
202+
for ((l, r), (add, sub, mul, div)) in left.iter().zip(right).zip(expected) {
203+
expect_eq!(
204+
filter.eval_expr(&(Expr::literal(l.clone()) + Expr::literal(r.clone()))),
205+
Some(add),
206+
"add({l:?}, {r:?})"
207+
);
208+
expect_eq!(
209+
filter.eval_expr(&(Expr::literal(l.clone()) - Expr::literal(r.clone()))),
210+
Some(sub),
211+
"sub({l:?}, {r:?})"
212+
);
213+
expect_eq!(
214+
filter.eval_expr(&(Expr::literal(l.clone()) * Expr::literal(r.clone()))),
215+
Some(mul),
216+
"mul({l:?}, {r:?})"
217+
);
218+
expect_eq!(
219+
filter.eval_expr(&(Expr::literal(l.clone()) / Expr::literal(r.clone()))),
220+
Some(div),
221+
"div({l:?}, {r:?})"
222+
);
223+
}
224+
225+
// Invalid type combinations
226+
expect_eq!(
227+
filter.eval_expr(&(Expr::literal("hi") + Expr::literal("ho"))),
228+
None,
229+
"add(string, string)"
230+
);
231+
expect_eq!(
232+
filter.eval_expr(&(Expr::literal(1i8) + Expr::literal(1i64))),
233+
None,
234+
"add(byte, long)"
235+
);
236+
expect_eq!(
237+
filter.eval_expr(&(Expr::literal(1i8) - Expr::literal(1i64))),
238+
None,
239+
"sub(byte, long)"
240+
);
241+
expect_eq!(
242+
filter.eval_expr(&(Expr::literal(1i8) * Expr::literal(1i64))),
243+
None,
244+
"mul(byte, long)"
245+
);
246+
expect_eq!(
247+
filter.eval_expr(&(Expr::literal(1i8) / Expr::literal(1i64))),
248+
None,
249+
"div(byte, long)"
250+
);
251+
252+
// Addition overflow
253+
let args = [
254+
(Byte(i8::MAX), Byte(1)),
255+
(Short(i16::MAX), Short(1)),
256+
(Integer(i32::MAX), Integer(1)),
257+
(Long(i64::MAX), Long(1)),
258+
];
259+
for (l, r) in args {
260+
expect_eq!(
261+
filter.eval_expr(&(Expr::literal(l.clone()) + Expr::literal(r.clone()))),
262+
None,
263+
"add({l:?}, {r:?})"
264+
);
265+
}
266+
267+
// Subtraction overflow
268+
let args = [
269+
(Byte(i8::MIN), Byte(1)),
270+
(Short(i16::MIN), Short(1)),
271+
(Integer(i32::MIN), Integer(1)),
272+
(Long(i64::MIN), Long(1)),
273+
];
274+
for (l, r) in args {
275+
expect_eq!(
276+
filter.eval_expr(&(Expr::literal(l.clone()) - Expr::literal(r.clone()))),
277+
None,
278+
"sub({l:?}, {r:?})"
279+
);
280+
}
281+
282+
// Multiplication overflow
283+
let args = [
284+
Byte(i8::MAX),
285+
Short(i16::MAX),
286+
Integer(i32::MAX),
287+
Long(i64::MAX),
288+
];
289+
for arg in args {
290+
expect_eq!(
291+
filter.eval_expr(&(Expr::literal(arg.clone()) * Expr::literal(arg.clone()))),
292+
None,
293+
"mul({arg:?}, {arg:?})"
294+
);
295+
}
296+
297+
// Division overflow
298+
let args = [
299+
(Byte(i8::MAX), Byte(0)),
300+
(Short(i16::MAX), Short(0)),
301+
(Integer(i32::MAX), Integer(0)),
302+
(Long(i64::MAX), Long(0)),
303+
];
304+
for (l, r) in args {
305+
expect_eq!(
306+
filter.eval_expr(&(Expr::literal(l.clone()) / Expr::literal(r.clone()))),
307+
None,
308+
"div({l:?}, {r:?})"
309+
);
310+
}
311+
}
312+
184313
// Verifies that eval_binary_scalars uses partial_cmp_scalars correctly
185314
#[test]
186315
fn test_eval_binary_scalars() {

0 commit comments

Comments
 (0)