Skip to content

Commit

Permalink
Change modeling of boxed ion literals to be lazy until evaluator.
Browse files Browse the repository at this point in the history
  • Loading branch information
jpschorr committed Nov 18, 2024
1 parent a3aeaf9 commit 6ba5963
Show file tree
Hide file tree
Showing 11 changed files with 366 additions and 138 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Changed
- *BREAKING* partiql-parser: Added a source location to `ParseError::UnexpectedEndOfInput`
- *BREAKING* partiql-ast: Changed the modelling of parsed literals.
- *BREAKING* partiql-logical: Changed the modelling of logical plan literals.
- partiql-eval: Fixed behavior of comparison and `BETWEEN` operations w.r.t. type mismatches

### Added
Expand Down
75 changes: 44 additions & 31 deletions partiql-eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ mod tests {
// TODO: once eval conformance tests added and/or modified evaluation API (to support other values
// in evaluator output), change or delete tests using this function
#[track_caller]
fn eval_bin_op(op: BinaryOp, lhs: Value, rhs: Value, expected_first_elem: Value) {
fn eval_bin_op<I: Into<logical::Lit>>(
op: BinaryOp,
lhs: Value,
rhs_lit: I,
expected_first_elem: Value,
) {
let mut plan = LogicalPlan::new();
let scan = plan.add_operator(BindingsOp::Scan(logical::Scan {
expr: ValueExpr::VarRef(
Expand All @@ -187,7 +192,7 @@ mod tests {
"lhs".to_string().into(),
))],
)),
Box::new(ValueExpr::Lit(Box::new(rhs))),
Box::new(ValueExpr::Lit(Box::new(rhs_lit.into()))),
),
)]),
}));
Expand Down Expand Up @@ -643,13 +648,13 @@ mod tests {
#[test]
fn and_or_null() {
#[track_caller]
fn eval_to_null(op: BinaryOp, lhs: Value, rhs: Value) {
fn eval_to_null<I: Into<logical::Lit>>(op: BinaryOp, lhs: I, rhs: I) {
let mut plan = LogicalPlan::new();
let expq = plan.add_operator(BindingsOp::ExprQuery(ExprQuery {
expr: ValueExpr::BinaryExpr(
op,
Box::new(ValueExpr::Lit(Box::new(lhs))),
Box::new(ValueExpr::Lit(Box::new(rhs))),
Box::new(ValueExpr::Lit(Box::new(lhs.into()))),
Box::new(ValueExpr::Lit(Box::new(rhs.into()))),
),
}));

Expand Down Expand Up @@ -697,8 +702,8 @@ mod tests {
"value".to_string().into(),
))],
)),
from: Box::new(ValueExpr::Lit(Box::new(from))),
to: Box::new(ValueExpr::Lit(Box::new(to))),
from: Box::new(ValueExpr::Lit(Box::new(from.into()))),
to: Box::new(ValueExpr::Lit(Box::new(to.into()))),
}),
)]),
}));
Expand Down Expand Up @@ -908,7 +913,7 @@ mod tests {
kind: JoinKind::Left,
left: Box::new(from_lhs),
right: Box::new(from_rhs),
on: Some(ValueExpr::Lit(Box::new(Value::from(true)))),
on: Some(ValueExpr::Lit(Box::new(Value::from(true).into()))),
}));

let sink = lg.add_operator(BindingsOp::Sink);
Expand Down Expand Up @@ -936,17 +941,21 @@ mod tests {
expr: Box::new(path_var("n", "a")),
cases: vec![
(
Box::new(ValueExpr::Lit(Box::new(Value::Integer(1)))),
Box::new(ValueExpr::Lit(Box::new(Value::from("one".to_string())))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(1).into()))),
Box::new(ValueExpr::Lit(Box::new(
Value::from("one".to_string()).into(),
))),
),
(
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::from("two".to_string())))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
Box::new(ValueExpr::Lit(Box::new(
Value::from("two".to_string()).into(),
))),
),
],
default: Some(Box::new(ValueExpr::Lit(Box::new(Value::from(
"other".to_string(),
))))),
default: Some(Box::new(ValueExpr::Lit(Box::new(
Value::from("other".to_string()).into(),
)))),
}
}

Expand All @@ -957,22 +966,26 @@ mod tests {
Box::new(ValueExpr::BinaryExpr(
BinaryOp::Eq,
Box::new(path_var("n", "a")),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(1)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(1).into()))),
)),
Box::new(ValueExpr::Lit(Box::new(Value::from("one".to_string())))),
Box::new(ValueExpr::Lit(Box::new(
Value::from("one".to_string()).into(),
))),
),
(
Box::new(ValueExpr::BinaryExpr(
BinaryOp::Eq,
Box::new(path_var("n", "a")),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
)),
Box::new(ValueExpr::Lit(Box::new(Value::from("two".to_string())))),
Box::new(ValueExpr::Lit(Box::new(
Value::from("two".to_string()).into(),
))),
),
],
default: Some(Box::new(ValueExpr::Lit(Box::new(Value::from(
"other".to_string(),
))))),
default: Some(Box::new(ValueExpr::Lit(Box::new(
Value::from("other".to_string()).into(),
)))),
}
}

Expand Down Expand Up @@ -1236,7 +1249,7 @@ mod tests {
"lhs".to_string().into(),
))],
)),
rhs: Box::new(ValueExpr::Lit(Box::new(rhs))),
rhs: Box::new(ValueExpr::Lit(Box::new(rhs.into()))),
}),
)]),
}));
Expand Down Expand Up @@ -1387,7 +1400,7 @@ mod tests {
println!("{:?}", &out);
assert_eq!(out, expected);
}
let list = ValueExpr::Lit(Box::new(Value::List(Box::new(list![1, 2, 3]))));
let list = ValueExpr::Lit(Box::new(Value::List(Box::new(list![1, 2, 3])).into()));

// `[1,2,3][0]` -> `1`
let index = ValueExpr::Path(Box::new(list.clone()), vec![PathComponent::Index(0)]);
Expand All @@ -1406,7 +1419,7 @@ mod tests {
test(index, Value::Integer(3));

// `{'a':10}[''||'a']` -> `10`
let tuple = ValueExpr::Lit(Box::new(Value::Tuple(Box::new(tuple![("a", 10)]))));
let tuple = ValueExpr::Lit(Box::new(Value::Tuple(Box::new(tuple![("a", 10)])).into()));
let index_expr = ValueExpr::BinaryExpr(
BinaryOp::Concat,
Box::new(ValueExpr::Lit(Box::new("".into()))),
Expand Down Expand Up @@ -1499,7 +1512,7 @@ mod tests {
expr: ValueExpr::BinaryExpr(
BinaryOp::Mul,
Box::new(va),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
),
}));

Expand Down Expand Up @@ -1578,7 +1591,7 @@ mod tests {
tuple_expr.values.push(ValueExpr::BinaryExpr(
BinaryOp::Mul,
Box::new(va),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
));

let project = lg.add_operator(ProjectValue(logical::ProjectValue {
Expand Down Expand Up @@ -1740,7 +1753,7 @@ mod tests {
list_expr.elements.push(ValueExpr::BinaryExpr(
BinaryOp::Mul,
Box::new(va),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
));

let select_value = lg.add_operator(ProjectValue(logical::ProjectValue {
Expand Down Expand Up @@ -1966,7 +1979,7 @@ mod tests {
"balance".to_string().into(),
))],
)),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(0)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(0).into()))),
),
}));

Expand Down Expand Up @@ -2096,7 +2109,7 @@ mod tests {
ValueExpr::BinaryExpr(
BinaryOp::Mul,
Box::new(va),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
),
)]),
}));
Expand Down Expand Up @@ -2170,7 +2183,7 @@ mod tests {
ValueExpr::BinaryExpr(
BinaryOp::Mul,
Box::new(va),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2)))),
Box::new(ValueExpr::Lit(Box::new(Value::Integer(2).into()))),
),
)]),
}));
Expand Down
18 changes: 11 additions & 7 deletions partiql-eval/src/plan.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use itertools::{Either, Itertools};
use ordered_float::OrderedFloat;
use partiql_logical as logical;
use petgraph::prelude::StableGraph;
use std::collections::HashMap;

use partiql_logical::{
AggFunc, BagOperator, BinaryOp, BindingsOp, CallName, GroupingStrategy, IsTypeExpr, JoinKind,
LogicalPlan, OpId, PathComponent, Pattern, PatternMatchExpr, SearchedCase, SetQuantifier,
Lit, LogicalPlan, OpId, PathComponent, Pattern, PatternMatchExpr, SearchedCase, SetQuantifier,
SortSpecNullOrder, SortSpecOrder, Type, UnaryOp, ValueExpr, VarRefType,
};
use petgraph::prelude::StableGraph;
use std::collections::HashMap;

use crate::error::{ErrorNode, PlanErr, PlanningError};
use crate::eval;
Expand All @@ -26,6 +26,7 @@ use crate::eval::expr::{
use crate::eval::EvalPlan;
use partiql_catalog::catalog::{Catalog, FunctionEntryFunction};
use partiql_value::Value::Null;
use partiql_value::{Bag, List, Tuple, Value};

#[macro_export]
macro_rules! correct_num_args_or_err {
Expand Down Expand Up @@ -392,7 +393,10 @@ impl<'c> EvaluatorPlanner<'c> {
),
ValueExpr::Lit(lit) => (
"literal",
EvalLitExpr { lit: *lit.clone() }.bind::<{ STRICT }>(vec![]),
EvalLitExpr {
lit: Value::from(lit.as_ref().clone()),
}
.bind::<{ STRICT }>(vec![]),
),
ValueExpr::Path(expr, components) => (
"path",
Expand Down Expand Up @@ -565,7 +569,7 @@ impl<'c> EvaluatorPlanner<'c> {
n.lhs.clone(),
n.rhs.clone(),
)),
Box::new(ValueExpr::Lit(Box::new(Null))),
Box::new(ValueExpr::Lit(Box::new(logical::Lit::Null))),
)],
default: Some(n.lhs.clone()),
});
Expand Down Expand Up @@ -794,7 +798,7 @@ mod tests {
// report the error.
let mut logical = LogicalPlan::new();
fn lit_int(i: usize) -> ValueExpr {
ValueExpr::Lit(Box::new(Value::from(i)))
ValueExpr::Lit(Box::new(logical::Lit::Int64(i as i64)))
}

let expq = logical.add_operator(BindingsOp::ExprQuery(ExprQuery {
Expand Down
2 changes: 0 additions & 2 deletions partiql-logical-planner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ partiql-ast = { path = "../partiql-ast", version = "0.11.*" }
partiql-ast-passes = { path = "../partiql-ast-passes", version = "0.11.*" }
partiql-catalog = { path = "../partiql-catalog", version = "0.11.*" }
partiql-common = { path = "../partiql-common", version = "0.11.*" }
partiql-extension-ion = { path = "../extension/partiql-extension-ion", version = "0.11.*" }
partiql-parser = { path = "../partiql-parser", version = "0.11.*" }
partiql-logical = { path = "../partiql-logical", version = "0.11.*" }
partiql-types = { path = "../partiql-types", version = "0.11.*" }
partiql-value = { path = "../partiql-value", version = "0.11.*" }

ion-rs_old = { version = "0.18", package = "ion-rs" }
ordered-float = "4"
itertools = "0.13"
unicase = "2.7"
Expand Down
6 changes: 3 additions & 3 deletions partiql-logical-planner/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ fn function_call_def_substring() -> CallDef {
CallSpec {
input: vec![CallSpecArg::Positional, CallSpecArg::Named("for".into())],
output: Box::new(|mut args| {
args.insert(1, ValueExpr::Lit(Box::new(Value::Integer(0))));
args.insert(1, ValueExpr::Lit(Box::new(logical::Lit::Int8(0))));
logical::ValueExpr::Call(logical::CallExpr {
name: logical::CallName::Substring,
arguments: args,
Expand Down Expand Up @@ -241,7 +241,7 @@ fn function_call_def_trim() -> CallDef {
output: Box::new(|mut args| {
args.insert(
0,
ValueExpr::Lit(Box::new(Value::String(" ".to_string().into()))),
ValueExpr::Lit(Box::new(logical::Lit::String(" ".to_string().into()))),
);

logical::ValueExpr::Call(logical::CallExpr {
Expand All @@ -255,7 +255,7 @@ fn function_call_def_trim() -> CallDef {
output: Box::new(|mut args| {
args.insert(
0,
ValueExpr::Lit(Box::new(Value::String(" ".to_string().into()))),
ValueExpr::Lit(Box::new(logical::Lit::String(" ".to_string().into()))),
);
logical::ValueExpr::Call(logical::CallExpr {
name: logical::CallName::BTrim,
Expand Down
Loading

0 comments on commit 6ba5963

Please sign in to comment.