Skip to content

Commit c848839

Browse files
bitnerbitner
authored
Further reductions for and/or (#78)
* remove duplicates in and/or args, flatten nested and/or args, short circuits for any false for and or any true for or. fixes #77 * if we reduce all the way to a single expression, remove the and and just return the expression
1 parent a39b3f2 commit c848839

File tree

3 files changed

+90
-34
lines changed

3 files changed

+90
-34
lines changed

‎src/expr.rs‎

Lines changed: 77 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use pg_escape::{quote_identifier, quote_literal};
77
use serde::{Deserialize, Serialize};
88
use serde_json::Value;
99
use std::collections::HashSet;
10+
use std::fmt::Debug;
1011
use std::ops::{Add, Deref};
1112
use std::str::FromStr;
1213
use unaccent::unaccent;
@@ -245,6 +246,11 @@ impl Expr {
245246
/// let toexpr: Expr = Expr::from_str("20").unwrap();
246247
/// assert_eq!(reduced, toexpr);
247248
///
249+
/// let fromexpr: Expr = Expr::from_str("(bork=1) and (bork=1) and (bork=1 and true)").unwrap();
250+
/// let reduced = fromexpr.reduce(Some(&item)).unwrap();
251+
/// let toexpr: Expr = Expr::from_str("bork=1").unwrap();
252+
/// assert_eq!(reduced, toexpr);
253+
///
248254
/// ```
249255
pub fn reduce(self, j: Option<&Value>) -> Result<Expr, Error> {
250256
match self {
@@ -270,19 +276,55 @@ impl Expr {
270276
.collect::<Result<_, _>>()?;
271277

272278
if BOOLOPS.contains(&op.as_str()) {
273-
let bools: Result<Vec<bool>, Error> = args
274-
.iter()
275-
.map(|expr| bool::try_from(expr.as_ref()))
276-
.collect();
277-
278-
if let Ok(bools) = bools {
279-
match op.as_str() {
280-
"and" => Ok(Expr::Bool(bools.into_iter().all(|x| x))),
281-
"or" => Ok(Expr::Bool(bools.into_iter().any(|x| x))),
282-
_ => Ok(Expr::Operation { op, args }),
279+
let curop = op.clone();
280+
let mut dedupargs: Vec<Box<Expr>> = vec![];
281+
let mut nestedargs: Vec<Box<Expr>> = vec![];
282+
for a in args {
283+
match *a {
284+
Expr::Operation { op, args } if op == curop => {
285+
nestedargs.append(&mut args.clone());
286+
}
287+
_ => {
288+
dedupargs.push(a.clone());
289+
}
290+
}
291+
}
292+
dedupargs.append(&mut nestedargs);
293+
dedupargs.sort_by(|a, b| a.partial_cmp(b).unwrap());
294+
dedupargs.dedup();
295+
296+
let mut anytrue: bool = false;
297+
let mut anyfalse: bool = false;
298+
let mut anyexp: bool = false;
299+
300+
for a in dedupargs.iter() {
301+
let b = bool::try_from(a.as_ref());
302+
match b {
303+
Ok(true) => {
304+
anytrue = true;
305+
}
306+
Ok(false) => {
307+
anyfalse = true;
308+
}
309+
_ => {
310+
anyexp = true;
311+
}
283312
}
313+
}
314+
if op == "and" && anytrue {
315+
dedupargs.retain(|x| !bool::try_from(x.as_ref()).unwrap_or(false));
316+
}
317+
if dedupargs.len() == 1 {
318+
Ok(dedupargs.pop().unwrap().as_ref().clone())
319+
} else if (op == "and" && anyfalse) || (op == "or" && !anytrue && !anyexp) {
320+
Ok(Expr::Bool(false))
321+
} else if (op == "and" && !anyfalse && !anyexp) || (op == "or" && anytrue) {
322+
Ok(Expr::Bool(true))
284323
} else {
285-
Ok(Expr::Operation { op, args })
324+
Ok(Expr::Operation {
325+
op,
326+
args: dedupargs.clone(),
327+
})
286328
}
287329
} else if op == "not" {
288330
match args[0].deref() {
@@ -308,26 +350,30 @@ impl Expr {
308350
let left = args[0].deref().clone();
309351
let right = args[1].deref().clone();
310352

311-
if SPATIALOPS.contains(&op.as_str()) {
312-
Ok(spatial_op(left, right, &op)
313-
.unwrap_or_else(|_| Expr::Operation { op, args }))
314-
} else if TEMPORALOPS.contains(&op.as_str()) {
315-
Ok(temporal_op(left, right, &op)
316-
.unwrap_or_else(|_| Expr::Operation { op, args }))
317-
} else if ARITHOPS.contains(&op.as_str()) {
318-
Ok(arith_op(left, right, &op)
319-
.unwrap_or_else(|_| Expr::Operation { op, args }))
320-
} else if EQOPS.contains(&op.as_str()) || CMPOPS.contains(&op.as_str()) {
321-
Ok(cmp_op(left, right, &op)
322-
.unwrap_or_else(|_| Expr::Operation { op, args }))
323-
} else if ARRAYOPS.contains(&op.as_str()) {
324-
Ok(array_op(left, right, &op)
325-
.unwrap_or_else(|_| Expr::Operation { op, args }))
326-
} else if op == "like" {
327-
let l: String = left.try_into()?;
328-
let r: String = right.try_into()?;
329-
let m: bool = Like::<true>::like(l.as_str(), r.as_str())?;
330-
Ok(Expr::Bool(m))
353+
if std::mem::discriminant(&left) == std::mem::discriminant(&right) {
354+
if SPATIALOPS.contains(&op.as_str()) {
355+
Ok(spatial_op(left, right, &op)
356+
.unwrap_or_else(|_| Expr::Operation { op, args }))
357+
} else if TEMPORALOPS.contains(&op.as_str()) {
358+
Ok(temporal_op(left, right, &op)
359+
.unwrap_or_else(|_| Expr::Operation { op, args }))
360+
} else if ARITHOPS.contains(&op.as_str()) {
361+
Ok(arith_op(left, right, &op)
362+
.unwrap_or_else(|_| Expr::Operation { op, args }))
363+
} else if EQOPS.contains(&op.as_str()) || CMPOPS.contains(&op.as_str()) {
364+
Ok(cmp_op(left, right, &op)
365+
.unwrap_or_else(|_| Expr::Operation { op, args }))
366+
} else if ARRAYOPS.contains(&op.as_str()) {
367+
Ok(array_op(left, right, &op)
368+
.unwrap_or_else(|_| Expr::Operation { op, args }))
369+
} else if op == "like" {
370+
let l: String = left.try_into()?;
371+
let r: String = right.try_into()?;
372+
let m: bool = Like::<true>::like(l.as_str(), r.as_str())?;
373+
Ok(Expr::Bool(m))
374+
} else {
375+
Ok(Expr::Operation { op, args })
376+
}
331377
} else if op == "in" {
332378
let l: String = left.to_text()?;
333379
let r: HashSet<String> = right.try_into()?;

‎tests/reductions.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,13 @@ POINT(0 0) = POINT(0 0)
5858
true
5959
POINT(0 0) = POINT(1 0)
6060
false
61+
a and b and c and a and a
62+
a and b and c
63+
true or b = false
64+
true
65+
true or false or c=3
66+
true
67+
false or c=3 or c=3
68+
c=3 or false
69+
true and c=3
70+
c=3

‎uv.lock‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)