Skip to content

Commit

Permalink
Improvements to Value iterators (#422)
Browse files Browse the repository at this point in the history
  • Loading branch information
jpschorr authored Aug 14, 2023
1 parent d93dfe7 commit 3dc0773
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 32 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
### Changed
- *BREAKING:* partiql-eval: Construction of expression evaluators changed to separate binding from evaluation of expression. & implement strict eval
- *BREAKING:* partiql-value: `Value` trait's `is_null_or_missing` renamed to `is_absent`
- *BREAKING:* partiql-value: `Value` trait's `coerce_to_tuple`, `coerece_to_bag`, and `coerce_to_list` methods renamed to `coerce_into_tuple`, `coerece_into_bag`, and `coerece_into_list`.
- *BREAKING:* partiql-value: `Tuple`'s `pairs` and `into_pairs` changed to return concrete `Iterator` types.
- *BREAKING:* partiql-eval: `EvaluatorPlanner` construction now takes an `EvaluationMode` parameter.
- *BREAKING:* partiql-eval: `like_to_re_pattern` is no longer public.
- *BREAKING:* partiql-value: Box Decimals in `Value` to assure `Value` fits in 16 bytes.
Expand Down
7 changes: 6 additions & 1 deletion extension/partiql-extension-ion/src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ impl<'a> Iterator for IonValueIter<'a> {
fn next(&mut self) -> Option<Self::Item> {
self.inner.next()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.inner.size_hint()
}
}

struct IonValueIterInner<D, R>
Expand Down Expand Up @@ -519,7 +524,7 @@ where
let is_bag = has_annotation(reader, BAG_ANNOT);
let list = decode_list(self, reader);
if is_bag {
Ok(Bag::from(list?.coerce_to_list()).into())
Ok(Bag::from(list?.coerce_into_list()).into())
} else {
list
}
Expand Down
20 changes: 10 additions & 10 deletions partiql-eval/src/eval/evaluable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ impl Evaluable for EvalGroupBy {
EvalGroupingStrategy::GroupFull => {
let mut groups: HashMap<Tuple, Vec<Value>> = HashMap::new();
for v in input_value.into_iter() {
let v_as_tuple = v.coerce_to_tuple();
let v_as_tuple = v.coerce_into_tuple();
let group = self.eval_group(&v_as_tuple, ctx);
// Compute next aggregation result for each of the aggregation expressions
for aggregate_expr in self.aggregate_exprs.iter_mut() {
Expand Down Expand Up @@ -877,7 +877,7 @@ impl Evaluable for EvalPivot {
let tuple: Tuple = input_value
.into_iter()
.filter_map(|binding| {
let binding = binding.coerce_to_tuple();
let binding = binding.coerce_into_tuple();
let key = self.key.evaluate(&binding, ctx);
if let Value::String(s) = key.as_ref() {
let value = self.value.evaluate(&binding, ctx);
Expand Down Expand Up @@ -931,7 +931,7 @@ impl Evaluable for EvalUnpivot {
fn evaluate(&mut self, ctx: &dyn EvalContext) -> Value {
let tuple = match self.expr.evaluate(&Tuple::new(), ctx).into_owned() {
Value::Tuple(tuple) => *tuple,
other => other.coerce_to_tuple(),
other => other.coerce_into_tuple(),
};

let as_key = self.as_key.as_str();
Expand Down Expand Up @@ -990,7 +990,7 @@ impl Evaluable for EvalFilter {

let filtered = input_value
.into_iter()
.map(Value::coerce_to_tuple)
.map(Value::coerce_into_tuple)
.filter_map(|v| self.eval_filter(&v, ctx).then_some(v));
Value::from(filtered.collect::<Bag>())
}
Expand Down Expand Up @@ -1035,7 +1035,7 @@ impl Evaluable for EvalHaving {

let filtered = input_value
.into_iter()
.map(Value::coerce_to_tuple)
.map(Value::coerce_into_tuple)
.filter_map(|v| self.eval_having(&v, ctx).then_some(v));
Value::from(filtered.collect::<Bag>())
}
Expand Down Expand Up @@ -1203,7 +1203,7 @@ impl Evaluable for EvalSelectValue {
let ordered = input_value.is_ordered();

let values = input_value.into_iter().map(|v| {
let v_as_tuple = v.coerce_to_tuple();
let v_as_tuple = v.coerce_into_tuple();
self.expr.evaluate(&v_as_tuple, ctx).into_owned()
});

Expand Down Expand Up @@ -1241,7 +1241,7 @@ impl Evaluable for EvalSelect {
let ordered = input_value.is_ordered();

let values = input_value.into_iter().map(|v| {
let v_as_tuple = v.coerce_to_tuple();
let v_as_tuple = v.coerce_into_tuple();

let tuple_pairs = self.exprs.iter().filter_map(|(alias, expr)| {
let evaluated_val = expr.evaluate(&v_as_tuple, ctx);
Expand Down Expand Up @@ -1285,9 +1285,9 @@ impl Evaluable for EvalSelectAll {
let ordered = input_value.is_ordered();

let values = input_value.into_iter().map(|val| {
val.coerce_to_tuple()
val.coerce_into_tuple()
.into_values()
.flat_map(|v| v.coerce_to_tuple().into_pairs())
.flat_map(|v| v.coerce_into_tuple().into_pairs())
.collect::<Tuple>()
});

Expand Down Expand Up @@ -1319,7 +1319,7 @@ impl EvalExprQuery {

impl Evaluable for EvalExprQuery {
fn evaluate(&mut self, ctx: &dyn EvalContext) -> Value {
let input_value = self.input.take().unwrap_or(Value::Null).coerce_to_tuple();
let input_value = self.input.take().unwrap_or(Value::Null).coerce_into_tuple();

self.expr.evaluate(&input_value, ctx).into_owned()
}
Expand Down
9 changes: 9 additions & 0 deletions partiql-eval/src/eval/expr/coll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,21 @@ where
{
type Item = V;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
match self {
SetQuantified::All(i) => i.next(),
SetQuantified::Distinct(i) => i.next(),
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
SetQuantified::All(i) => i.size_hint(),
SetQuantified::Distinct(i) => i.size_hint(),
}
}
}

/// An [`Iterator`] over a 'set' of values
Expand Down
10 changes: 5 additions & 5 deletions partiql-eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ mod tests {
let mut bindings = MapBindings::default();
bindings.insert("data", list![Tuple::from([("lhs", lhs)])].into());

let result = evaluate(plan, bindings).coerce_to_bag();
let result = evaluate(plan, bindings).coerce_into_bag();
assert!(!&result.is_empty());
let expected_result = if expected_first_elem != Missing {
bag!(Tuple::from([("result", expected_first_elem)]))
Expand Down Expand Up @@ -701,7 +701,7 @@ mod tests {
let mut bindings = MapBindings::default();
bindings.insert("data", list![Tuple::from([("value", value)])].into());

let result = evaluate(plan, bindings).coerce_to_bag();
let result = evaluate(plan, bindings).coerce_into_bag();
assert!(!&result.is_empty());
let expected_result = bag!(Tuple::from([("result", expected_first_elem)]));
assert_eq!(expected_result, result);
Expand Down Expand Up @@ -1170,7 +1170,7 @@ mod tests {
let mut bindings = MapBindings::default();
bindings.insert("data", list![Tuple::from([("expr", expr)])].into());

let result = evaluate(plan, bindings).coerce_to_bag();
let result = evaluate(plan, bindings).coerce_into_bag();
assert!(!&result.is_empty());
assert_eq!(bag!(Tuple::from([("result", expected_first_elem)])), result);
}
Expand Down Expand Up @@ -1236,7 +1236,7 @@ mod tests {
let mut bindings = MapBindings::default();
bindings.insert("data", list![Tuple::from([("lhs", lhs)])].into());

let result = evaluate(plan, bindings).coerce_to_bag();
let result = evaluate(plan, bindings).coerce_into_bag();
assert!(!&result.is_empty());
let expected_result = if expected_first_elem != Missing {
bag!(Tuple::from([("result", expected_first_elem)]))
Expand Down Expand Up @@ -1304,7 +1304,7 @@ mod tests {
.for_each(|(i, e)| data.insert(&format!("arg{i}"), e));
bindings.insert("data", list![data].into());

let result = evaluate(plan, bindings).coerce_to_bag();
let result = evaluate(plan, bindings).coerce_into_bag();
assert!(!&result.is_empty());
assert_eq!(bag!(Tuple::from([("result", expected_first_elem)])), result);
}
Expand Down
13 changes: 13 additions & 0 deletions partiql-value/src/bag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ impl<'a> IntoIterator for &'a Bag {
type Item = &'a Value;
type IntoIter = BagIter<'a>;

#[inline]
fn into_iter(self) -> Self::IntoIter {
BagIter(self.0.iter())
}
Expand All @@ -119,9 +120,15 @@ pub struct BagIter<'a>(slice::Iter<'a, Value>);
impl<'a> Iterator for BagIter<'a> {
type Item = &'a Value;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}

impl IntoIterator for Bag {
Expand All @@ -138,9 +145,15 @@ pub struct BagIntoIterator(vec::IntoIter<Value>);
impl Iterator for BagIntoIterator {
type Item = Value;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}

impl Debug for Bag {
Expand Down
Loading

1 comment on commit 3dc0773

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PartiQL (rust) Benchmark

Benchmark suite Current: 3dc0773 Previous: d93dfe7 Ratio
parse-1 5328 ns/iter (± 24) 5274 ns/iter (± 59) 1.01
parse-15 48913 ns/iter (± 66) 48677 ns/iter (± 153) 1.00
parse-30 96644 ns/iter (± 102) 96312 ns/iter (± 207) 1.00
compile-1 5552 ns/iter (± 12) 5704 ns/iter (± 18) 0.97
compile-15 43153 ns/iter (± 34) 43733 ns/iter (± 80) 0.99
compile-30 88649 ns/iter (± 59) 88337 ns/iter (± 260) 1.00
plan-1 67748 ns/iter (± 272) 68312 ns/iter (± 89) 0.99
plan-15 1065888 ns/iter (± 2343) 1078230 ns/iter (± 1377) 0.99
plan-30 2135651 ns/iter (± 8123) 2161453 ns/iter (± 4788) 0.99
eval-1 23319471 ns/iter (± 648698) 22792114 ns/iter (± 316601) 1.02
eval-15 140944323 ns/iter (± 840104) 139040080 ns/iter (± 514338) 1.01
eval-30 270431546 ns/iter (± 433155) 266873798 ns/iter (± 254469) 1.01
join 15574 ns/iter (± 125) 15490 ns/iter (± 33) 1.01
simple 3819 ns/iter (± 11) 3938 ns/iter (± 6) 0.97
simple-no 786 ns/iter (± 0) 743 ns/iter (± 0) 1.06
numbers 44 ns/iter (± 0) 44 ns/iter (± 0) 1
parse-simple 729 ns/iter (± 1) 735 ns/iter (± 0) 0.99
parse-ion 2500 ns/iter (± 13) 2221 ns/iter (± 2) 1.13
parse-group 7840 ns/iter (± 13) 7306 ns/iter (± 14) 1.07
parse-complex 20152 ns/iter (± 48) 20127 ns/iter (± 44) 1.00
parse-complex-fexpr 29667 ns/iter (± 62) 29077 ns/iter (± 64) 1.02

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.