From 1acd753a6321d9a7a297d286c1ca8937fd8f6225 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 28 Feb 2021 12:43:46 +0100 Subject: [PATCH 1/3] Sync series with official source --- src/vtab/series.rs | 85 ++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index 4ab39dd..c8736c8 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -13,7 +13,7 @@ use crate::vtab::{ eponymous_only_module, Context, IndexConstraintOp, IndexInfo, VTab, VTabConnection, VTabCursor, Values, }; -use crate::{Connection, Result}; +use crate::{Connection, Error, Result}; /// `feature = "series"` Register the "generate_series" module. pub fn load_module(conn: &Connection) -> Result<()> { @@ -38,6 +38,8 @@ bitflags::bitflags! { const STEP = 4; // output in descending order const DESC = 8; + // output in descending order + const ASC = 16; // Both start and stop const BOTH = QueryPlanFlags::START.bits | QueryPlanFlags::STOP.bits; } @@ -71,54 +73,39 @@ unsafe impl<'vtab> VTab<'vtab> for SeriesTab { fn best_index(&self, info: &mut IndexInfo) -> Result<()> { // The query plan bitmask let mut idx_num: QueryPlanFlags = QueryPlanFlags::empty(); - // Index of the start= constraint - let mut start_idx = None; - // Index of the stop= constraint - let mut stop_idx = None; - // Index of the step= constraint - let mut step_idx = None; + // Mask of unusable constraints + let mut unusable_mask: QueryPlanFlags = QueryPlanFlags::empty(); + // Constraints on start, stop, and step + let mut a_idx: [Option; 3] = [None, None, None]; for (i, constraint) in info.constraints().enumerate() { - if !constraint.is_usable() { + if constraint.column() < SERIES_COLUMN_START { continue; } - if constraint.operator() != IndexConstraintOp::SQLITE_INDEX_CONSTRAINT_EQ { - continue; - } - match constraint.column() { - SERIES_COLUMN_START => { - start_idx = Some(i); - idx_num |= QueryPlanFlags::START; + let (i_col, i_mask) = match constraint.column() { + SERIES_COLUMN_START => (0, QueryPlanFlags::START), + SERIES_COLUMN_STOP => (1, QueryPlanFlags::STOP), + SERIES_COLUMN_STEP => (2, QueryPlanFlags::STEP), + _ => { + unreachable!() } - SERIES_COLUMN_STOP => { - stop_idx = Some(i); - idx_num |= QueryPlanFlags::STOP; - } - SERIES_COLUMN_STEP => { - step_idx = Some(i); - idx_num |= QueryPlanFlags::STEP; - } - _ => {} }; + if !constraint.is_usable() { + unusable_mask |= i_mask; + } else if constraint.operator() == IndexConstraintOp::SQLITE_INDEX_CONSTRAINT_EQ { + idx_num |= i_mask; + a_idx[i_col] = Some(i); + } } - - let mut num_of_arg = 0; - if let Some(start_idx) = start_idx { - num_of_arg += 1; - let mut constraint_usage = info.constraint_usage(start_idx); - constraint_usage.set_argv_index(num_of_arg); + // Number of arguments that SeriesTabCursor::filter expects + let mut n_arg = 0; + for j in a_idx.iter().flatten() { + n_arg += 1; + let mut constraint_usage = info.constraint_usage(*j); + constraint_usage.set_argv_index(n_arg); constraint_usage.set_omit(true); } - if let Some(stop_idx) = stop_idx { - num_of_arg += 1; - let mut constraint_usage = info.constraint_usage(stop_idx); - constraint_usage.set_argv_index(num_of_arg); - constraint_usage.set_omit(true); - } - if let Some(step_idx) = step_idx { - num_of_arg += 1; - let mut constraint_usage = info.constraint_usage(step_idx); - constraint_usage.set_argv_index(num_of_arg); - constraint_usage.set_omit(true); + if !(unusable_mask & !idx_num).is_empty() { + return Err(Error::SqliteFailure(ffi::Error::new(ffi::SQLITE_CONSTRAINT), None)); } if idx_num.contains(QueryPlanFlags::BOTH) { // Both start= and stop= boundaries are available. @@ -135,6 +122,8 @@ unsafe impl<'vtab> VTab<'vtab> for SeriesTab { if let Some(order_by) = order_bys.next() { if order_by.is_order_by_desc() { idx_num |= QueryPlanFlags::DESC; + } else { + idx_num |= QueryPlanFlags::ASC; } true } else { @@ -145,7 +134,9 @@ unsafe impl<'vtab> VTab<'vtab> for SeriesTab { info.set_order_by_consumed(true); } } else { - info.set_estimated_cost(2_147_483_647f64); + // If either boundary is missing, we have to generate a huge span + // of numbers. Make this case very expensive so that the query + // planner will work hard to avoid it. info.set_estimated_rows(2_147_483_647); } info.set_idx_num(idx_num.bits()); @@ -193,7 +184,7 @@ impl SeriesTabCursor<'_> { } unsafe impl VTabCursor for SeriesTabCursor<'_> { fn filter(&mut self, idx_num: c_int, _idx_str: Option<&str>, args: &Values<'_>) -> Result<()> { - let idx_num = QueryPlanFlags::from_bits_truncate(idx_num); + let mut idx_num = QueryPlanFlags::from_bits_truncate(idx_num); let mut i = 0; if idx_num.contains(QueryPlanFlags::START) { self.min_value = args.get(i)?; @@ -209,8 +200,14 @@ unsafe impl VTabCursor for SeriesTabCursor<'_> { } if idx_num.contains(QueryPlanFlags::STEP) { self.step = args.get(i)?; - if self.step < 1 { + #[allow(clippy::comparison_chain)] + if self.step == 0 { self.step = 1; + } else if self.step < 0 { + self.step = -self.step; + if !idx_num.contains(QueryPlanFlags::ASC) { + idx_num |= QueryPlanFlags::DESC; + } } } else { self.step = 1; From 5e79126a60940bc4c280e2a3ce224b0c6da260a9 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 28 Feb 2021 12:58:59 +0100 Subject: [PATCH 2/3] Rustfmt --- src/vtab/series.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index c8736c8..72dce9f 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -105,7 +105,10 @@ unsafe impl<'vtab> VTab<'vtab> for SeriesTab { constraint_usage.set_omit(true); } if !(unusable_mask & !idx_num).is_empty() { - return Err(Error::SqliteFailure(ffi::Error::new(ffi::SQLITE_CONSTRAINT), None)); + return Err(Error::SqliteFailure( + ffi::Error::new(ffi::SQLITE_CONSTRAINT), + None, + )); } if idx_num.contains(QueryPlanFlags::BOTH) { // Both start= and stop= boundaries are available. From c9cc63908017c5aea0b18ab4e1c204c7eeac7434 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 6 Mar 2021 12:55:25 +0100 Subject: [PATCH 3/3] Add tests adapted from official SQLite tests --- src/vtab/series.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index 72dce9f..80693cf 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -274,6 +274,7 @@ mod test { use crate::ffi; use crate::vtab::series; use crate::{Connection, Result}; + use fallible_iterator::FallibleIterator; #[test] fn test_series_module() -> Result<()> { @@ -294,6 +295,18 @@ mod test { assert_eq!(expected, value?); expected += 5; } + + let mut s = + db.prepare("SELECT * FROM generate_series WHERE start=1 AND stop=9 AND step=2")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(vec![1, 3, 5, 7, 9], series); + let mut s = db.prepare("SELECT * FROM generate_series LIMIT 5")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(vec![0, 1, 2, 3, 4], series); + let mut s = db.prepare("SELECT * FROM generate_series(0,32,5) ORDER BY value DESC")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(vec![30, 25, 20, 15, 10, 5, 0], series); + Ok(()) } }