Fix computation of argv_index in best_index

The `filter` methods for extensions affected by this fix expect arguments
to be passed in a specific order. For example, `generate_series` assumes
that if the `start` argument exists, it is always passed to `filter`
first. If `start` does not exist, then `stop` is passed first — but
`stop` must never come before `start`.

Previously, this was not guaranteed: `best_index` relied on constraints
being passed in the order matching `filter`'s expectations.
This commit is contained in:
Piotr Rzysko
2025-07-31 09:18:55 +02:00
parent c465ce6e7b
commit 6a4cf02a90
4 changed files with 394 additions and 55 deletions

View File

@@ -190,23 +190,23 @@ impl PragmaVirtualTable {
}
}
let mut argv_idx = 1;
let argv_arg0_idx = arg0_idx.map_or(0, |_| 1);
let argv_arg1_idx = arg1_idx.map_or(argv_arg0_idx, |_| argv_arg0_idx + 1);
let constraint_usages = constraints
.iter()
.enumerate()
.map(|(i, _)| {
if Some(i) == arg0_idx || Some(i) == arg1_idx {
let usage = ConstraintUsage {
argv_index: Some(argv_idx),
omit: true,
};
argv_idx += 1;
usage
let argv_index = if Some(i) == arg0_idx {
Some(argv_arg0_idx)
} else if Some(i) == arg1_idx {
Some(argv_arg1_idx)
} else {
ConstraintUsage {
argv_index: None,
omit: false,
}
None
};
ConstraintUsage {
argv_index,
omit: argv_index.is_some(),
}
})
.collect();
@@ -309,3 +309,151 @@ impl PragmaVirtualTableCursor {
self.next()
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_best_index_argv_order_both_hidden_constraints() {
// Test when both hidden constraints are present (arg and schema)
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 6,
max_arg_count: 2,
has_pragma_arg: true,
};
let constraints = vec![
usable_constraint(6), // arg (first hidden column)
usable_constraint(7), // schema (second hidden column)
];
let index_info = pragma_vtab.best_index(&constraints);
// Verify arg gets argv_index 1, schema gets argv_index 2
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // arg
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // schema
assert!(index_info.constraint_usages[0].omit);
assert!(index_info.constraint_usages[1].omit);
}
#[test]
fn test_best_index_argv_order_only_arg() {
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 6,
max_arg_count: 2,
has_pragma_arg: true,
};
let constraints = vec![
usable_constraint(6), // arg (first hidden column)
];
let index_info = pragma_vtab.best_index(&constraints);
// Verify arg gets argv_index 1
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // arg
assert!(index_info.constraint_usages[0].omit);
}
#[test]
fn test_best_index_argv_order_only_schema() {
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 1,
max_arg_count: 1,
has_pragma_arg: false,
};
let constraints = vec![
usable_constraint(1), // schema (first hidden column after visible columns)
];
let index_info = pragma_vtab.best_index(&constraints);
// Verify schema gets argv_index 1
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // schema
assert!(index_info.constraint_usages[0].omit);
}
#[test]
fn test_best_index_argv_order_reverse_constraint_order() {
// Test when constraints are provided in reverse order (schema first, then arg)
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 6,
max_arg_count: 2,
has_pragma_arg: true,
};
let constraints = vec![
usable_constraint(7), // schema (second hidden column)
usable_constraint(6), // arg (first hidden column)
];
let index_info = pragma_vtab.best_index(&constraints);
// Verify arg still gets argv_index 1, schema gets argv_index 2 regardless of constraint order
assert_eq!(index_info.constraint_usages[0].argv_index, Some(2)); // schema
assert_eq!(index_info.constraint_usages[1].argv_index, Some(1)); // arg
assert!(index_info.constraint_usages[0].omit);
assert!(index_info.constraint_usages[1].omit);
}
#[test]
fn test_best_index_visible_columns_ignored() {
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 6,
max_arg_count: 2,
has_pragma_arg: true,
};
let constraints = vec![
usable_constraint(0), // visible column (cid)
usable_constraint(6), // arg (hidden)
];
let index_info = pragma_vtab.best_index(&constraints);
// Verify visible column constraint is ignored, arg gets argv_index 1
assert_eq!(index_info.constraint_usages[0].argv_index, None); // visible column
assert_eq!(index_info.constraint_usages[1].argv_index, Some(1)); // arg
assert!(!index_info.constraint_usages[0].omit); // visible column not omitted
assert!(index_info.constraint_usages[1].omit); // arg omitted
}
#[test]
fn test_best_index_no_usable_constraints() {
let pragma_vtab = PragmaVirtualTable {
pragma_name: "table_info".to_string(),
visible_column_count: 6,
max_arg_count: 2,
has_pragma_arg: true,
};
let constraints = vec![ConstraintInfo {
column_index: 6,
op: ConstraintOp::Eq,
usable: false,
plan_info: 0,
}];
let index_info = pragma_vtab.best_index(&constraints);
// Verify no argv_index is assigned
assert_eq!(index_info.constraint_usages[0].argv_index, None);
assert!(!index_info.constraint_usages[0].omit);
}
fn usable_constraint(column_index: u32) -> ConstraintInfo {
ConstraintInfo {
column_index,
op: ConstraintOp::Eq,
usable: true,
plan_info: 0,
}
}
}

View File

@@ -58,53 +58,50 @@ impl VTable for GenerateSeriesTable {
}
fn best_index(constraints: &[ConstraintInfo], _order_by: &[OrderByInfo]) -> IndexInfo {
const START_COLUMN_INDEX: u32 = 1;
const STEP_COLUMN_INDEX: u32 = 3;
// The bits of `idx_num` are used to indicate which arguments are available to the filter method:
// - Bit 0 set -> 'start' is available
// - Bit 1 set -> 'stop' is available
// - Bit 2 set -> 'step' is available
let mut idx_num = 0;
let mut start_idx = None;
let mut stop_idx = None;
let mut step_idx = None;
let mut positions = [None; 4]; // maps column index to constraint position
for (i, c) in constraints.iter().enumerate() {
if !c.usable || c.op != ConstraintOp::Eq {
continue;
}
match c.column_index {
1 => {
start_idx = Some(i);
idx_num |= 1;
}
2 => {
stop_idx = Some(i);
idx_num |= 2;
}
3 => {
step_idx = Some(i);
idx_num |= 4;
}
_ => {}
if c.column_index >= START_COLUMN_INDEX && c.column_index <= STEP_COLUMN_INDEX {
let bit = 1 << (c.column_index - 1);
idx_num |= bit;
positions[c.column_index as usize] = Some(i);
}
}
// Assign argv indexes contiguously
let mut argv_idx = 1;
let mut argv_indexes = [None; 4];
for (i, pos) in positions.iter().enumerate() {
if pos.is_some() {
argv_indexes[i] = Some(argv_idx);
argv_idx += 1;
}
}
let constraint_usages = constraints
.iter()
.enumerate()
.map(|(i, _)| {
if Some(i) == start_idx || Some(i) == stop_idx || Some(i) == step_idx {
let usage = ConstraintUsage {
argv_index: Some(argv_idx),
omit: true,
};
argv_idx += 1;
usage
} else {
ConstraintUsage {
argv_index: None,
omit: false,
}
.map(|(idx, c)| {
let argv_index = positions.get(c.column_index as usize).and_then(|&pos| {
pos.filter(|&i| i == idx)
.and_then(|_| argv_indexes[c.column_index as usize])
});
ConstraintUsage {
argv_index,
omit: argv_index.is_some(),
}
})
.collect();
@@ -668,4 +665,109 @@ mod tests {
((stop.saturating_sub(start)).saturating_div(step)).saturating_add(1) as usize
}
}
#[test]
fn test_best_index_argv_order_all_constraints() {
// Test when start, stop, and step constraints are present
let constraints = vec![
usable_constraint(1), // start
usable_constraint(2), // stop
usable_constraint(3), // step
];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify start gets argv_index 1, stop gets 2, step gets 3
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // start
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // stop
assert_eq!(index_info.constraint_usages[2].argv_index, Some(3)); // step
assert_eq!(index_info.idx_num, 7); // All bits set (1 | 2 | 4)
}
#[test]
fn test_best_index_argv_order_start_stop_only() {
let constraints = vec![
usable_constraint(1), // start
usable_constraint(2), // stop
];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify start gets argv_index 1, stop gets 2
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // start
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // stop
assert_eq!(index_info.idx_num, 3); // Bits 0 and 1 set (1 | 2)
}
#[test]
fn test_best_index_argv_order_only_start() {
let constraints = vec![
usable_constraint(1), // start
];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify start gets argv_index 1
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // start
assert_eq!(index_info.idx_num, 1); // Only bit 0 set
}
#[test]
fn test_best_index_argv_order_reverse_constraint_order() {
// Test when constraints are provided in reverse order (step, stop, start)
let constraints = vec![
usable_constraint(3), // step
usable_constraint(2), // stop
usable_constraint(1), // start
];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify start still gets argv_index 1, stop gets 2, step gets 3 regardless of constraint order
assert_eq!(index_info.constraint_usages[0].argv_index, Some(3)); // step
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // stop
assert_eq!(index_info.constraint_usages[2].argv_index, Some(1)); // start
assert_eq!(index_info.idx_num, 7); // All bits set (1 | 2 | 4)
}
#[test]
fn test_best_index_argv_order_missing_start() {
// Test when start constraint is missing but stop and step are present
let constraints = vec![
usable_constraint(2), // stop
usable_constraint(3), // step
];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify stop gets argv_index 1, step gets 2
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // stop
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // step
assert_eq!(index_info.idx_num, 6); // Bits 1 and 2 set (2 | 4)
}
#[test]
fn test_best_index_no_usable_constraints() {
let constraints = vec![ConstraintInfo {
column_index: 1,
op: ConstraintOp::Eq,
usable: false,
plan_info: 0,
}];
let index_info = GenerateSeriesTable::best_index(&constraints, &[]);
// Verify no argv_index is assigned
assert_eq!(index_info.constraint_usages[0].argv_index, None);
assert_eq!(index_info.idx_num, 0); // No bits set
}
fn usable_constraint(column_index: u32) -> ConstraintInfo {
ConstraintInfo {
column_index,
op: ConstraintOp::Eq,
usable: true,
plan_info: 0,
}
}
}

View File

@@ -117,23 +117,23 @@ impl VTable for CompletionTable {
}
}
let mut argv_idx = 1;
let argv_prefix_idx = prefix_idx.map_or(0, |_| 1);
let argv_wholeline_idx = wholeline_idx.map_or(argv_prefix_idx, |_| argv_prefix_idx + 1);
let constraint_usages = constraints
.iter()
.enumerate()
.map(|(i, _)| {
if Some(i) == prefix_idx || Some(i) == wholeline_idx {
let usage = ConstraintUsage {
argv_index: Some(argv_idx),
omit: true,
};
argv_idx += 1;
usage
let argv_index = if Some(i) == prefix_idx {
Some(argv_prefix_idx)
} else if Some(i) == wholeline_idx {
Some(argv_wholeline_idx)
} else {
ConstraintUsage {
argv_index: None,
omit: false,
}
None
};
ConstraintUsage {
argv_index,
omit: argv_index.is_some(),
}
})
.collect();
@@ -271,4 +271,89 @@ impl VTabCursor for CompletionCursor {
}
#[cfg(test)]
mod tests {}
mod tests {
use super::*;
#[test]
fn test_best_index_argv_order_both_constraints() {
// Test when both prefix and wholeline constraints are present
let constraints = vec![
usable_constraint(1), // prefix
usable_constraint(2), // wholeline
];
let index_info = CompletionTable::best_index(&constraints, &[]);
// Verify prefix gets argv_index 1 and wholeline gets argv_index 2
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // prefix
assert_eq!(index_info.constraint_usages[1].argv_index, Some(2)); // wholeline
assert_eq!(index_info.idx_num, 3); // Both bits set (1 | 2)
}
#[test]
fn test_best_index_argv_order_only_wholeline() {
let constraints = vec![
usable_constraint(2), // wholeline
];
let index_info = CompletionTable::best_index(&constraints, &[]);
// Verify wholeline gets argv_index 1 when prefix is missing
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // wholeline
assert_eq!(index_info.idx_num, 2); // Only bit 1 set
}
#[test]
fn test_best_index_argv_order_only_prefix() {
let constraints = vec![
usable_constraint(1), // prefix
];
let index_info = CompletionTable::best_index(&constraints, &[]);
// Verify prefix gets argv_index 1
assert_eq!(index_info.constraint_usages[0].argv_index, Some(1)); // prefix
assert_eq!(index_info.idx_num, 1); // Only bit 0 set
}
#[test]
fn test_best_index_argv_order_reverse_constraint_order() {
// Test when constraints are provided in reverse order (wholeline first, then prefix)
let constraints = vec![
usable_constraint(2), // wholeline
usable_constraint(1), // prefix
];
let index_info = CompletionTable::best_index(&constraints, &[]);
// Verify prefix still gets argv_index 1 and wholeline gets argv_index 2 regardless of constraint order
assert_eq!(index_info.constraint_usages[0].argv_index, Some(2)); // wholeline
assert_eq!(index_info.constraint_usages[1].argv_index, Some(1)); // prefix
assert_eq!(index_info.idx_num, 3); // Both bits set (1 | 2)
}
#[test]
fn test_best_index_no_usable_constraints() {
let constraints = vec![ConstraintInfo {
column_index: 1,
op: ConstraintOp::Eq,
usable: false,
plan_info: 0,
}];
let index_info = CompletionTable::best_index(&constraints, &[]);
// Verify no argv_index is assigned
assert_eq!(index_info.constraint_usages[0].argv_index, None);
assert_eq!(index_info.idx_num, 0); // No bits set
}
fn usable_constraint(column_index: u32) -> ConstraintInfo {
ConstraintInfo {
column_index,
op: ConstraintOp::Eq,
usable: true,
plan_info: 0,
}
}
}

View File

@@ -320,6 +320,10 @@ def _test_series(limbo: TestTursoShell):
"SELECT * FROM generate_series WHERE start = 1 AND stop = 10;",
lambda res: res == "1\n2\n3\n4\n5\n6\n7\n8\n9\n10",
)
limbo.run_test_fn(
"SELECT * FROM generate_series WHERE stop = 10 AND start = 1;",
lambda res: res == "1\n2\n3\n4\n5\n6\n7\n8\n9\n10",
)
limbo.run_test_fn(
"SELECT * FROM generate_series(1, 10) WHERE value < 5;",
lambda res: res == "1\n2\n3\n4",