Add quickcheck tests for generate_series() and refine implementation

This commit is contained in:
Jussi Saurio 2025-02-06 18:36:21 +02:00
parent 098da0794f
commit d5f58f5fea
9 changed files with 684 additions and 20 deletions

14
Cargo.lock generated
View file

@ -738,6 +738,16 @@ dependencies = [
"log",
]
[[package]]
name = "env_logger"
version = "0.8.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3"
dependencies = [
"log",
"regex",
]
[[package]]
name = "env_logger"
version = "0.10.2"
@ -1709,6 +1719,8 @@ dependencies = [
"limbo_ext",
"log",
"mimalloc",
"quickcheck",
"quickcheck_macros",
]
[[package]]
@ -2387,6 +2399,8 @@ version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6"
dependencies = [
"env_logger 0.8.4",
"log",
"rand 0.8.5",
]

View file

@ -530,7 +530,7 @@ impl VirtualTable {
cursor: &VTabOpaqueCursor,
arg_count: usize,
args: Vec<OwnedValue>,
) -> Result<()> {
) -> Result<bool> {
let mut filter_args = Vec::with_capacity(arg_count);
for i in 0..arg_count {
let ownedvalue_arg = args.get(i).unwrap();
@ -551,7 +551,8 @@ impl VirtualTable {
(self.implementation.filter)(cursor.as_ptr(), arg_count as i32, filter_args.as_ptr())
};
match rc {
ResultCode::OK => Ok(()),
ResultCode::OK => Ok(true),
ResultCode::EOF => Ok(false),
_ => Err(LimboError::ExtensionError(rc.to_string())),
}
}

View file

@ -300,6 +300,7 @@ pub fn open_loop(
}
program.emit_insn(Insn::VFilter {
cursor_id,
pc_if_empty: loop_end,
arg_count: args.len(),
args_reg: start_reg,
});

View file

@ -410,7 +410,10 @@ impl ProgramBuilder {
Insn::VNext { pc_if_next, .. } => {
resolve(pc_if_next, "VNext");
}
_ => continue,
Insn::VFilter { pc_if_empty, .. } => {
resolve(pc_if_empty, "VFilter");
}
_ => {}
}
}
self.label_to_resolved_offset.clear();

View file

@ -383,13 +383,14 @@ pub fn insn_to_str(
),
Insn::VFilter {
cursor_id,
pc_if_empty,
arg_count,
args_reg,
..
} => (
"VFilter",
*cursor_id as i32,
pc_if_empty.to_debug_int(),
*arg_count as i32,
*args_reg as i32,
OwnedValue::build_text(Rc::new("".to_string())),
0,
"".to_string(),

View file

@ -224,6 +224,7 @@ pub enum Insn {
/// Initialize the position of the virtual table cursor.
VFilter {
cursor_id: CursorID,
pc_if_empty: BranchOffset,
arg_count: usize,
args_reg: usize,
},

View file

@ -879,6 +879,7 @@ impl Program {
}
Insn::VFilter {
cursor_id,
pc_if_empty,
arg_count,
args_reg,
} => {
@ -892,8 +893,12 @@ impl Program {
for i in 0..*arg_count {
args.push(state.registers[args_reg + i].clone());
}
virtual_table.filter(cursor, *arg_count, args)?;
state.pc += 1;
let has_rows = virtual_table.filter(cursor, *arg_count, args)?;
if !has_rows {
state.pc = pc_if_empty.to_offset_int();
} else {
state.pc += 1;
}
}
Insn::VColumn {
cursor_id,

View file

@ -19,3 +19,7 @@ log = "0.4.20"
[target.'cfg(not(target_family = "wasm"))'.dependencies]
mimalloc = { version = "*", default-features = false }
[dev-dependencies]
quickcheck = "1.0.3"
quickcheck_macros = "1.0.0"

View file

@ -51,16 +51,30 @@ impl VTabModule for GenerateSeriesVTab {
let start = try_option!(args[0].to_integer(), ResultCode::InvalidArgs);
let stop = try_option!(
args.get(1).map(|v| v.to_integer().unwrap_or(i64::MAX)),
ResultCode::InvalidArgs
);
let step = try_option!(
args.get(2).map(|v| v.to_integer().unwrap_or(1)),
ResultCode::InvalidArgs
ResultCode::EOF // Sqlite returns an empty series for wacky args
);
let mut step = args
.get(2)
.map(|v| v.to_integer().unwrap_or(1))
.unwrap_or(1);
// Convert zero step to 1, matching SQLite behavior
if step == 0 {
step = 1;
}
cursor.start = start;
cursor.current = start;
cursor.step = step;
cursor.stop = stop;
// Set initial value based on range validity
// For invalid input SQLite returns an empty series
cursor.current = if cursor.is_invalid_range() {
return ResultCode::EOF;
} else {
start
};
ResultCode::OK
}
@ -69,11 +83,33 @@ impl VTabModule for GenerateSeriesVTab {
}
fn next(cursor: &mut Self::VCursor) -> ResultCode {
GenerateSeriesCursor::next(cursor)
// Check for invalid ranges (empty series) first
if cursor.is_invalid_range() {
return ResultCode::EOF;
}
// Check if we've reached the end of the sequence
if cursor.would_exceed() {
return ResultCode::EOF;
}
// Handle overflow by truncating to MAX/MIN
cursor.current = match cursor.step {
step if step > 0 => match cursor.current.checked_add(step) {
Some(val) => val,
None => i64::MAX,
},
step => match cursor.current.checked_add(step) {
Some(val) => val,
None => i64::MIN,
},
};
ResultCode::OK
}
fn eof(cursor: &Self::VCursor) -> bool {
cursor.eof()
cursor.is_invalid_range() || cursor.would_exceed()
}
}
@ -86,20 +122,56 @@ struct GenerateSeriesCursor {
current: i64,
}
impl GenerateSeriesCursor {
/// Returns true if this is an ascending series (positive step) but start > stop
fn is_invalid_ascending_series(&self) -> bool {
self.step > 0 && self.start > self.stop
}
/// Returns true if this is a descending series (negative step) but start < stop
fn is_invalid_descending_series(&self) -> bool {
self.step < 0 && self.start < self.stop
}
/// Returns true if this is an invalid range that should produce an empty series
fn is_invalid_range(&self) -> bool {
self.is_invalid_ascending_series() || self.is_invalid_descending_series()
}
/// Returns true if we would exceed the stop value in the current direction
fn would_exceed(&self) -> bool {
(self.step > 0 && self.current + self.step > self.stop)
|| (self.step < 0 && self.current + self.step < self.stop)
}
}
impl VTabCursor for GenerateSeriesCursor {
type Error = ResultCode;
fn next(&mut self) -> ResultCode {
let next_val = self.current.saturating_add(self.step);
if (self.step > 0 && next_val > self.stop) || (self.step < 0 && next_val < self.stop) {
// Check for invalid ranges (empty series) first
if self.eof() {
return ResultCode::EOF;
}
self.current = next_val;
// Handle overflow by truncating to MAX/MIN
self.current = self.current.saturating_add(self.step);
ResultCode::OK
}
fn eof(&self) -> bool {
(self.step > 0 && self.current > self.stop) || (self.step < 0 && self.current < self.stop)
// Check for invalid ranges (empty series) first
if self.is_invalid_range() {
return true;
}
// Check if we would exceed the stop value in the current direction
if self.would_exceed() {
return true;
}
false
}
fn column(&self, idx: u32) -> Value {
@ -113,6 +185,568 @@ impl VTabCursor for GenerateSeriesCursor {
}
fn rowid(&self) -> i64 {
((self.current - self.start) / self.step) + 1
let sub = self.current.saturating_sub(self.start);
// Handle overflow in rowid calculation by capping at MAX/MIN
match sub.checked_div(self.step) {
Some(val) => val.saturating_add(1),
None => {
if self.step > 0 {
i64::MAX
} else {
i64::MIN
}
}
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use quickcheck_macros::quickcheck;
// Helper function to collect all values from a cursor, returns Result with error code
fn collect_series(start: i64, stop: i64, step: i64) -> Result<Vec<i64>, ResultCode> {
let mut cursor = GenerateSeriesCursor {
start: 0,
stop: 0,
step: 0,
current: 0,
};
// Create args array for filter
let args = vec![
Value::from_integer(start),
Value::from_integer(stop),
Value::from_integer(step),
];
// Validate inputs through filter
match GenerateSeriesVTab::filter(&mut cursor, 3, &args) {
ResultCode::OK => (),
ResultCode::EOF => return Ok(vec![]),
err => return Err(err),
}
let mut values = Vec::new();
loop {
values.push(cursor.column(0).to_integer().unwrap());
match GenerateSeriesVTab::next(&mut cursor) {
ResultCode::OK => (),
ResultCode::EOF => break,
err => return Err(err),
}
}
Ok(values)
}
#[quickcheck]
/// Test that the series length is correct for a positive step
/// Example:
/// start = 1, stop = 10, step = 1
/// expected length = 10
fn prop_series_length_positive_step(start: i64, stop: i64) {
// Limit the range to make test run faster, since we're testing with step 1.
let start = start % 1000;
let stop = stop % 1000;
let step = 1;
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start > stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}, got {:?}",
start,
stop,
step,
values
);
} else {
let expected_len = ((stop - start) / step + 1) as usize;
assert_eq!(
values.len(),
expected_len,
"Series length mismatch for start={}, stop={}, step={}: expected {}, got {}",
start,
stop,
step,
expected_len,
values.len()
);
}
}
#[quickcheck]
/// Test that the series length is correct for a negative step
/// Example:
/// start = 10, stop = 1, step = -1
/// expected length = 10
fn prop_series_length_negative_step(start: i64, stop: i64) {
// Limit the range to make test run faster, since we're testing with step -1.
let start = start % 1000;
let stop = stop % 1000;
let step = -1;
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start < stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else {
let expected_len = ((start - stop) / step.abs() + 1) as usize;
assert_eq!(
values.len(),
expected_len,
"Series length mismatch for start={}, stop={}, step={}: expected {}, got {}",
start,
stop,
step,
expected_len,
values.len()
);
}
}
#[quickcheck]
/// Test that the series is monotonically increasing
/// Example:
/// start = 1, stop = 10, step = 1
/// expected series = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
fn prop_series_monotonic_increasing(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 10000;
let stop = stop % 10000;
let step = (step % 100).max(1); // Ensure positive non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start > stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else {
assert!(
values.windows(2).all(|w| w[0] < w[1]),
"Series not monotonically increasing: {:?} (start={}, stop={}, step={})",
values,
start,
stop,
step
);
}
}
#[quickcheck]
/// Test that the series is monotonically decreasing
/// Example:
/// start = 10, stop = 1, step = -1
/// expected series = [10, 9, 8, 7, 6, 5, 4, 3, 2, 1]
fn prop_series_monotonic_decreasing(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 10000;
let stop = stop % 10000;
let step = -((step % 100).max(1)); // Ensure negative non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start < stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else {
assert!(
values.windows(2).all(|w| w[0] > w[1]),
"Series not monotonically decreasing: {:?} (start={}, stop={}, step={})",
values,
start,
stop,
step
);
}
}
#[quickcheck]
/// Test that the series step size is consistent
/// Example:
/// start = 1, stop = 10, step = 1
/// expected step size = 1
fn prop_series_step_size_positive(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 1000;
let stop = stop % 1000;
let step = (step % 100).max(1); // Ensure positive non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start > stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else if !values.is_empty() {
assert!(
values.windows(2).all(|w| (w[1] - w[0]).abs() == step.abs()),
"Step size not consistent: {:?} (expected step size: {})",
values,
step.abs()
);
}
}
#[quickcheck]
/// Test that the series step size is consistent for a negative step
/// Example:
/// start = 10, stop = 1, step = -1
/// expected step size = 1
fn prop_series_step_size_negative(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 1000;
let stop = stop % 1000;
let step = -((step % 100).max(1)); // Ensure negative non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start < stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else if !values.is_empty() {
assert!(
values.windows(2).all(|w| (w[1] - w[0]).abs() == step.abs()),
"Step size not consistent: {:?} (expected step size: {})",
values,
step.abs()
);
}
}
#[quickcheck]
/// Test that the series bounds are correct for a positive step
/// Example:
/// start = 1, stop = 10, step = 1
/// expected bounds = [1, 10]
fn prop_series_bounds_positive(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 10000;
let stop = stop % 10000;
let step = (step % 100).max(1); // Ensure positive non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start > stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else if !values.is_empty() {
assert_eq!(
values.first(),
Some(&start),
"Series doesn't start with start value: {:?} (expected start: {})",
values,
start
);
assert!(
values.last().map_or(true, |&last| last <= stop),
"Series exceeds stop value: {:?} (stop: {})",
values,
stop
);
}
}
#[quickcheck]
/// Test that the series bounds are correct for a negative step
/// Example:
/// start = 10, stop = 1, step = -1
/// expected bounds = [10, 1]
fn prop_series_bounds_negative(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster.
let start = start % 10000;
let stop = stop % 10000;
let step = -((step % 100).max(1)); // Ensure negative non-zero step
let values = collect_series(start, stop, step).unwrap_or_else(|e| {
panic!(
"Failed to generate series for start={}, stop={}, step={}: {:?}",
start, stop, step, e
)
});
if start < stop {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else if !values.is_empty() {
assert_eq!(
values.first(),
Some(&start),
"Series doesn't start with start value: {:?} (expected start: {})",
values,
start
);
assert!(
values.last().map_or(true, |&last| last >= stop),
"Series exceeds stop value: {:?} (stop: {})",
values,
stop
);
}
}
#[test]
fn test_series_empty_positive_step() {
let values = collect_series(10, 5, 1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Series should be empty when start > stop with positive step"
);
}
#[test]
fn test_series_empty_negative_step() {
let values = collect_series(5, 10, -1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Series should be empty when start < stop with negative step"
);
}
#[test]
fn test_series_single_element() {
let values = collect_series(5, 5, 1).expect("Failed to generate single element series");
assert_eq!(
values,
vec![5],
"Single element series should contain only the start value"
);
}
#[test]
fn test_zero_step_is_interpreted_as_1() {
let values = collect_series(1, 10, 0).expect("Failed to generate series");
assert_eq!(
values,
vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
"Zero step should be interpreted as 1"
);
}
#[test]
fn test_invalid_inputs() {
// Test that invalid ranges return empty series instead of errors
let values = collect_series(10, 1, 1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Invalid positive range should return empty series, got {:?}",
values
);
let values = collect_series(1, 10, -1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Invalid negative range should return empty series"
);
// Test that extreme ranges return empty series
let values = collect_series(i64::MAX, i64::MIN, 1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Extreme range (MAX to MIN) should return empty series"
);
let values = collect_series(i64::MIN, i64::MAX, -1).expect("Failed to generate series");
assert!(
values.is_empty(),
"Extreme range (MIN to MAX) should return empty series"
);
}
#[quickcheck]
/// Test that rowid is always monotonically increasing regardless of step direction
fn prop_series_rowid_monotonic(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster
let start = start % 1000;
let stop = stop % 1000;
let step = if step == 0 { 1 } else { step % 100 }; // Ensure non-zero step
let mut cursor = GenerateSeriesCursor {
start: 0,
stop: 0,
step: 0,
current: 0,
};
let args = vec![
Value::from_integer(start),
Value::from_integer(stop),
Value::from_integer(step),
];
// Initialize cursor through filter
GenerateSeriesVTab::filter(&mut cursor, 3, &args);
let mut rowids = vec![];
while !GenerateSeriesVTab::eof(&cursor) {
let cur_rowid = cursor.rowid();
match GenerateSeriesVTab::next(&mut cursor) {
ResultCode::OK => rowids.push(cur_rowid),
ResultCode::EOF => break,
err => panic!(
"Unexpected error {:?} for start={}, stop={}, step={}",
err, start, stop, step
),
}
}
assert!(
rowids.windows(2).all(|w| w[1] == w[0] + 1),
"Rowids not monotonically increasing: {:?} (start={}, stop={}, step={})",
rowids,
start,
stop,
step
);
}
// #[quickcheck]
// /// Test that integer overflow is properly handled
// fn prop_series_overflow(start: i64, step: i64) {
// // Use values close to MAX/MIN to test overflow
// let start = if start >= 0 {
// i64::MAX - (start % 100)
// } else {
// i64::MIN + (start.abs() % 100)
// };
// let step = if step == 0 { 1 } else { step }; // Ensure non-zero step
// let stop = if step > 0 { i64::MAX } else { i64::MIN };
// let result = collect_series(start, stop, step);
// // If we would overflow, expect InvalidArgs
// if start.checked_add(step).is_none() {
// assert!(
// matches!(result, Err(ResultCode::InvalidArgs)),
// "Expected InvalidArgs for overflow case: start={}, stop={}, step={}",
// start, stop, step
// );
// } else {
// // Otherwise the series should be valid
// let values = result.unwrap_or_else(|e| {
// panic!(
// "Failed to generate series for start={}, stop={}, step={}: {:?}",
// start, stop, step, e
// )
// });
// // Verify no values overflow
// for window in values.windows(2) {
// assert!(
// window[0].checked_add(step).is_some(),
// "Overflow occurred in series: {:?} + {} (start={}, stop={}, step={})",
// window[0], step, start, stop, step
// );
// }
// }
// }
#[quickcheck]
/// Test that empty series are handled consistently
fn prop_series_empty(start: i64, stop: i64, step: i64) {
// Limit the range to make test run faster
let start = start % 1000;
let stop = stop % 1000;
let step = if step == 0 { 1 } else { step % 100 }; // Ensure non-zero step
let result = collect_series(start, stop, step);
match result {
Ok(values) => {
if (step > 0 && start > stop) || (step < 0 && start < stop) {
assert!(
values.is_empty(),
"Series should be empty for invalid range: start={}, stop={}, step={}",
start,
stop,
step
);
} else if start == stop {
assert_eq!(
values,
vec![start],
"Series with start==stop should contain exactly one element"
);
}
}
Err(e) => panic!(
"Unexpected error for start={}, stop={}, step={}: {:?}",
start, stop, step, e
),
}
}
}