Skip to content

Commit

Permalink
fix: disbale now in duration param
Browse files Browse the repository at this point in the history
  • Loading branch information
@Taylor-lagrange
Taylor-lagrange committed May 10, 2024
1 parent 75fd790 commit d7e3ec0
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 23 deletions.
67 changes: 61 additions & 6 deletions src/query/src/range_select/plan_rewrite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ fn parse_expr_to_string(args: &[Expr], i: usize) -> DFResult<String> {
/// Parse a duraion expr:
/// 1. duration string (e.g. `'1h'`)
/// 2. Interval expr (e.g. `INTERVAL '1 year 3 hours 20 minutes'`)
/// 3. An expr can be evaluated at the logical plan stage (e.g. `INTERVAL '2' day - INTERVAL '1' day`)
/// 3. An interval expr can be evaluated at the logical plan stage (e.g. `INTERVAL '2' day - INTERVAL '1' day`)
fn parse_duration_expr(args: &[Expr], i: usize) -> DFResult<Duration> {
match args.get(i) {
Some(Expr::Literal(ScalarValue::Utf8(Some(str)))) => {
parse_duration(str).map_err(DataFusionError::Plan)
}
Some(expr) => {
let ms = evaluate_expr_to_millisecond(args, i)?;
let ms = evaluate_expr_to_millisecond(args, i, true)?;
if ms <= 0 {
return Err(dispose_parse_error(Some(expr)));
}
Expand All @@ -131,13 +131,18 @@ fn parse_duration_expr(args: &[Expr], i: usize) -> DFResult<Duration> {

/// Evaluate a time calculation expr, case like:
/// 1. `INTERVAL '1' day + INTERVAL '1 year 2 hours 3 minutes'`
/// 2. `now() - INTERVAL '1' day`
/// 2. `now() - INTERVAL '1' day` (when `interval_only==false`)
///
/// Output a millisecond timestamp
fn evaluate_expr_to_millisecond(args: &[Expr], i: usize) -> DFResult<i64> {
///
/// if `interval_only==true`, only accept expr with all interval type (case 2 will return a error)
fn evaluate_expr_to_millisecond(args: &[Expr], i: usize, interval_only: bool) -> DFResult<i64> {
let Some(expr) = args.get(i) else {
return Err(dispose_parse_error(None));
};
if interval_only && !interval_only_in_expr(expr) {
return Err(dispose_parse_error(Some(expr)));
}
let execution_props = ExecutionProps::new().with_query_execution_start_time(Utc::now());
let info = SimplifyContext::new(&execution_props).with_schema(Arc::new(DFSchema::empty()));
let interval_to_ms =
Expand Down Expand Up @@ -183,7 +188,7 @@ fn evaluate_expr_to_millisecond(args: &[Expr], i: usize) -> DFResult<i64> {
/// 4. leave empty (as Default Option): align to unix epoch 0 (timezone aware)
fn parse_align_to(args: &[Expr], i: usize, timezone: Option<&Timezone>) -> DFResult<i64> {
let Ok(s) = parse_str_expr(args, i) else {
return evaluate_expr_to_millisecond(args, i);
return evaluate_expr_to_millisecond(args, i, false);
};
let upper = s.to_uppercase();
match upper.as_str() {
Expand Down Expand Up @@ -518,6 +523,25 @@ fn have_range_in_exprs(exprs: &[Expr]) -> bool {
})
}

fn interval_only_in_expr(expr: &Expr) -> bool {
let mut all_interval = true;
let _ = expr.apply(&mut |expr| {
if !matches!(
expr,
Expr::Literal(ScalarValue::IntervalDayTime(_))
| Expr::Literal(ScalarValue::IntervalMonthDayNano(_))
| Expr::Literal(ScalarValue::IntervalYearMonth(_))
| Expr::BinaryExpr(_)
) {
all_interval = false;
Ok(TreeNodeRecursion::Stop)
} else {
Ok(TreeNodeRecursion::Continue)
}
});
all_interval
}

#[cfg(test)]
mod test {

Expand Down Expand Up @@ -762,7 +786,7 @@ mod test {
.to_string();
assert_eq!(
error,
"Error during planning: Int64(5) is not a expr can be evaluate and use in range query"
"Error during planning: Illegal argument `Int64(5)` in range select query"
)
}

Expand Down Expand Up @@ -831,6 +855,15 @@ mod test {
})];
// test zero interval error
assert!(parse_duration_expr(&args, 0).is_err());
// test must all be interval
let args = vec![Expr::BinaryExpr(BinaryExpr {
left: Box::new(Expr::Literal(ScalarValue::IntervalYearMonth(Some(
Interval::from_year_month(10).to_i32(),
)))),
op: Operator::Minus,
right: Box::new(Expr::Literal(ScalarValue::Time64Microsecond(Some(0)))),
})];
assert!(parse_duration_expr(&args, 0).is_err());
}

#[test]
Expand Down Expand Up @@ -892,4 +925,26 @@ mod test {
20 * 30 * 24 * 60 * 60 * 1000
);
}

#[test]
fn test_interval_only() {
let expr = Expr::BinaryExpr(BinaryExpr {
left: Box::new(Expr::Literal(ScalarValue::DurationMillisecond(Some(20)))),
op: Operator::Minus,
right: Box::new(Expr::Literal(ScalarValue::IntervalYearMonth(Some(
Interval::from_year_month(10).to_i32(),
)))),
});
assert!(!interval_only_in_expr(&expr));
let expr = Expr::BinaryExpr(BinaryExpr {
left: Box::new(Expr::Literal(ScalarValue::IntervalYearMonth(Some(
Interval::from_year_month(10).to_i32(),
)))),
op: Operator::Minus,
right: Box::new(Expr::Literal(ScalarValue::IntervalYearMonth(Some(
Interval::from_year_month(10).to_i32(),
)))),
});
assert!(interval_only_in_expr(&expr));
}
}
26 changes: 13 additions & 13 deletions tests/cases/standalone/common/range/to.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,15 @@ SELECT ts, min(val) RANGE (INTERVAL '1' day) FROM host ALIGN (INTERVAL '1' day)
| 2024-01-24T23:00:00 | 3 |
+---------------------+------------------------------------------------------------------+

SELECT ts, min(val) RANGE (now() - (now() - INTERVAL '2' day + INTERVAL '1' day)) FROM host ALIGN (now() - (now() - INTERVAL '2' day + INTERVAL '1' day)) TO (now() - (now() + INTERVAL '1' hour)) by (1) ORDER BY ts;
SELECT ts, min(val) RANGE (INTERVAL '2' day - INTERVAL '1' day) FROM host ALIGN (INTERVAL '2' day - INTERVAL '1' day) TO (now() - (now() + INTERVAL '1' hour)) by (1) ORDER BY ts;

+---------------------+---------------------------------------------------------------------------------------------------------------------------------+
| ts | MIN(host.val) RANGE now() - now() - IntervalMonthDayNano("36893488147419103232") + IntervalMonthDayNano("18446744073709551616") |
+---------------------+---------------------------------------------------------------------------------------------------------------------------------+
| 2024-01-22T23:00:00 | 0 |
| 2024-01-23T23:00:00 | 1 |
| 2024-01-24T23:00:00 | 3 |
+---------------------+---------------------------------------------------------------------------------------------------------------------------------+

-- TODO(Taylor-lagrange): coerce issue in datatype `Duration(Nanosecond)`(made by `now() - now()`) and `Interval(MonthDayNano)`
SELECT ts, host, min(val) RANGE (now() - now() + INTERVAL '1' day) FROM host ALIGN '1d' TO '2023-01-01T00:00:00+01:00' ORDER BY host, ts;

Error: 3000(PlanQuery), Failed to plan SQL: Error during planning: Cannot coerce arithmetic expression Duration(Nanosecond) + Interval(MonthDayNano) to valid types
+---------------------+-----------------------------------------------------------------------------------------------------------------+
| ts | MIN(host.val) RANGE IntervalMonthDayNano("36893488147419103232") - IntervalMonthDayNano("18446744073709551616") |
+---------------------+-----------------------------------------------------------------------------------------------------------------+
| 2024-01-22T23:00:00 | 0 |
| 2024-01-23T23:00:00 | 1 |
| 2024-01-24T23:00:00 | 3 |
+---------------------+-----------------------------------------------------------------------------------------------------------------+

-- non-positive duration
SELECT ts, min(val) RANGE (INTERVAL '1' day - INTERVAL '2' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;
Expand All @@ -106,6 +101,11 @@ SELECT ts, min(val) RANGE (INTERVAL '1' day - INTERVAL '1' day) FROM host ALIGN

Error: 3000(PlanQuery), DataFusion error: Error during planning: Illegal argument `IntervalMonthDayNano("18446744073709551616") - IntervalMonthDayNano("18446744073709551616")` in range select query

-- duration not all interval
SELECT ts, min(val) RANGE (now() - INTERVAL '1' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;

Error: 3000(PlanQuery), DataFusion error: Error during planning: Illegal argument `now() - IntervalMonthDayNano("18446744073709551616")` in range select query

--- ALIGN TO with time zone ---
set time_zone='Asia/Shanghai';

Expand Down
9 changes: 5 additions & 4 deletions tests/cases/standalone/common/range/to.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,18 @@ SELECT ts, host, min(val) RANGE '1d' FROM host ALIGN '1d' TO '2023-01-01T00:00:0

SELECT ts, min(val) RANGE (INTERVAL '1' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;

SELECT ts, min(val) RANGE (now() - (now() - INTERVAL '2' day + INTERVAL '1' day)) FROM host ALIGN (now() - (now() - INTERVAL '2' day + INTERVAL '1' day)) TO (now() - (now() + INTERVAL '1' hour)) by (1) ORDER BY ts;

-- TODO(Taylor-lagrange): coerce issue in datatype `Duration(Nanosecond)`(made by `now() - now()`) and `Interval(MonthDayNano)`
SELECT ts, host, min(val) RANGE (now() - now() + INTERVAL '1' day) FROM host ALIGN '1d' TO '2023-01-01T00:00:00+01:00' ORDER BY host, ts;
SELECT ts, min(val) RANGE (INTERVAL '2' day - INTERVAL '1' day) FROM host ALIGN (INTERVAL '2' day - INTERVAL '1' day) TO (now() - (now() + INTERVAL '1' hour)) by (1) ORDER BY ts;

-- non-positive duration

SELECT ts, min(val) RANGE (INTERVAL '1' day - INTERVAL '2' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;

SELECT ts, min(val) RANGE (INTERVAL '1' day - INTERVAL '1' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;

-- duration not all interval

SELECT ts, min(val) RANGE (now() - INTERVAL '1' day) FROM host ALIGN (INTERVAL '1' day) TO '1900-01-01T00:00:00+01:00' by (1) ORDER BY ts;

--- ALIGN TO with time zone ---
set time_zone='Asia/Shanghai';

Expand Down

0 comments on commit d7e3ec0

Please sign in to comment.