Make WhereTerm::consumed a plain bool

Now that virtual tables are integrated into the optimizer, this field no
longer needs to be wrapped in Cell<bool>.
This commit is contained in:
Piotr Rzysko
2025-08-03 12:24:30 +02:00
parent 99f87c07c1
commit 8fb4fbf8af
6 changed files with 35 additions and 47 deletions

View File

@@ -501,7 +501,7 @@ fn generate_join_bitmasks(table_number_max_exclusive: usize, how_many: usize) ->
#[cfg(test)]
mod tests {
use std::{cell::Cell, sync::Arc};
use std::sync::Arc;
use turso_sqlite3_parser::ast::{self, Expr, Operator, SortOrder, TableInternalId};
@@ -1344,7 +1344,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(5.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
let table_references = TableReferences::new(joined_tables, vec![]);
@@ -1436,7 +1436,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(5.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
},
WhereTerm {
expr: Expr::Binary(
@@ -1450,7 +1450,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(7.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
},
];
@@ -1547,7 +1547,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(5.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
},
WhereTerm {
expr: Expr::Binary(
@@ -1561,7 +1561,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(10.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
},
WhereTerm {
expr: Expr::Binary(
@@ -1575,7 +1575,7 @@ mod tests {
Box::new(Expr::Literal(ast::Literal::Numeric(7.to_string()))),
),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
},
];
@@ -1692,7 +1692,7 @@ mod tests {
WhereTerm {
expr: Expr::Binary(Box::new(lhs), op, Box::new(rhs)),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}
}

View File

@@ -1,5 +1,3 @@
use std::cell::Cell;
use turso_sqlite3_parser::ast::{Expr, Operator};
use crate::{
@@ -115,7 +113,7 @@ pub(crate) fn lift_common_subexpressions_from_binary_or_terms(
if found_non_empty_or_branches {
// If we found an empty OR branch, we can remove the entire OR term.
// E.g. (a AND b) OR (a) OR (a AND c) just becomes a.
where_clause[i].consumed.set(true);
where_clause[i].consumed = true;
} else {
assert!(new_or_operands_for_original_term.len() > 1);
// Update the original WhereTerm's expression with the new OR structure (without common parts).
@@ -127,7 +125,7 @@ pub(crate) fn lift_common_subexpressions_from_binary_or_terms(
where_clause.push(WhereTerm {
expr: common_expr_to_add,
from_outer_join: term_from_outer_join,
consumed: Cell::new(false),
consumed: false,
});
}
@@ -258,7 +256,7 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: or_expr,
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
@@ -269,7 +267,7 @@ mod tests {
// 3. b = 1
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 3);
assert_eq!(
@@ -357,7 +355,7 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: or_expr,
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
@@ -367,7 +365,7 @@ mod tests {
// 2. a = 1
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 2);
assert_eq!(
@@ -424,7 +422,7 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: or_expr.clone(),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
@@ -432,7 +430,7 @@ mod tests {
// Should remain unchanged since no common terms
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 1);
assert_eq!(nonconsumed_terms[0].expr, or_expr);
@@ -491,7 +489,7 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: or_expr,
from_outer_join: Some(TableInternalId::default()), // Set from_outer_join
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
@@ -499,7 +497,7 @@ mod tests {
// Should have 2 terms, both with from_outer_join set
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 2);
assert_eq!(
@@ -543,7 +541,7 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: single_expr.clone(),
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
@@ -551,7 +549,7 @@ mod tests {
// Should remain unchanged
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 1);
assert_eq!(nonconsumed_terms[0].expr, single_expr);
@@ -596,14 +594,14 @@ mod tests {
let mut where_clause = vec![WhereTerm {
expr: or_expr,
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
}];
lift_common_subexpressions_from_binary_or_terms(&mut where_clause)?;
let nonconsumed_terms = where_clause
.iter()
.filter(|term| !term.consumed.get())
.filter(|term| !term.consumed)
.collect::<Vec<_>>();
assert_eq!(nonconsumed_terms.len(), 1);
assert_eq!(nonconsumed_terms[0].expr, a_expr);

View File

@@ -319,13 +319,11 @@ fn optimize_table_access(
let constraint =
&constraints_per_table[table_idx].constraints[cref.constraint_vec_pos];
assert!(
!where_clause[constraint.where_clause_pos.0].consumed.get(),
!where_clause[constraint.where_clause_pos.0].consumed,
"trying to consume a where clause term twice: {:?}",
where_clause[constraint.where_clause_pos.0]
);
where_clause[constraint.where_clause_pos.0]
.consumed
.set(true);
where_clause[constraint.where_clause_pos.0].consumed = true;
}
if let Some(index) = &index {
joined_tables[table_idx].op = Operation::Search(Search::Seek {
@@ -428,9 +426,7 @@ fn build_vtab_scan_op(
let (pred_idx, _) = vtab_constraint.unpack_plan_info();
let constraint = &table_constraints.constraints[pred_idx];
if usage.omit {
where_clause[constraint.where_clause_pos.0]
.consumed
.set(true);
where_clause[constraint.where_clause_pos.0].consumed = true;
}
let expr = constraint.get_constraining_expr(where_clause);
constraints[zero_based_argv_index] = Some(expr);
@@ -476,7 +472,7 @@ fn eliminate_constant_conditions(
let predicate = &where_clause[i];
if predicate.expr.is_always_true()? {
// true predicates can be removed since they don't affect the result
where_clause[i].consumed.set(true);
where_clause[i].consumed = true;
i += 1;
} else if predicate.expr.is_always_false()? {
// any false predicate in a list of conjuncts (AND-ed predicates) will make the whole list false,
@@ -487,7 +483,7 @@ fn eliminate_constant_conditions(
}
where_clause
.iter_mut()
.for_each(|term| term.consumed.set(true));
.for_each(|term| term.consumed = true);
return Ok(ConstantConditionEliminationResult::ImpossibleCondition);
} else {
i += 1;

View File

@@ -1,4 +1,4 @@
use std::{cell::Cell, cmp::Ordering, sync::Arc};
use std::{cmp::Ordering, sync::Arc};
use turso_sqlite3_parser::ast::{self, SortOrder};
use crate::{
@@ -84,16 +84,12 @@ pub struct WhereTerm {
/// A term may have been consumed e.g. if:
/// - it has been converted into a constraint in a seek key
/// - it has been removed due to being trivially true or false
///
/// FIXME: this can be made into a simple `bool` once we move the virtual table constraint resolution
/// code out of `init_loop()`, because that's the only place that requires a mutable reference to the where clause
/// that causes problems to other code that needs immutable references to the where clause.
pub consumed: Cell<bool>,
pub consumed: bool,
}
impl WhereTerm {
pub fn should_eval_before_loop(&self, join_order: &[JoinOrderMember]) -> bool {
if self.consumed.get() {
if self.consumed {
return false;
}
let Ok(eval_at) = self.eval_at(join_order) else {
@@ -103,7 +99,7 @@ impl WhereTerm {
}
pub fn should_eval_at_loop(&self, loop_idx: usize, join_order: &[JoinOrderMember]) -> bool {
if self.consumed.get() {
if self.consumed {
return false;
}
let Ok(eval_at) = self.eval_at(join_order) else {

View File

@@ -1,4 +1,3 @@
use std::cell::Cell;
use std::sync::Arc;
use super::{
@@ -662,7 +661,7 @@ pub fn parse_where(
out_where_clause.push(WhereTerm {
expr,
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
});
}
Ok(())
@@ -950,7 +949,7 @@ fn parse_join(
} else {
None
},
consumed: Cell::new(false),
consumed: false,
});
}
}
@@ -1031,7 +1030,7 @@ fn parse_join(
} else {
None
},
consumed: Cell::new(false),
consumed: false,
});
}
using = Some(distinct_names);

View File

@@ -16,7 +16,6 @@ use crate::vdbe::builder::{ProgramBuilderOpts, TableRefIdCounter};
use crate::vdbe::insn::Insn;
use crate::{schema::Schema, vdbe::builder::ProgramBuilder, Result};
use crate::{Connection, SymbolTable};
use std::cell::Cell;
use std::sync::Arc;
use turso_sqlite3_parser::ast::{self, CompoundSelect, Expr, SortOrder};
use turso_sqlite3_parser::ast::{ResultColumn, SelectInner};
@@ -661,7 +660,7 @@ fn add_vtab_predicates_to_where_clause(
plan.where_clause.push(WhereTerm {
expr,
from_outer_join: None,
consumed: Cell::new(false),
consumed: false,
});
}
Ok(())