Skip to content

fix(cubesql): Fix incorrect datetime parsing in filters rewrite rules #9732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 1, 2025
Merged

fix(cubesql): Fix incorrect datetime parsing in filters rewrite rules #9732

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9220bd2
fix(cubesql): Fix incorrect datetime parsing in filters rewrite rules
KSDaemon Jun 30, 2025
e13d29e
remove copy/paste
KSDaemon Jun 30, 2025
b75ec12
add test
KSDaemon Jun 30, 2025
1e8646a
fix suggestions
KSDaemon Jun 30, 2025
f9cf303
fix map err
KSDaemon Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions rust/cubesql/cubesql/src/compile/date_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use chrono::ParseError;
use chrono::{NaiveDate, NaiveDateTime};

pub fn parse_date_str(s: &str) -> Result<NaiveDateTime, ParseError> {
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())
})
}
23 changes: 4 additions & 19 deletions rust/cubesql/cubesql/src/compile/engine/df/scan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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::{
Expand Down Expand Up @@ -917,15 +918,7 @@ pub fn transform_response<V: ValueObject>(
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: '{}': {}",
Expand Down Expand Up @@ -959,15 +952,7 @@ pub fn transform_response<V: ValueObject>(
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: '{}': {}",
Expand Down
46 changes: 44 additions & 2 deletions rust/cubesql/cubesql/src/compile/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub mod service;
pub mod session;

// Internal API
mod date_parser;
pub mod test;

// Re-export for Public API
Expand Down Expand Up @@ -15541,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();
Expand Down Expand Up @@ -15584,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,
Expand All @@ -15594,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,
Expand Down
48 changes: 27 additions & 21 deletions rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::utils;
use crate::compile::date_parser::parse_date_str;
use crate::{
compile::rewrite::{
alias_expr,
Expand Down Expand Up @@ -36,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::{
Expand Down Expand Up @@ -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<String> {
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<String> {
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<String> {
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
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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;
};
Expand Down
Loading