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;