From 9caac0c8128a894c08b9bdbd2b3bf6ce625bab5e Mon Sep 17 00:00:00 2001 From: stefan-gorules <127550877+stefan-gorules@users.noreply.github.com> Date: Sat, 1 Jul 2023 16:59:09 +0200 Subject: [PATCH] fix: expression date range; (#49) * fix: expression date range; * restore tests; update v8; * perf: add precomputation for interval; add standard tests; --- core/engine/Cargo.toml | 4 +-- core/expression/src/parser/iter.rs | 24 +++++++++++++ core/expression/src/parser/standard/mod.rs | 40 +++++++++++++++++----- core/expression/src/parser/unary/mod.rs | 40 +++++++++++++++++----- core/expression/tests/isolate.rs | 28 +++++++++++++++ 5 files changed, 116 insertions(+), 20 deletions(-) diff --git a/core/engine/Cargo.toml b/core/engine/Cargo.toml index dac70116..9dc0bb24 100644 --- a/core/engine/Cargo.toml +++ b/core/engine/Cargo.toml @@ -18,10 +18,10 @@ async-trait = { workspace = true } bincode = { workspace = true, optional = true } serde_json = { workspace = true, features = ["arbitrary_precision"] } serde = { version = "1.0.163", features = ["derive"] } -serde_v8 = { version = "0.100.0" } +serde_v8 = { version = "0.103.0" } once_cell = { version = "1.17.2" } futures = "0.3.28" -v8 = { version = "0.73.0" } +v8 = { version = "0.74.0" } zen-expression = { path = "../expression", version = "0.5.2" } [dev-dependencies] diff --git a/core/expression/src/parser/iter.rs b/core/expression/src/parser/iter.rs index ab4eb67d..781bba7c 100644 --- a/core/expression/src/parser/iter.rs +++ b/core/expression/src/parser/iter.rs @@ -17,25 +17,48 @@ pub(crate) struct ParserIterator<'a, 'b> { position: Cell, bump: &'b Bump, is_done: Cell, + has_interval: bool, } impl<'a, 'b> ParserIterator<'a, 'b> { pub fn try_new(tokens: &'a Vec>, bump: &'b Bump) -> Result { let current = tokens.get(0).ok_or(ParserError::TokenOutOfBounds)?; + let has_interval = tokens + .iter() + .any(|t| t.kind == TokenKind::Operator && t.value == ".."); Ok(Self { tokens, bump, + has_interval, current: Cell::new(current), position: Cell::new(0), is_done: Cell::new(false), }) } + pub fn has_interval(&self) -> bool { + self.has_interval + } + pub fn current(&self) -> &'a Token<'a> { self.current.get() } + pub fn position(&self) -> usize { + self.position.get() + } + + pub fn set_position(&self, position: usize) -> ParserResult<()> { + let Some(token) = self.tokens.get(position) else { + return Err(ParserError::TokenOutOfBounds) + }; + + self.position.set(position); + self.current.set(token); + Ok(()) + } + pub fn is_done(&self) -> bool { self.is_done.get() } @@ -66,6 +89,7 @@ impl<'a, 'b> ParserIterator<'a, 'b> { Ok(()) } + #[allow(dead_code)] pub fn lookup(&self, dx: usize, kind: TokenKind, values: TokenValues<'a>) -> bool { self.token_cmp_at_bool(self.position.get() + dx, kind, values) } diff --git a/core/expression/src/parser/standard/mod.rs b/core/expression/src/parser/standard/mod.rs index 0567d4ca..f3335bef 100644 --- a/core/expression/src/parser/standard/mod.rs +++ b/core/expression/src/parser/standard/mod.rs @@ -178,21 +178,43 @@ where } fn parse_interval(&self) -> ParserResult>> { + // Performance optimisation: skip if expression does not contain an interval for faster evaluation + if !self.iterator.has_interval() { + return Ok(None); + } + if self.iterator.current().kind != TokenKind::Bracket { return Ok(None); } - if !self.iterator.lookup(2, TokenKind::Operator, Some(&[".."])) { - return Ok(None); - } - + let initial_position = self.iterator.position(); let left_bracket = self.iterator.current().value; - self.iterator.expect(TokenKind::Bracket, None)?; - let left = self.parse_primary_expression()?; - self.iterator.expect(TokenKind::Operator, Some(&[".."]))?; - let right = self.parse_primary_expression()?; + if let Err(_) = self.iterator.expect(TokenKind::Bracket, None) { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + + let Ok(left) = self.parse_primary_expression() else { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + + if let Err(_) = self.iterator.expect(TokenKind::Operator, Some(&[".."])) { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + + let Ok(right) = self.parse_primary_expression() else { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + let right_bracket = self.iterator.current().value; - self.iterator.expect(TokenKind::Bracket, None)?; + + if let Err(_) = self.iterator.expect(TokenKind::Bracket, None) { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; let interval_node = self.iterator.node(Node::Interval { left_bracket: self.iterator.str_value(left_bracket), diff --git a/core/expression/src/parser/unary/mod.rs b/core/expression/src/parser/unary/mod.rs index 81f6f467..febc8577 100644 --- a/core/expression/src/parser/unary/mod.rs +++ b/core/expression/src/parser/unary/mod.rs @@ -135,27 +135,49 @@ where } fn parse_interval(&self, node: &'b Node<'b>) -> ParserResult>> { + // Performance optimisation: skip if expression does not contain an interval for faster evaluation + if !self.iterator.has_interval() { + return Ok(None); + } + let current_token = self.iterator.current(); if current_token.kind != TokenKind::Bracket { return Ok(None); } - if !self.iterator.lookup(2, TokenKind::Operator, Some(&[".."])) { - return Ok(None); - } - + let initial_position = self.iterator.position(); let should_wrap = !self .iterator .lookup_back(1, TokenKind::Operator, Some(&["not in", "in"])); let left_bracket = self.iterator.current().value; - self.iterator.expect(TokenKind::Bracket, None)?; - let left = self.parse_primary()?; - self.iterator.expect(TokenKind::Operator, Some(&[".."]))?; - let right = self.parse_primary()?; + if let Err(_) = self.iterator.expect(TokenKind::Bracket, None) { + self.iterator.set_position(initial_position)?; + return Ok(None); + } + + let Ok(left) = self.parse_primary() else { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + + if let Err(_) = self.iterator.expect(TokenKind::Operator, Some(&[".."])) { + self.iterator.set_position(initial_position)?; + return Ok(None); + } + + let Ok(right) = self.parse_primary() else { + self.iterator.set_position(initial_position)?; + return Ok(None); + }; + let right_bracket = self.iterator.current().value; - self.iterator.expect(TokenKind::Bracket, None)?; + + if let Err(_) = self.iterator.expect(TokenKind::Bracket, None) { + self.iterator.set_position(initial_position)?; + return Ok(None); + } let interval_node = self.iterator.node(Node::Interval { left, diff --git a/core/expression/tests/isolate.rs b/core/expression/tests/isolate.rs index 7f97ece9..bf39d910 100644 --- a/core/expression/tests/isolate.rs +++ b/core/expression/tests/isolate.rs @@ -128,6 +128,14 @@ fn isolate_standard_test() { expr: r#"date("2022-04-04") > date("2022-03-04")"#, result: json!(true), }, + TestCase { + expr: r#"date("2022-04-04") in [date("2022-03-04")..date("2022-04-04")]"#, + result: json!(true), + }, + TestCase { + expr: r#"date("2022-04-04") in [date("2022-03-04")..date("2022-04-04"))"#, + result: json!(false), + }, TestCase { expr: r#"time("2022-04-04T21:48:30Z") > time("2022-05-04 21:48:20")"#, result: json!(true), @@ -513,6 +521,26 @@ fn isolate_unary_tests() { result: json!(true), }]), }, + UnaryTestEnv { + env: json!({ + "input": "2023-01-02" + }), + reference: "date(input)", + cases: Vec::from([ + TestCase { + expr: r#"[date("2023-01-01")..date("2023-01-03")]"#, + result: json!(true), + }, + TestCase { + expr: r#"[date("2023-02-01")..date("2023-01-03")]"#, + result: json!(false), + }, + TestCase { + expr: r#"$ in [date("2023-01-01")..date("2023-01-03")]"#, + result: json!(true), + }, + ]), + }, UnaryTestEnv { env: json!({ "customer": {