From 74e04634aa8811d27cc8560fc794a21bb5b6a5d5 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 10 Oct 2025 13:12:53 +0300 Subject: [PATCH] Fix incorrectly using an equality constraint twice for index seek Prevents something like `WHERE x = 5 AND x = 5` from becoming a two component index key. Closes #3656 --- core/translate/optimizer/constraints.rs | 28 ++++++++++++++++++------- testing/join.test | 7 +++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/core/translate/optimizer/constraints.rs b/core/translate/optimizer/constraints.rs index 7e32f17c5..486c36687 100644 --- a/core/translate/optimizer/constraints.rs +++ b/core/translate/optimizer/constraints.rs @@ -486,7 +486,7 @@ pub fn usable_constraints_for_join_order<'a>( .map(|j| j.original_idx), ); let mut usable: Vec = Vec::new(); - let mut last_column_pos = 0; + let mut current_required_column_pos = 0; for cref in refs.iter() { let constraint = &constraints[cref.constraint_vec_pos]; let other_side_refers_to_self = constraint.lhs_mask.contains_table(table_idx); @@ -498,6 +498,7 @@ pub fn usable_constraints_for_join_order<'a>( break; } if Some(cref.index_col_pos) == usable.last().map(|x| x.index_col_pos) { + // Two constraints on the same index column can be combined into a single range constraint. assert_eq!(cref.sort_order, usable.last().unwrap().sort_order); assert_eq!(cref.index_col_pos, usable.last().unwrap().index_col_pos); assert_eq!( @@ -520,15 +521,28 @@ pub fn usable_constraints_for_join_order<'a>( } continue; } - if cref.index_col_pos != last_column_pos { + if cref.index_col_pos != current_required_column_pos { + // Index columns must be consumed contiguously in the order they appear in the index. break; } if usable.last().is_some_and(|x| x.eq.is_none()) { + // Usable index key must have 0-n equalities and then a maximum of 1 range constraint with one or both bounds set. + // If we already have a range constraint before this one, we must not add anything to it break; } - let constraint_group = match constraints[cref.constraint_vec_pos].operator { + let operator = constraints[cref.constraint_vec_pos].operator; + let table_col_pos = constraints[cref.constraint_vec_pos].table_col_pos; + if operator == ast::Operator::Equals + && usable + .last() + .is_some_and(|x| x.table_col_pos == table_col_pos) + { + // If we already have an equality constraint for this column, we can't use it again + continue; + } + let constraint_group = match operator { ast::Operator::Equals => RangeConstraintRef { - table_col_pos: constraints[cref.constraint_vec_pos].table_col_pos, + table_col_pos, index_col_pos: cref.index_col_pos, sort_order: cref.sort_order, eq: Some(cref.constraint_vec_pos), @@ -536,7 +550,7 @@ pub fn usable_constraints_for_join_order<'a>( upper_bound: None, }, ast::Operator::Greater | ast::Operator::GreaterEquals => RangeConstraintRef { - table_col_pos: constraints[cref.constraint_vec_pos].table_col_pos, + table_col_pos, index_col_pos: cref.index_col_pos, sort_order: cref.sort_order, eq: None, @@ -544,7 +558,7 @@ pub fn usable_constraints_for_join_order<'a>( upper_bound: None, }, ast::Operator::Less | ast::Operator::LessEquals => RangeConstraintRef { - table_col_pos: constraints[cref.constraint_vec_pos].table_col_pos, + table_col_pos, index_col_pos: cref.index_col_pos, sort_order: cref.sort_order, eq: None, @@ -554,7 +568,7 @@ pub fn usable_constraints_for_join_order<'a>( _ => continue, }; usable.push(constraint_group); - last_column_pos += 1; + current_required_column_pos += 1; } usable } diff --git a/testing/join.test b/testing/join.test index c14b96ab4..6c6aa7314 100755 --- a/testing/join.test +++ b/testing/join.test @@ -384,3 +384,10 @@ do_execsql_test_on_specific_db {:memory:} left-join-using-null { select a, b from t left join s using (a, b); } {1| 2|} + +# Regression test for: https://github.com/tursodatabase/turso/issues/3656 +do_execsql_test_on_specific_db {:memory:} redundant-join-condition { + create table t(x); + insert into t values ('lol'); + select t1.x from t t1 join t t2 on t1.x=t2.x where t1.x=t2.x; +} {lol} \ No newline at end of file