From 84362c02c650a24171440a42facab27e4f59791a Mon Sep 17 00:00:00 2001 From: David Selassie Date: Thu, 6 Jul 2023 16:51:57 -0700 Subject: [PATCH 1/5] Tests that NULL parameters to generate_series return no rows --- src/vtab/series.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index 4b1993e..53d13b9 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -316,6 +316,16 @@ mod test { let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; assert_eq!(vec![30, 25, 20, 15, 10, 5, 0], series); + let mut s = db.prepare("SELECT * FROM generate_series(NULL)")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(Vec::::new(), series); + let mut s = db.prepare("SELECT * FROM generate_series(5,NULL)")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(Vec::::new(), series); + let mut s = db.prepare("SELECT * FROM generate_series(5,10,NULL)")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(Vec::::new(), series); + Ok(()) } } From 3a2312e0bd3b1a57bd5ab8c3d57700fbba7ce16b Mon Sep 17 00:00:00 2001 From: David Selassie Date: Thu, 6 Jul 2023 17:23:28 -0700 Subject: [PATCH 2/5] Interpret generate_series arguments as possibly NULL --- src/vtab/series.rs | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index 53d13b9..1e178de 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -8,7 +8,6 @@ use std::marker::PhantomData; use std::os::raw::c_int; use crate::ffi; -use crate::types::Type; use crate::vtab::{ eponymous_only_module, Context, IndexConstraintOp, IndexInfo, VTab, VTabConfig, VTabConnection, VTabCursor, Values, @@ -198,21 +197,34 @@ impl SeriesTabCursor<'_> { unsafe impl VTabCursor for SeriesTabCursor<'_> { fn filter(&mut self, idx_num: c_int, _idx_str: Option<&str>, args: &Values<'_>) -> Result<()> { let mut idx_num = QueryPlanFlags::from_bits_truncate(idx_num); + let mut any_null = false; let mut i = 0; if idx_num.contains(QueryPlanFlags::START) { - self.min_value = args.get(i)?; + if let Some(min_value) = args.get(i)? { + self.min_value = min_value; + } else { + any_null = true; + } i += 1; } else { self.min_value = 0; } if idx_num.contains(QueryPlanFlags::STOP) { - self.max_value = args.get(i)?; + if let Some(max_value) = args.get(i)? { + self.max_value = max_value; + } else { + any_null = true; + } i += 1; } else { self.max_value = 0xffff_ffff; } if idx_num.contains(QueryPlanFlags::STEP) { - self.step = args.get(i)?; + if let Some(step) = args.get(i)? { + self.step = step; + } else { + any_null = true; + } if self.step == 0 { self.step = 1; } else if self.step < 0 { @@ -224,13 +236,11 @@ unsafe impl VTabCursor for SeriesTabCursor<'_> { } else { self.step = 1; }; - for arg in args.iter() { - if arg.data_type() == Type::Null { - // If any of the constraints have a NULL value, then return no rows. - self.min_value = 1; - self.max_value = 0; - break; - } + if any_null { + // If any of the constraints have a NULL value, then + // return no rows. + self.min_value = 1; + self.max_value = 0; } self.is_desc = idx_num.contains(QueryPlanFlags::DESC); if self.is_desc { From 3c5a9be349612709539d4af9c31a2af4158756b4 Mon Sep 17 00:00:00 2001 From: David Selassie Date: Fri, 7 Jul 2023 09:17:30 -0700 Subject: [PATCH 3/5] Adds a few more NULL generate_series tests --- src/vtab/series.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index 1e178de..a8a13a3 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -328,13 +328,23 @@ mod test { let mut s = db.prepare("SELECT * FROM generate_series(NULL)")?; let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; - assert_eq!(Vec::::new(), series); + let empty = Vec::::new(); + assert_eq!(empty, series); let mut s = db.prepare("SELECT * FROM generate_series(5,NULL)")?; let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; - assert_eq!(Vec::::new(), series); + assert_eq!(empty, series); let mut s = db.prepare("SELECT * FROM generate_series(5,10,NULL)")?; let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; - assert_eq!(Vec::::new(), series); + assert_eq!(empty, series); + let mut s = db.prepare("SELECT * FROM generate_series(NULL,10,2)")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(empty, series); + let mut s = db.prepare("SELECT * FROM generate_series(5,NULL,2)")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(empty, series); + let mut s = db.prepare("SELECT * FROM generate_series(NULL) ORDER BY value DESC")?; + let series: Vec = s.query([])?.map(|r| r.get(0)).collect()?; + assert_eq!(empty, series); Ok(()) } From 379c6c8dcf536ba0f98ccf025a081428966dc175 Mon Sep 17 00:00:00 2001 From: David Selassie Date: Sat, 8 Jul 2023 09:30:23 -0700 Subject: [PATCH 4/5] Revert "Interpret generate_series arguments as possibly NULL" This reverts commit 3a2312e0bd3b1a57bd5ab8c3d57700fbba7ce16b. --- src/vtab/series.rs | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index a8a13a3..c42fd4e 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -8,6 +8,7 @@ use std::marker::PhantomData; use std::os::raw::c_int; use crate::ffi; +use crate::types::Type; use crate::vtab::{ eponymous_only_module, Context, IndexConstraintOp, IndexInfo, VTab, VTabConfig, VTabConnection, VTabCursor, Values, @@ -197,34 +198,21 @@ impl SeriesTabCursor<'_> { unsafe impl VTabCursor for SeriesTabCursor<'_> { fn filter(&mut self, idx_num: c_int, _idx_str: Option<&str>, args: &Values<'_>) -> Result<()> { let mut idx_num = QueryPlanFlags::from_bits_truncate(idx_num); - let mut any_null = false; let mut i = 0; if idx_num.contains(QueryPlanFlags::START) { - if let Some(min_value) = args.get(i)? { - self.min_value = min_value; - } else { - any_null = true; - } + self.min_value = args.get(i)?; i += 1; } else { self.min_value = 0; } if idx_num.contains(QueryPlanFlags::STOP) { - if let Some(max_value) = args.get(i)? { - self.max_value = max_value; - } else { - any_null = true; - } + self.max_value = args.get(i)?; i += 1; } else { self.max_value = 0xffff_ffff; } if idx_num.contains(QueryPlanFlags::STEP) { - if let Some(step) = args.get(i)? { - self.step = step; - } else { - any_null = true; - } + self.step = args.get(i)?; if self.step == 0 { self.step = 1; } else if self.step < 0 { @@ -236,11 +224,13 @@ unsafe impl VTabCursor for SeriesTabCursor<'_> { } else { self.step = 1; }; - if any_null { - // If any of the constraints have a NULL value, then - // return no rows. - self.min_value = 1; - self.max_value = 0; + for arg in args.iter() { + if arg.data_type() == Type::Null { + // If any of the constraints have a NULL value, then return no rows. + self.min_value = 1; + self.max_value = 0; + break; + } } self.is_desc = idx_num.contains(QueryPlanFlags::DESC); if self.is_desc { From 256cfdd31102b063214b3521628358cee0a1c812 Mon Sep 17 00:00:00 2001 From: David Selassie Date: Sat, 8 Jul 2023 09:36:00 -0700 Subject: [PATCH 5/5] Handles NULL generate_series params via defaults --- src/vtab/series.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vtab/series.rs b/src/vtab/series.rs index c42fd4e..5b67758 100644 --- a/src/vtab/series.rs +++ b/src/vtab/series.rs @@ -200,19 +200,19 @@ unsafe impl VTabCursor for SeriesTabCursor<'_> { 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)?; + self.min_value = args.get::>(i)?.unwrap_or_default(); i += 1; } else { self.min_value = 0; } if idx_num.contains(QueryPlanFlags::STOP) { - self.max_value = args.get(i)?; + self.max_value = args.get::>(i)?.unwrap_or_default(); i += 1; } else { self.max_value = 0xffff_ffff; } if idx_num.contains(QueryPlanFlags::STEP) { - self.step = args.get(i)?; + self.step = args.get::>(i)?.unwrap_or_default(); if self.step == 0 { self.step = 1; } else if self.step < 0 {