core: support modifiers in time function

COMPAT.md

@@ -89,7 +89,7 @@ This document describes the SQLite compatibility status of Limbo:
 | nullif(X,Y)                  | Yes    |         |
 | octet_length(X)              | No     |         |
 | printf(FORMAT,...)           | No     |         |
-| quote(X)                     | Yes     |         |
+| quote(X)                     | Yes    |         |
 | random()                     | Yes    |         |
 | randomblob(N)                | No     |         |
 | replace(X,Y,Z)               | No     |         |
@@ -137,15 +137,15 @@ This document describes the SQLite compatibility status of Limbo:
 
 ### Date and time functions
 
-| Function                     | Status  | Comment                |
-|------------------------------|---------|------------------------|
-| date()                       | Partial |                        |
-| time()                       | Partial | not supported modifier |
-| datetime()                   | No      |                        |
-| julianday()                  | No      |                        |
-| unixepoch()                  | No      |                        |
-| strftime()                   | No      |                        |
-| timediff()                   | No      |                        |
+| Function                     | Status  | Comment                      |
+|------------------------------|---------|------------------------------|
+| date()                       | Partial |                              |
+| time()                       | Partial | partially supports modifiers |
+| datetime()                   | No      |                              |
+| julianday()                  | No      |                              |
+| unixepoch()                  | No      |                              |
+| strftime()                   | No      |                              |
+| timediff()                   | No      |                              |
 
 ### JSON functions
 

core/translate/expr.rs

@@ -945,24 +945,21 @@ pub fn translate_expr(
                             Ok(target_register)
                         }
                         ScalarFunc::Time => {
-                            let mut start_reg = 0;
                             if let Some(args) = args {
-                                if args.len() > 1 {
-                                    crate::bail_parse_error!("date function with > 1 arguments. Modifiers are not yet supported.");
-                                } else if args.len() == 1 {
-                                    let arg_reg = program.alloc_register();
-                                    let _ = translate_expr(
+                                for arg in args.iter() {
+                                    // register containing result of each argument expression
+                                    let target_reg = program.alloc_register();
+                                    _ = translate_expr(
                                         program,
                                         referenced_tables,
-                                        &args[0],
-                                        arg_reg,
+                                        arg,
+                                        target_reg,
                                         cursor_hint,
                                     )?;
-                                    start_reg = arg_reg;
                                 }
                             }
                             program.emit_insn(Insn::Function {
-                                start_reg: start_reg,
+                                start_reg: target_register + 1,
                                 dest: target_register,
                                 func: crate::vdbe::Func::Scalar(ScalarFunc::Time),
                             });

core/vdbe/datetime.rs

@@ -1,6 +1,7 @@
 use chrono::{
     DateTime, Datelike, NaiveDate, NaiveDateTime, NaiveTime, TimeDelta, TimeZone, Timelike, Utc,
 };
+use std::rc::Rc;
 
 use crate::types::OwnedValue;
 use crate::LimboError::InvalidModifier;
@@ -16,12 +17,29 @@ pub fn exec_date(time_value: &OwnedValue) -> Result<String> {
 }
 
 /// Implementation of the time() SQL function.
-pub fn exec_time(time_value: &OwnedValue) -> Result<String> {
-    let dt = parse_naive_date_time(time_value);
-    match dt {
-        Some(dt) => Ok(get_time_from_naive_datetime(dt)),
-        None => Ok(String::new()),
+pub fn exec_time(time_value: &[OwnedValue]) -> OwnedValue {
+    let maybe_dt = match time_value.first() {
+        None => parse_naive_date_time(&OwnedValue::Text(Rc::new("now".to_string()))),
+        Some(value) => parse_naive_date_time(value),
+    };
+    // early return, no need to look at modifiers if result invalid
+    if maybe_dt.is_none() {
+        return OwnedValue::Text(Rc::new(String::new()));
     }
+
+    // apply modifiers if result is valid
+    let mut dt = maybe_dt.unwrap();
+    for modifier in time_value.iter().skip(1) {
+        if let OwnedValue::Text(modifier_str) = modifier {
+            if apply_modifier(&mut dt, modifier_str).is_err() {
+                return OwnedValue::Text(Rc::new(String::new()));
+            }
+        } else {
+            return OwnedValue::Text(Rc::new(String::new()));
+        }
+    }
+
+    OwnedValue::Text(Rc::new(get_time_from_naive_datetime(dt)))
 }
 
 fn apply_modifier(dt: &mut NaiveDateTime, modifier: &str) -> Result<()> {
@@ -630,7 +648,7 @@ mod tests {
         let prev_time_str = "20:30:45";
         let next_time_str = "03:30:45";
 
-        let test_cases = [
+        let test_cases = vec![
             // Format 1: YYYY-MM-DD (no timezone applicable)
             (
                 OwnedValue::Text(Rc::new("2024-07-21".to_string())),
@@ -791,18 +809,18 @@ mod tests {
         ];
 
         for (input, expected) in test_cases {
-            assert_eq!(
-                exec_time(&input).unwrap(),
-                expected,
-                "Failed for input: {:?}",
-                input
-            );
+            let result = exec_time(&[input]);
+            if let OwnedValue::Text(result_str) = result {
+                assert_eq!(result_str.as_str(), expected);
+            } else {
+                panic!("Expected OwnedValue::Text, but got: {:?}", result);
+            }
         }
     }
 
     #[test]
     fn test_invalid_get_time_from_datetime_value() {
-        let invalid_cases = [
+        let invalid_cases = vec![
             OwnedValue::Text(Rc::new("2024-07-21 25:00".to_string())), // Invalid hour
             OwnedValue::Text(Rc::new("2024-07-21 24:00:00".to_string())), // Invalid hour
             OwnedValue::Text(Rc::new("2024-07-21 23:60:00".to_string())), // Invalid minute
@@ -830,21 +848,15 @@ mod tests {
             OwnedValue::Text(Rc::new("2024-07-21T12:00:00UTC".to_string())), // Named timezone (not supported)
         ];
 
-        for case in invalid_cases.iter() {
-            let result = exec_time(case);
-            assert!(
-                result.is_ok(),
-                "Error encountered while parsing time value {}: {}",
-                case,
-                result.unwrap_err()
-            );
-            let result_str = result.unwrap();
-            assert!(
-                result_str.is_empty(),
-                "Expected empty string for input: {:?}, but got: {:?}",
-                case,
-                result_str
-            );
+        for case in invalid_cases {
+            let result = exec_time(&[case.clone()]);
+            match result {
+                OwnedValue::Text(ref result_str) if result_str.is_empty() => (),
+                _ => panic!(
+                    "Expected empty string for input: {:?}, but got: {:?}",
+                    case, result
+                ),
+            }
         }
     }
 

core/vdbe/mod.rs

@@ -1361,25 +1361,8 @@ impl Program {
                         state.pc += 1;
                     }
                     Func::Scalar(ScalarFunc::Time) => {
-                        if *start_reg == 0 {
-                            let time_str =
-                                exec_time(&OwnedValue::Text(Rc::new("now".to_string())))?;
-                            state.registers[*dest] = OwnedValue::Text(Rc::new(time_str));
-                        } else {
-                            let datetime_value = &state.registers[*start_reg];
-                            let time_str = exec_time(datetime_value);
-                            match time_str {
-                                Ok(time) => {
-                                    state.registers[*dest] = OwnedValue::Text(Rc::new(time))
-                                }
-                                Err(e) => {
-                                    return Err(LimboError::ParseError(format!(
-                                        "Error encountered while parsing time value: {}",
-                                        e
-                                    )));
-                                }
-                            }
-                        }
+                        let result = exec_time(&state.registers[*start_reg..]);
+                        state.registers[*dest] = result;
                         state.pc += 1;
                     }
                     Func::Scalar(ScalarFunc::Unicode) => {

testing/all.test

@@ -13,5 +13,6 @@ source $testdir/like.test
 source $testdir/orderby.test
 source $testdir/pragma.test
 source $testdir/scalar-functions.test
+source $testdir/scalar-functions-datetime.test
 source $testdir/select.test
 source $testdir/where.test

testing/scalar-functions-datetime.test

@@ -0,0 +1,125 @@
+#!/usr/bin/env tclsh
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+
+do_execsql_test time-no-arg {
+  SELECT length(time()) = 8;
+} {1}
+
+do_execsql_test time-current-time {
+  SELECT length(time('now')) = 8;
+} {1}
+
+do_execsql_test time-specific-time {
+  SELECT time('04:02:00');
+} {04:02:00}
+
+do_execsql_test time-of-datetime {
+  SELECT time('2023-05-18 15:30:45');
+} {15:30:45}
+
+do_execsql_test time-iso8601 {
+  SELECT time('2023-05-18T15:30:45');
+} {15:30:45}
+
+do_execsql_test time-with-milliseconds {
+  SELECT time('2023-05-18 15:30:45.123');
+} {15:30:45}
+
+do_execsql_test time-julian-day-integer {
+  SELECT time(2460082);
+} {12:00:00}
+
+do_execsql_test time-julian-day-float {
+  SELECT time(2460082.2);
+} {16:48:00}
+
+do_execsql_test time-invalid-input {
+  SELECT time('not a time');
+} {{}}
+
+do_execsql_test time-null-input {
+  SELECT time(NULL);
+} {{}}
+
+do_execsql_test time-out-of-range {
+  SELECT time('25:05:01');
+} {{}}
+
+do_execsql_test time-date-only {
+  SELECT time('2024-02-02');
+} {00:00:00}
+
+do_execsql_test time-with-timezone-utc {
+  SELECT time('2023-05-18 15:30:45Z');
+} {15:30:45}
+
+do_execsql_test time-with-timezone-positive {
+  SELECT time('2023-05-18 23:30:45+07:00');
+} {16:30:45}
+
+do_execsql_test time-with-timezone-negative {
+  SELECT time('2023-05-19 01:30:45-05:00');
+} {06:30:45}
+
+do_execsql_test time-with-timezone-day-change-positive {
+  SELECT time('2023-05-18 23:30:45-03:00');
+} {02:30:45}
+
+do_execsql_test time-with-timezone-day-change-negative {
+  SELECT time('2023-05-19 01:30:45+03:00');
+} {22:30:45}
+
+do_execsql_test time-with-timezone-iso8601 {
+  SELECT time('2023-05-18T15:30:45+02:00');
+} {13:30:45}
+
+do_execsql_test time-with-timezone-and-milliseconds {
+  SELECT time('2023-05-18 15:30:45.123+02:00');
+} {13:30:45}
+
+do_execsql_test time-with-invalid-timezone {
+  SELECT time('2023-05-18 15:30:45+25:00');
+} {{}}
+
+do_execsql_test time-with-modifier-start-of-day {
+  SELECT time('2023-05-18 15:30:45', 'start of day');
+} {00:00:00}
+
+do_execsql_test time-with-modifier-add-hours {
+  SELECT time('2023-05-18 15:30:45', '+2 hours');
+} {17:30:45}
+
+do_execsql_test time-with-modifier-add-minutes {
+  SELECT time('2023-05-18 15:30:45', '+45 minutes');
+} {16:15:45}
+
+do_execsql_test time-with-modifier-add-seconds {
+  SELECT time('2023-05-18 15:30:45', '+30 seconds');
+} {15:31:15}
+
+do_execsql_test time-with-modifier-subtract-hours {
+  SELECT time('2023-05-18 15:30:45', '-3 hours');
+} {12:30:45}
+
+do_execsql_test time-with-modifier-subtract-minutes {
+  SELECT time('2023-05-18 15:30:45', '-15 minutes');
+} {15:15:45}
+
+do_execsql_test time-with-modifier-subtract-seconds {
+  SELECT time('2023-05-18 15:30:45', '-45 seconds');
+} {15:30:00}
+
+do_execsql_test time-with-multiple-modifiers {
+  SELECT time('2023-05-18 15:30:45', '+1 hours', '-30 minutes', '+15 seconds');
+} {16:01:00}
+
+do_execsql_test time-with-invalid-modifier {
+  SELECT time('2023-05-18 15:30:45', 'invalid modifier');
+} {{}}
+
+do_execsql_test time-with-invalid-modifier {
+  SELECT time('2023-05-18 15:30:45', '+1 hour', 'invalid modifier');
+} {{}}
+

testing/scalar-functions.test

@@ -403,86 +403,6 @@ do_execsql_test date-with-invalid-timezone {
   SELECT date('2023-05-18 15:30:45+25:00');
 } {{}}
 
-do_execsql_test time-no-arg {
-  SELECT length(time()) = 8;
-} {1}
-
-do_execsql_test time-current-time {
-  SELECT length(time('now')) = 8;
-} {1}
-
-do_execsql_test time-specific-time {
-  SELECT time('04:02:00');
-} {04:02:00}
-
-do_execsql_test time-of-datetime {
-  SELECT time('2023-05-18 15:30:45');
-} {15:30:45}
-
-do_execsql_test time-iso8601 {
-  SELECT time('2023-05-18T15:30:45');
-} {15:30:45}
-
-do_execsql_test time-with-milliseconds {
-  SELECT time('2023-05-18 15:30:45.123');
-} {15:30:45}
-
-do_execsql_test time-julian-day-integer {
-  SELECT time(2460082);
-} {12:00:00}
-
-do_execsql_test time-julian-day-float {
-  SELECT time(2460082.2);
-} {16:48:00}
-
-do_execsql_test time-invalid-input {
-  SELECT time('not a time');
-} {{}}
-
-do_execsql_test time-null-input {
-  SELECT time(NULL);
-} {{}}
-
-do_execsql_test time-out-of-range {
-  SELECT time('25:05:01');
-} {{}}
-
-do_execsql_test time-date-only {
-  SELECT time('2024-02-02');
-} {00:00:00}
-
-do_execsql_test time-with-timezone-utc {
-  SELECT time('2023-05-18 15:30:45Z');
-} {15:30:45}
-
-do_execsql_test time-with-timezone-positive {
-  SELECT time('2023-05-18 23:30:45+07:00');
-} {16:30:45}
-
-do_execsql_test time-with-timezone-negative {
-  SELECT time('2023-05-19 01:30:45-05:00');
-} {06:30:45}
-
-do_execsql_test time-with-timezone-day-change-positive {
-  SELECT time('2023-05-18 23:30:45-03:00');
-} {02:30:45}
-
-do_execsql_test time-with-timezone-day-change-negative {
-  SELECT time('2023-05-19 01:30:45+03:00');
-} {22:30:45}
-
-do_execsql_test time-with-timezone-iso8601 {
-  SELECT time('2023-05-18T15:30:45+02:00');
-} {13:30:45}
-
-do_execsql_test time-with-timezone-and-milliseconds {
-  SELECT time('2023-05-18 15:30:45.123+02:00');
-} {13:30:45}
-
-do_execsql_test time-with-invalid-timezone {
-  SELECT time('2023-05-18 15:30:45+25:00');
-} {{}}
-
 do_execsql_test unicode-a {
   SELECT unicode('a');
 } {97}