Skip to content

Commit

Permalink
Merge 'Strftime compatibility solved' from Pedro Muniz
Browse files Browse the repository at this point in the history
This PR closes #787. Chrono offers to format the string from an iterator
of Format Items. I created a custom iterator that only allows formatters
specified by sqlite. This approach however does not address the
inefficient way that julianday is calculated. Also, with this
implementation we avoid having to maintain a separate vendored package
for strftime that may become incompatible with Chrono in the future.

Closes #792
  • Loading branch information
penberg committed Jan 30, 2025
2 parents e66648b + a94f2bc commit c779537
Show file tree
Hide file tree
Showing 4 changed files with 242 additions and 3 deletions.
7 changes: 4 additions & 3 deletions core/vdbe/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ fn format_dt(dt: NaiveDateTime, output_type: DateTimeOutput, subsec: bool) -> St
// Not as fast as if the formatting was native to chrono, but a good enough
// for now, just to have the feature implemented
fn strftime_format(dt: &NaiveDateTime, format_str: &str) -> String {
use super::strftime::CustomStrftimeItems;
use std::fmt::Write;
// Necessary to remove %f and %J that are exclusive formatters to sqlite
// Chrono does not support them, so it is necessary to replace the modifiers manually
Expand All @@ -130,13 +131,13 @@ fn strftime_format(dt: &NaiveDateTime, format_str: &str) -> String {
let copy_format = format_str
.to_string()
.replace("%J", &format!("{:.9}", to_julian_day_exact(dt)));
// Just change the formatting here to have fractional seconds using chrono builtin modifier
let copy_format = copy_format.replace("%f", "%S.%3f");

let items = CustomStrftimeItems::new(&copy_format);

// The write! macro is used here as chrono's format can panic if the formatting string contains
// unknown specifiers. By using a writer, we can catch the panic and handle the error
let mut formatted = String::new();
match write!(formatted, "{}", dt.format(&copy_format)) {
match write!(formatted, "{}", dt.format_with_items(items)) {
Ok(_) => formatted,
// On sqlite when the formatting fails nothing is printed
Err(_) => "".to_string(),
Expand Down
1 change: 1 addition & 0 deletions core/vdbe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub mod explain;
pub mod insn;
pub mod likeop;
pub mod sorter;
mod strftime;

use crate::error::{LimboError, SQLITE_CONSTRAINT_PRIMARYKEY};
use crate::ext::ExtValue;
Expand Down
228 changes: 228 additions & 0 deletions core/vdbe/strftime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
//! Code adapted from Chrono StrftimeItems but for sqlite strftime compatibility
//! Sqlite reference https://www.sqlite.org/lang_datefunc.html
use chrono::format::{Fixed, Item, Numeric, Pad};

const fn num(numeric: Numeric) -> Item<'static> {
Item::Numeric(numeric, Pad::None)
}

const fn num0(numeric: Numeric) -> Item<'static> {
Item::Numeric(numeric, Pad::Zero)
}

const fn nums(numeric: Numeric) -> Item<'static> {
Item::Numeric(numeric, Pad::Space)
}

const fn fixed(fixed: Fixed) -> Item<'static> {
Item::Fixed(fixed)
}

#[derive(Clone, Debug)]
pub struct CustomStrftimeItems<'a> {
// Remaining portion of the string.
remainder: &'a str,
/// If the current specifier is composed of multiple formatting items (e.g. `%+`),
/// `queue` stores a slice of `Item`s that have to be returned one by one.
queue: &'static [Item<'static>],
}

impl<'a> CustomStrftimeItems<'a> {
pub const fn new(s: &'a str) -> CustomStrftimeItems<'a> {
CustomStrftimeItems {
remainder: s,
queue: &[],
}
}
}

// const HAVE_ALTERNATES: &str = "z";

impl<'a> Iterator for CustomStrftimeItems<'a> {
type Item = Item<'a>;

fn next(&mut self) -> Option<Item<'a>> {
// We have items queued to return from a specifier composed of multiple formatting items.
if let Some((item, remainder)) = self.queue.split_first() {
self.queue = remainder;
return Some(item.clone());
}

// Normal: we are parsing the formatting string.
let (remainder, item) = self.parse_next_item(self.remainder)?;
self.remainder = remainder;
Some(item)
}
}

impl<'a> CustomStrftimeItems<'a> {
fn parse_next_item(&mut self, mut remainder: &'a str) -> Option<(&'a str, Item<'a>)> {
// use InternalInternal::*;
use Item::{Literal, Space};
use Numeric::*;

match remainder.chars().next() {
// we are done
None => None,

// the next item is a specifier
Some('%') => {
remainder = &remainder[1..];

macro_rules! next {
() => {
match remainder.chars().next() {
Some(x) => {
remainder = &remainder[x.len_utf8()..];
x
}
None => return Some((remainder, Item::Error)), // premature end of string
}
};
}

let spec = next!();
let pad_override = match spec {
'-' => Some(Pad::None),
'0' => Some(Pad::Zero),
'_' => Some(Pad::Space),
_ => None,
};

// let is_alternate = spec == '#';
// let spec = if pad_override.is_some() || is_alternate { next!() } else { spec };
// if is_alternate && !HAVE_ALTERNATES.contains(spec) {
// return Some((remainder, Item::Error));
// }

macro_rules! queue {
[$head:expr, $($tail:expr),+ $(,)*] => ({
const QUEUE: &'static [Item<'static>] = &[$($tail),+];
self.queue = QUEUE;
$head
})
}

// macro_rules! queue_from_slice {
// ($slice:expr) => {{
// self.queue = &$slice[1..];
// $slice[0].clone()
// }};
// }

let item = match spec {
// day of month: 01-31
'd' => num0(Day),
// day of month without leading zero: 1-31
'e' => nums(Day),
// fractional seconds: SS.SSS
'f' => {
queue![num0(Second), fixed(Fixed::Nanosecond3)]
}
// ISO 8601 date: YYYY-MM-DD
'F' => queue![
num0(Year),
Literal("-"),
num0(Month),
Literal("-"),
num0(Day)
],
// ISO 8601 year corresponding to %V
'G' => num0(IsoYear),
// 2-digit ISO 8601 year corresponding to %V
'g' => num0(IsoYearMod100),
// hour: 00-24
'H' => num0(Hour),
// hour for 12-hour clock: 01-12
'I' => num0(Hour12),
// day of year: 001-366
'j' => num0(Ordinal),
// hour without leading zero: 0-24
'k' => nums(Hour),
// %I without leading zero: 1-12
'l' => nums(Hour12),
// month: 01-12
'm' => num0(Month),
// minute: 00-59
'M' => num0(Minute),
// "AM" or "PM" depending on the hour
'p' => fixed(Fixed::UpperAmPm),
// "am" or "pm" depending on the hour
'P' => fixed(Fixed::LowerAmPm),
// ISO 8601 time: HH:MM
'R' => queue![num0(Hour), Literal(":"), num0(Minute)],
// seconds since 1970-01-01
's' => num(Timestamp),
// seconds: 00-59
'S' => num0(Second),
// ISO 8601 time: HH:MM:SS
'T' => {
queue![
num0(Hour),
Literal(":"),
num0(Minute),
Literal(":"),
num0(Second)
]
}
// week of year (00-53) - week 01 starts on the first Sunday
'U' => num0(WeekFromSun),
// day of week 1-7 with Monday==1
'u' => num(WeekdayFromMon),
// ISO 8601 week of year
'V' => num0(IsoWeek),
// day of week 0-6 with Sunday==0
'w' => num(NumDaysFromSun),
// week of year (00-53) - week 01 starts on the first Monday
'W' => num0(WeekFromMon),
// year: 0000-9999
'Y' => num0(Year),
// %
'%' => Literal("%"),
// TODO instead of doing a preprocessing of the %J specifier, it could be done post as postprocessing
// step by just emitting the formatter again to the string
// 'J' => Literal("%J"),
_ => Item::Error, // no such specifier
};

// Adjust `item` if we have any padding modifier.
// Not allowed on non-numeric items or on specifiers composed out of multiple
// formatting items.
if let Some(new_pad) = pad_override {
match item {
Item::Numeric(ref kind, _pad) if self.queue.is_empty() => {
Some((remainder, Item::Numeric(kind.clone(), new_pad)))
}
_ => Some((remainder, Item::Error)),
}
} else {
Some((remainder, item))
}
}

// the next item is space
Some(c) if c.is_whitespace() => {
// `%` is not a whitespace, so `c != '%'` is redundant
let nextspec = remainder
.find(|c: char| !c.is_whitespace())
.unwrap_or(remainder.len());
assert!(nextspec > 0);
let item = Space(&remainder[..nextspec]);
remainder = &remainder[nextspec..];
Some((remainder, item))
}

// the next item is literal
_ => {
let nextspec = remainder
.find(|c: char| c.is_whitespace() || c == '%')
.unwrap_or(remainder.len());
assert!(nextspec > 0);
let item = Literal(&remainder[..nextspec]);
remainder = &remainder[nextspec..];
Some((remainder, item))
}
}
}
}
9 changes: 9 additions & 0 deletions testing/scalar-functions-datetime.test
Original file line number Diff line number Diff line change
Expand Up @@ -580,3 +580,12 @@ do_execsql_test strftime-percent {
SELECT strftime('%%', '2025-01-23T13:14:30.567');
} {%}

# Tests that should return null or empty string
# Will test formatter strings that exist in chrono
# But should not exist in sqlite

set FMT [list %S.%3f %C %y %b %B %h %a %A %D %x %v %.f %.3f %.6f %.9f %3f %6f %9f %X %r %Z %z %:z %::z %:::z %#z %c %+ %t %n %-? %_? %0?]

foreach i $FMT {
do_execsql_test strftime-invalid-$i "SELECT strftime('$i','2025-01-23T13:14:30.567');" {}
}

0 comments on commit c779537

Please sign in to comment.