Skip to content

Commit

Permalink
Improvements to Value iterators
Browse files Browse the repository at this point in the history
  • Loading branch information
jpschorr committed Aug 8, 2023
1 parent 191b10e commit 39e856e
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 24 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
4 changes: 2 additions & 2 deletions partiql-eval/src/eval/evaluable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,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 @@ -956,7 +956,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
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 @@ -75,12 +75,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 @@ -182,7 +182,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 @@ -689,7 +689,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 @@ -1154,7 +1154,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 @@ -1216,7 +1216,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 @@ -1280,7 +1280,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
132 changes: 121 additions & 11 deletions partiql-value/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::borrow::Cow;
use std::fmt::{Debug, Formatter};
use std::hash::Hash;

use std::iter::Once;
use std::{ops, vec};

use rust_decimal::prelude::FromPrimitive;
Expand Down Expand Up @@ -553,13 +554,25 @@ impl Value {
}

#[inline]
pub fn coerce_to_tuple(self) -> Tuple {
pub fn coerce_into_tuple(self) -> Tuple {
match self {
Value::Tuple(t) => *t,
Value::Missing => tuple![],
_ => self
.into_bindings()
.map(|(k, v)| (k.unwrap_or_else(|| "_1".to_string()), v))
.collect(),
}
}

#[inline]
pub fn coerce_to_tuple(&self) -> Tuple {
match self {
Value::Tuple(t) => t.as_ref().clone(),
_ => {
let fresh_key = "_1"; // TODO don't hard-code 'fresh' keys
tuple![(fresh_key, self)]
let fresh = "_1".to_string();
self.as_bindings()
.map(|(k, v)| (k.unwrap_or(&fresh), v.clone()))
.collect()
}
}
}
Expand All @@ -569,12 +582,30 @@ impl Value {
if let Value::Tuple(t) = self {
Cow::Borrowed(t)
} else {
Cow::Owned(self.clone().coerce_to_tuple())
Cow::Owned(self.coerce_to_tuple())
}
}

#[inline]
pub fn as_bindings(&self) -> BindingIter {
match self {
Value::Tuple(t) => BindingIter::Tuple(t.pairs()),
Value::Missing => BindingIter::Empty,
_ => BindingIter::Single(std::iter::once(self)),
}
}

#[inline]
pub fn into_bindings(self) -> BindingIntoIter {
match self {
Value::Tuple(t) => BindingIntoIter::Tuple(t.into_pairs()),
Value::Missing => BindingIntoIter::Empty,
_ => BindingIntoIter::Single(std::iter::once(self)),
}
}

#[inline]
pub fn coerce_to_bag(self) -> Bag {
pub fn coerce_into_bag(self) -> Bag {
if let Value::Bag(b) = self {
*b
} else {
Expand All @@ -587,12 +618,12 @@ impl Value {
if let Value::Bag(b) = self {
Cow::Borrowed(b)
} else {
Cow::Owned(self.clone().coerce_to_bag())
Cow::Owned(self.clone().coerce_into_bag())
}
}

#[inline]
pub fn coerce_to_list(self) -> List {
pub fn coerce_into_list(self) -> List {
if let Value::List(b) = self {
*b
} else {
Expand All @@ -605,7 +636,7 @@ impl Value {
if let Value::List(l) = self {
Cow::Borrowed(l)
} else {
Cow::Owned(self.clone().coerce_to_list())
Cow::Owned(self.clone().coerce_into_list())
}
}

Expand All @@ -629,6 +660,64 @@ impl Value {
}
}

#[derive(Debug, Clone)]
pub enum BindingIter<'a> {
Tuple(PairsIter<'a>),
Single(Once<&'a Value>),
Empty,
}

impl<'a> Iterator for BindingIter<'a> {
type Item = (Option<&'a String>, &'a Value);

#[inline]
fn next(&mut self) -> Option<Self::Item> {
match self {
BindingIter::Tuple(t) => t.next().map(|(k, v)| (Some(k), v)),
BindingIter::Single(single) => single.next().map(|v| (None, v)),
BindingIter::Empty => None,
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
BindingIter::Tuple(t) => t.size_hint(),
BindingIter::Single(_single) => (1, Some(1)),
BindingIter::Empty => (0, Some(0)),
}
}
}

#[derive(Debug)]
pub enum BindingIntoIter {
Tuple(PairsIntoIter),
Single(Once<Value>),
Empty,
}

impl Iterator for BindingIntoIter {
type Item = (Option<String>, Value);

#[inline]
fn next(&mut self) -> Option<Self::Item> {
match self {
BindingIntoIter::Tuple(t) => t.next().map(|(k, v)| (Some(k), v)),
BindingIntoIter::Single(single) => single.next().map(|v| (None, v)),
BindingIntoIter::Empty => None,
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
BindingIntoIter::Tuple(t) => t.size_hint(),
BindingIntoIter::Single(_single) => (1, Some(1)),
BindingIntoIter::Empty => (0, Some(0)),
}
}
}

#[derive(Debug, Clone)]
pub enum ValueIter<'a> {
List(ListIter<'a>),
Expand All @@ -639,19 +728,30 @@ pub enum ValueIter<'a> {
impl<'a> Iterator for ValueIter<'a> {
type Item = &'a Value;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
match self {
ValueIter::List(list) => list.next(),
ValueIter::Bag(bag) => bag.next(),
ValueIter::Single(v) => v.take(),
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
ValueIter::List(list) => list.size_hint(),
ValueIter::Bag(bag) => bag.size_hint(),
ValueIter::Single(_) => (1, Some(1)),
}
}
}

impl IntoIterator for Value {
type Item = Value;
type IntoIter = ValueIntoIterator;

#[inline]
fn into_iter(self) -> ValueIntoIterator {
match self {
Value::List(list) => ValueIntoIterator::List(list.into_iter()),
Expand All @@ -670,13 +770,23 @@ pub enum ValueIntoIterator {
impl Iterator for ValueIntoIterator {
type Item = Value;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
match self {
ValueIntoIterator::List(list) => list.next(),
ValueIntoIterator::Bag(bag) => bag.next(),
ValueIntoIterator::Single(v) => v.take(),
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
match self {
ValueIntoIterator::List(list) => list.size_hint(),
ValueIntoIterator::Bag(bag) => bag.size_hint(),
ValueIntoIterator::Single(_) => (1, Some(1)),
}
}
}

// TODO make debug emit proper PartiQL notation
Expand Down Expand Up @@ -1038,9 +1148,9 @@ mod tests {

let mut pairs = tuple.pairs();
let list_val = Value::from(list);
assert_eq!(pairs.next(), Some(("list", &list_val)));
assert_eq!(pairs.next(), Some((&"list".to_string(), &list_val)));
let bag_val = Value::from(bag);
assert_eq!(pairs.next(), Some(("bag", &bag_val)));
assert_eq!(pairs.next(), Some((&"bag".to_string(), &bag_val)));
assert_eq!(pairs.next(), None);
}

Expand Down
Loading

0 comments on commit 39e856e

Please sign in to comment.