From 9220bd272279b0d903bcc3b74071790f4eee66ea Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 30 Jun 2025 22:09:20 +0300 Subject: [PATCH 1/5] fix(cubesql): Fix incorrect datetime parsing in filters rewrite rules --- .../cubesql/src/compile/date_parser.rs | 12 +++++ .../src/compile/rewrite/rules/filters.rs | 46 +++++++++++-------- 2 files changed, 38 insertions(+), 20 deletions(-) create mode 100644 rust/cubesql/cubesql/src/compile/date_parser.rs diff --git a/rust/cubesql/cubesql/src/compile/date_parser.rs b/rust/cubesql/cubesql/src/compile/date_parser.rs new file mode 100644 index 0000000000000..88d1462e7ec6a --- /dev/null +++ b/rust/cubesql/cubesql/src/compile/date_parser.rs @@ -0,0 +1,12 @@ +use chrono::ParseError; +use chrono::{NaiveDate, NaiveDateTime}; + +pub fn parse_date_str(s: &str) -> Result { + NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S%.f") + .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S%.f")) + .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S")) + .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S%.fZ")) + .or_else(|_| { + NaiveDate::parse_from_str(s, "%Y-%m-%d").map(|date| date.and_hms_opt(0, 0, 0).unwrap()) + }) +} diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs index c1cb83075fce6..aef47a5b18d6b 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs @@ -1,4 +1,5 @@ use super::utils; +use crate::compile::date_parser::parse_date_str; use crate::{ compile::rewrite::{ alias_expr, @@ -4568,36 +4569,36 @@ impl FilterRules { let date_range_start_op_var = date_range_start_op_var.parse().unwrap(); let date_range_end_op_var = date_range_end_op_var.parse().unwrap(); move |egraph, subst| { - fn resolve_time_delta(date_var: &String, op: &String) -> String { + fn resolve_time_delta(date_var: &String, op: &String) -> Option { if op == "afterDate" { return increment_iso_timestamp_time(date_var); } else if op == "beforeDate" { return decrement_iso_timestamp_time(date_var); } else { - return date_var.clone(); + return Some(date_var.clone()); } } - fn increment_iso_timestamp_time(date_var: &String) -> String { - let timestamp = NaiveDateTime::parse_from_str(date_var, "%Y-%m-%dT%H:%M:%S%.fZ"); + fn increment_iso_timestamp_time(date_var: &String) -> Option { + let timestamp = parse_date_str(date_var); let value = match timestamp { Ok(val) => format_iso_timestamp( val.checked_add_signed(Duration::milliseconds(1)).unwrap(), ), - Err(_) => date_var.clone(), + Err(_) => return None, }; - return value; + return Some(value); } - fn decrement_iso_timestamp_time(date_var: &String) -> String { - let timestamp = NaiveDateTime::parse_from_str(date_var, "%Y-%m-%dT%H:%M:%S%.fZ"); + fn decrement_iso_timestamp_time(date_var: &String) -> Option { + let timestamp = parse_date_str(date_var); let value = match timestamp { Ok(val) => format_iso_timestamp( val.checked_sub_signed(Duration::milliseconds(1)).unwrap(), ), - Err(_) => date_var.clone(), + Err(_) => return None, }; - return value; + return Some(value); } for date_range_start in @@ -4630,10 +4631,20 @@ impl FilterRules { } let mut result = Vec::new(); - let resolved_start_date = - resolve_time_delta(&date_range_start[0], date_range_start_op); - let resolved_end_date = - resolve_time_delta(&date_range_end[0], date_range_end_op); + let resolved_start_date = if let Some(rtd) = + resolve_time_delta(&date_range_start[0], date_range_start_op) + { + rtd + } else { + return false; + }; + let resolved_end_date = if let Some(rtd) = + resolve_time_delta(&date_range_end[0], date_range_end_op) + { + rtd + } else { + return false; + }; if swap_left_and_right { result.extend(vec![resolved_end_date]); @@ -5222,12 +5233,7 @@ impl FilterRules { let Some(str) = str else { return Some(None); }; - let dt = NaiveDateTime::parse_from_str(str, "%Y-%m-%d %H:%M:%S%.f") - .or_else(|_| NaiveDateTime::parse_from_str(str, "%Y-%m-%d %H:%M:%S")) - .or_else(|_| { - NaiveDate::parse_from_str(str, "%Y-%m-%d") - .map(|date| date.and_hms_opt(0, 0, 0).unwrap()) - }); + let dt = parse_date_str(str.as_str()); let Ok(dt) = dt else { return None; }; From e13d29e49dacd08408433932490f39997cd718eb Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 30 Jun 2025 22:09:41 +0300 Subject: [PATCH 2/5] remove copy/paste --- .../cubesql/src/compile/engine/df/scan.rs | 23 ++++--------------- rust/cubesql/cubesql/src/compile/mod.rs | 1 + .../src/compile/rewrite/rules/filters.rs | 2 +- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/engine/df/scan.rs b/rust/cubesql/cubesql/src/compile/engine/df/scan.rs index 4c37489961dae..fc12f8b08442b 100644 --- a/rust/cubesql/cubesql/src/compile/engine/df/scan.rs +++ b/rust/cubesql/cubesql/src/compile/engine/df/scan.rs @@ -28,6 +28,7 @@ use std::{ task::{Context, Poll}, }; +use crate::compile::date_parser::parse_date_str; use crate::{ compile::{ engine::df::wrapper::{CubeScanWrappedSqlNode, CubeScanWrapperNode, SqlQuery}, @@ -38,7 +39,7 @@ use crate::{ transport::{CubeStreamReceiver, LoadRequestMeta, SpanId, TransportService}, CubeError, }; -use chrono::{Datelike, NaiveDate, NaiveDateTime}; +use chrono::{Datelike, NaiveDate}; use datafusion::{ arrow::{ array::{ @@ -917,15 +918,7 @@ pub fn transform_response( field_name, { (FieldValue::String(s), builder) => { - let timestamp = NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S%.f") - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%d %H:%M:%S%.f")) - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S")) - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S%.fZ")) - .or_else(|_| { - NaiveDate::parse_from_str(s.as_ref(), "%Y-%m-%d").map(|date| { - date.and_hms_opt(0, 0, 0).unwrap() - }) - }) + let timestamp = parse_date_str(s.as_ref()) .map_err(|e| { DataFusionError::Execution(format!( "Can't parse timestamp: '{}': {}", @@ -959,15 +952,7 @@ pub fn transform_response( field_name, { (FieldValue::String(s), builder) => { - let timestamp = NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S%.f") - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%d %H:%M:%S%.f")) - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S")) - .or_else(|_| NaiveDateTime::parse_from_str(s.as_ref(), "%Y-%m-%dT%H:%M:%S%.fZ")) - .or_else(|_| { - NaiveDate::parse_from_str(s.as_ref(), "%Y-%m-%d").map(|date| { - date.and_hms_opt(0, 0, 0).unwrap() - }) - }) + let timestamp = parse_date_str(s.as_ref()) .map_err(|e| { DataFusionError::Execution(format!( "Can't parse timestamp: '{}': {}", diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index 9da6c6b5facbd..58c72ef540ac9 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -14,6 +14,7 @@ pub mod service; pub mod session; // Internal API +mod date_parser; pub mod test; // Re-export for Public API diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs index aef47a5b18d6b..58ed47674b49a 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs @@ -37,7 +37,7 @@ use chrono::{ Numeric::{Day, Hour, Minute, Month, Second, Year}, Pad::Zero, }, - DateTime, Datelike, Days, Duration, Months, NaiveDate, NaiveDateTime, Timelike, Weekday, + DateTime, Datelike, Days, Duration, Months, NaiveDateTime, Timelike, Weekday, }; use cubeclient::models::V1CubeMeta; use datafusion::{ From b75ec12bcd2678e26483170cdc174366eac46ea0 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 30 Jun 2025 22:27:37 +0300 Subject: [PATCH 3/5] add test --- rust/cubesql/cubesql/src/compile/mod.rs | 45 +++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index 58c72ef540ac9..3f7e0e037cdf3 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -15542,6 +15542,47 @@ LIMIT {{ limit }}{% endif %}"#.to_string(), ); } + #[tokio::test] + async fn test_daterange_filter_literals() -> Result<(), CubeError> { + init_testing_logger(); + + let query_plan = convert_select_to_query_plan( + // language=PostgreSQL + r#"SELECT + DATE_TRUNC('month', order_date) AS order_date, + COUNT(*) AS month_count + FROM "KibanaSampleDataEcommerce" ecom + WHERE ecom.order_date >= '2025-01-01' and ecom.order_date < '2025-02-01' + GROUP BY 1"# + .to_string(), + DatabaseProtocol::PostgreSQL, + ) + .await; + + let logical_plan = query_plan.as_logical_plan(); + assert_eq!( + logical_plan.find_cube_scan().request, + V1LoadRequestQuery { + measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), + segments: Some(vec![]), + dimensions: Some(vec![]), + time_dimensions: Some(vec![V1LoadRequestQueryTimeDimension { + dimension: "KibanaSampleDataEcommerce.order_date".to_owned(), + granularity: Some("month".to_string()), + date_range: Some(json!(vec![ + // WHY NOT "2025-01-01T00:00:00.000Z".to_string(), ? + "2025-01-01".to_string(), + "2025-01-31T23:59:59.999Z".to_string() + ])), + }]), + order: Some(vec![]), + ..Default::default() + } + ); + + Ok(()) + } + #[tokio::test] async fn test_time_dimension_range_filter_chain_or() { init_testing_logger(); @@ -15585,7 +15626,7 @@ LIMIT {{ limit }}{% endif %}"#.to_string(), operator: Some("inDateRange".to_string()), values: Some(vec![ "2019-01-01 00:00:00.0".to_string(), - "2020-01-01 00:00:00.0".to_string(), + "2019-12-31T23:59:59.999Z".to_string(), ]), or: None, and: None, @@ -15595,7 +15636,7 @@ LIMIT {{ limit }}{% endif %}"#.to_string(), operator: Some("inDateRange".to_string()), values: Some(vec![ "2021-01-01 00:00:00.0".to_string(), - "2022-01-01 00:00:00.0".to_string(), + "2021-12-31T23:59:59.999Z".to_string(), ]), or: None, and: None, From 1e8646a4bc8fbfb3ebbf11dcb4135a9c369a0326 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 1 Jul 2025 00:10:13 +0300 Subject: [PATCH 4/5] fix suggestions --- rust/cubesql/cubesql/src/compile/date_parser.rs | 12 ++++++++---- .../cubesql/src/compile/rewrite/rules/filters.rs | 12 ++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/date_parser.rs b/rust/cubesql/cubesql/src/compile/date_parser.rs index 88d1462e7ec6a..77900a3c0f59d 100644 --- a/rust/cubesql/cubesql/src/compile/date_parser.rs +++ b/rust/cubesql/cubesql/src/compile/date_parser.rs @@ -1,12 +1,16 @@ -use chrono::ParseError; +use crate::compile::engine::df::scan::DataFusionError; use chrono::{NaiveDate, NaiveDateTime}; -pub fn parse_date_str(s: &str) -> Result { - NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S%.f") +pub fn parse_date_str(s: &str) -> Result { + let parsed = NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S%.f") .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S%.f")) .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S")) .or_else(|_| NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S%.fZ")) .or_else(|_| { NaiveDate::parse_from_str(s, "%Y-%m-%d").map(|date| date.and_hms_opt(0, 0, 0).unwrap()) - }) + }); + + parsed.map_err(|e| { + DataFusionError::Internal(format!("Can't parse date/time string literal: {}", e)) + }) } diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs index 58ed47674b49a..f1525c66b62de 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs @@ -4631,18 +4631,14 @@ impl FilterRules { } let mut result = Vec::new(); - let resolved_start_date = if let Some(rtd) = + let Some(resolved_start_date) = resolve_time_delta(&date_range_start[0], date_range_start_op) - { - rtd - } else { + else { return false; }; - let resolved_end_date = if let Some(rtd) = + let Some(resolved_end_date) = resolve_time_delta(&date_range_end[0], date_range_end_op) - { - rtd - } else { + else { return false; }; From f9cf3035f64ea8d5df73e8b02ec0c6c3250c486e Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 1 Jul 2025 00:27:18 +0300 Subject: [PATCH 5/5] fix map err --- .../cubesql/src/compile/engine/df/scan.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/engine/df/scan.rs b/rust/cubesql/cubesql/src/compile/engine/df/scan.rs index fc12f8b08442b..19d6ccf8b2527 100644 --- a/rust/cubesql/cubesql/src/compile/engine/df/scan.rs +++ b/rust/cubesql/cubesql/src/compile/engine/df/scan.rs @@ -918,13 +918,7 @@ pub fn transform_response( field_name, { (FieldValue::String(s), builder) => { - let timestamp = parse_date_str(s.as_ref()) - .map_err(|e| { - DataFusionError::Execution(format!( - "Can't parse timestamp: '{}': {}", - s, e - )) - })?; + let timestamp = parse_date_str(s.as_ref())?; // TODO switch parsing to microseconds if timestamp.and_utc().timestamp_millis() > (((1i64) << 62) / 1_000_000) { builder.append_null()?; @@ -952,13 +946,7 @@ pub fn transform_response( field_name, { (FieldValue::String(s), builder) => { - let timestamp = parse_date_str(s.as_ref()) - .map_err(|e| { - DataFusionError::Execution(format!( - "Can't parse timestamp: '{}': {}", - s, e - )) - })?; + let timestamp = parse_date_str(s.as_ref())?; // TODO switch parsing to microseconds if timestamp.and_utc().timestamp_millis() > (((1 as i64) << 62) / 1_000_000) { builder.append_null()?;