From 93e7bb5593d63464d5d97fb0e93c68692c1bbe75 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:15:26 +0400 Subject: [PATCH 01/12] add simple test --- .../query_processing/test_write_path.rs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index cfc95876b..53cebc7bd 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -1,4 +1,4 @@ -use crate::common::{self, maybe_setup_tracing}; +use crate::common::{self, limbo_exec_rows, maybe_setup_tracing}; use crate::common::{compare_string, do_flush, TempDatabase}; use log::debug; use std::io::{Read, Seek, Write}; @@ -790,3 +790,24 @@ fn test_insert_with_column_names() -> anyhow::Result<()> { Ok(()) } + +#[test] +pub fn delete_search_op_ignore_nulls() { + let limbo = TempDatabase::new_empty(true); + let conn = limbo.db.connect().unwrap(); + for sql in [ + "CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, c INT);", + "CREATE UNIQUE INDEX t_idx ON t(c);", + "INSERT INTO t VALUES (NULL, NULL)", + "DELETE FROM t WHERE c < -1;", + ] { + conn.execute(sql).unwrap(); + } + assert_eq!( + vec![vec![ + rusqlite::types::Value::Integer(1), + rusqlite::types::Value::Null + ]], + limbo_exec_rows(&limbo, &conn, "SELECT * FROM t ORDER BY id") + ); +} From e9b8b0265d86f6f4874f6a75ddcef5413c3bcb9e Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:16:04 +0400 Subject: [PATCH 02/12] skip NULL in case of search over index --- core/translate/main_loop.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index eb1c874e9..6b5107259 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -1148,9 +1148,19 @@ fn emit_seek( // depending on the iteration direction. match seek_def.iter_dir { IterationDirection::Forwards => { - program.emit_insn(Insn::Rewind { + // the seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) + // so, NULL always must be filtered out + // (note, that complex filters like c < ? OR c IS NULL will produce Scan operation instead of Search) + program.emit_insn(Insn::Null { + dest: start_reg, + dest_end: None, + }); + program.emit_insn(Insn::SeekGT { + is_index, cursor_id: seek_cursor_id, - pc_if_empty: loop_end, + start_reg, + num_regs: 1, + target_pc: loop_end, }); } IterationDirection::Backwards => { From a32ed53bd8775d753919be7bca601fdbaf6b3479 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:37:41 +0400 Subject: [PATCH 03/12] remove optimization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - even if index search will return only 1 row - it will call next in the loop - and we incorrecty can process same row values multiple times - the following query failed with this optimization: turso> CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, k TEXT, c0 INT); turso> CREATE UNIQUE INDEX idx_p1_0 ON t(c0); turso> insert into t values (null, 'uu', -1); turso> insert into t values (null, 'uu', -2); turso> UPDATE t SET c0 = NULL WHERE c0 = -1; turso> SELECT * FROM t ┌────┬────┬────┐ │ id │ k │ c0 │ ├────┼────┼────┤ │ 1 │ uu │ │ ├────┼────┼────┤ │ 2 │ uu │ │ └────┴────┴────┘ --- core/translate/optimizer/mod.rs | 4 ---- core/translate/plan.rs | 13 ------------- 2 files changed, 17 deletions(-) diff --git a/core/translate/optimizer/mod.rs b/core/translate/optimizer/mod.rs index 627017b27..a9829208e 100644 --- a/core/translate/optimizer/mod.rs +++ b/core/translate/optimizer/mod.rs @@ -142,10 +142,6 @@ fn optimize_update_plan(plan: &mut UpdatePlan, schema: &Schema) -> Result<()> { if !plan.indexes_to_update.iter().any(|i| Arc::ptr_eq(index, i)) { return Ok(()); } - // Fine to use index if we aren't going to be iterating over it, since it returns at most 1 row. - if table_ref.op.returns_max_1_row() { - return Ok(()); - } // Otherwise, fall back to a table scan. table_ref.op = Operation::Scan(Scan::BTreeTable { iter_dir: IterationDirection::Forwards, diff --git a/core/translate/plan.rs b/core/translate/plan.rs index a7fd42dba..afcb46882 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -795,19 +795,6 @@ impl Operation { Operation::Search(Search::Seek { index, .. }) => index.as_ref(), } } - - pub fn returns_max_1_row(&self) -> bool { - match self { - Operation::Scan(_) => false, - Operation::Search(Search::RowidEq { .. }) => true, - Operation::Search(Search::Seek { index, seek_def }) => { - let Some(index) = index else { - return false; - }; - index.unique && seek_def.seek.as_ref().is_some_and(|seek| seek.op.eq_only()) - } - } - } } impl JoinedTable { From 6d3bdc81e52129f4a032474e26971906cad0857e Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:46:43 +0400 Subject: [PATCH 04/12] add simple test --- .../query_processing/test_write_path.rs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index 53cebc7bd..3e0be89e9 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -811,3 +811,31 @@ pub fn delete_search_op_ignore_nulls() { limbo_exec_rows(&limbo, &conn, "SELECT * FROM t ORDER BY id") ); } + +#[test] +pub fn delete_eq_correct() { + let limbo = TempDatabase::new_empty(true); + let conn = limbo.db.connect().unwrap(); + for sql in [ + "CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, c INT);", + "CREATE UNIQUE INDEX t_idx ON t(c);", + "INSERT INTO t VALUES (null, -1);", + "INSERT INTO t VALUES (null, -2);", + "UPDATE t SET c = NULL WHERE c = -1;", + ] { + conn.execute(sql).unwrap(); + } + assert_eq!( + vec![ + vec![ + rusqlite::types::Value::Integer(1), + rusqlite::types::Value::Null + ], + vec![ + rusqlite::types::Value::Integer(2), + rusqlite::types::Value::Integer(-2), + ] + ], + limbo_exec_rows(&limbo, &conn, "SELECT * FROM t ORDER BY id") + ); +} From 5693d4052c937b5078e113909c66e6dac35a0526 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:47:01 +0400 Subject: [PATCH 05/12] improve fuzz test --- tests/integration/fuzz/mod.rs | 105 ++++++++++++++++++++++------------ 1 file changed, 67 insertions(+), 38 deletions(-) diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index f5e96dee2..e5674d98f 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -672,6 +672,15 @@ mod tests { #[test] pub fn partial_index_mutation_and_upsert_fuzz() { + index_mutation_upsert_fuzz(1.0, 1); + } + + #[test] + pub fn simple_index_mutation_and_upsert_fuzz() { + index_mutation_upsert_fuzz(0.0, 0); + } + + fn index_mutation_upsert_fuzz(partial_index_prob: f64, conflict_chain_max_len: u32) { let _ = env_logger::try_init(); const OUTER_ITERS: usize = 5; const INNER_ITERS: usize = 500; @@ -723,6 +732,7 @@ mod tests { let functions = ["lower", "upper", "length"]; let num_pidx = rng.random_range(0..=3); + let mut conflict_match_targets = vec!["".to_string(), "(id)".to_string()]; let mut idx_ddls: Vec = Vec::new(); for i in 0..num_pidx { // Pick 1 or 2 key columns; always include "k" sometimes to get frequent conflicts. @@ -790,13 +800,24 @@ mod tests { parts.join(" AND ") }; - let ddl = format!( - "CREATE UNIQUE INDEX idx_p{}_{} ON t({}) WHERE {}", - outer, - i, - key_cols.join(","), - pred - ); + let ddl = if rng.random_bool(partial_index_prob) { + format!( + "CREATE UNIQUE INDEX idx_p{}_{} ON t({}) WHERE {}", + outer, + i, + key_cols.join(","), + pred + ) + } else { + // ON CONFLICT (...) can use only column set with non-partial UNIQUE constraint + conflict_match_targets.push(format!("({})", key_cols.join(","))); + format!( + "CREATE UNIQUE INDEX idx_p{}_{} ON t({})", + outer, + i, + key_cols.join(","), + ) + }; idx_ddls.push(ddl.clone()); // Create in both engines println!("{ddl};"); @@ -817,9 +838,11 @@ mod tests { vals.push(v); } let ins = format!("INSERT INTO t VALUES ({})", vals.join(", ")); + println!("{ins}; -- seed"); // Execute on both; ignore errors due to partial unique conflicts (keep seeding going) - let _ = sqlite.execute(&ins, rusqlite::params![]); - let _ = limbo_exec_rows_fallible(&limbo_db, &limbo_conn, &ins); + let sqlite_res = sqlite.execute(&ins, rusqlite::params![]); + let limbo_res = limbo_exec_rows_fallible(&limbo_db, &limbo_conn, &ins); + assert!(sqlite_res.is_ok() == limbo_res.is_ok()); } for _ in 0..INNER_ITERS { @@ -909,37 +932,43 @@ mod tests { }); } } - if rng.random_bool(0.3) { - // 30% chance ON CONFLICT DO UPDATE SET ... - let mut set_list = Vec::new(); - let num_set = rng.random_range(1..=cols_list.len()); - let set_cols = cols_list - .choose_multiple(&mut rng, num_set) - .cloned() - .collect::>(); - for c in set_cols.iter() { - let v = if c == "k" { - format!("'{}'", K_POOL.choose(&mut rng).unwrap()) - } else if rng.random_bool(0.2) { - "NULL".into() - } else { - rng.random_range(-5..=15).to_string() - }; - set_list.push(format!("{c} = {v}")); + let chain_length = rng.random_range(0..=conflict_chain_max_len); + let mut on_conflict = String::new(); + for _ in 0..chain_length { + let idx = rng.random_range(0..conflict_match_targets.len()); + let target = &conflict_match_targets[idx]; + if rng.random_bool(0.8) { + let mut set_list = Vec::new(); + let num_set = rng.random_range(1..=cols_list.len()); + let set_cols = cols_list + .choose_multiple(&mut rng, num_set) + .cloned() + .collect::>(); + for c in set_cols.iter() { + let v = if c == "k" { + format!("'{}'", K_POOL.choose(&mut rng).unwrap()) + } else if rng.random_bool(0.2) { + "NULL".into() + } else { + rng.random_range(-5..=15).to_string() + }; + set_list.push(format!("{c} = {v}")); + } + on_conflict.push_str(&format!( + " ON CONFLICT{} DO UPDATE SET {}", + target, + set_list.join(", ") + )); + } else { + on_conflict.push_str(&format!(" ON CONFLICT{} DO NOTHING", target)); } - format!( - "INSERT INTO t({}) VALUES({}) ON CONFLICT DO UPDATE SET {}", - cols_list.join(","), - vals_list.join(","), - set_list.join(", ") - ) - } else { - format!( - "INSERT INTO t({}) VALUES({}) ON CONFLICT DO NOTHING", - cols_list.join(","), - vals_list.join(",") - ) } + format!( + "INSERT INTO t({}) VALUES({}) {}", + cols_list.join(","), + vals_list.join(","), + on_conflict + ) } _ => unreachable!(), }; From 78c05d8ce388d4e01caaf9470be01805a93a87e4 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 16:51:54 +0400 Subject: [PATCH 06/12] fix clippy --- tests/integration/fuzz/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index e5674d98f..1404c5e8f 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -960,7 +960,7 @@ mod tests { set_list.join(", ") )); } else { - on_conflict.push_str(&format!(" ON CONFLICT{} DO NOTHING", target)); + on_conflict.push_str(&format!(" ON CONFLICT{target} DO NOTHING")); } } format!( From c211fd1359214c31e3329ff5244bb72aa92890d7 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 17:05:39 +0400 Subject: [PATCH 07/12] handle btree-table search properly - btree-table doesn't have nulls in keys - so seek operation do some conversions and we shouldn't emit SeekGT { Null } in this case --- core/translate/main_loop.rs | 38 +++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 6b5107259..e283b7cef 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -1148,20 +1148,30 @@ fn emit_seek( // depending on the iteration direction. match seek_def.iter_dir { IterationDirection::Forwards => { - // the seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) - // so, NULL always must be filtered out - // (note, that complex filters like c < ? OR c IS NULL will produce Scan operation instead of Search) - program.emit_insn(Insn::Null { - dest: start_reg, - dest_end: None, - }); - program.emit_insn(Insn::SeekGT { - is_index, - cursor_id: seek_cursor_id, - start_reg, - num_regs: 1, - target_pc: loop_end, - }); + // table seek has some rules to convert seek key values to the integer affinity + it has some logic to check for explicit NULL searches + // so, we emit simple Rewind in case of search over table BTree + // (this is safe as table BTree keys are always non-null single integer) + if !is_index { + program.emit_insn(Insn::Rewind { + cursor_id: seek_cursor_id, + pc_if_empty: loop_end, + }); + } else { + // the seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) + // so, NULL always must be filtered out + // (note, that complex filters like c < ? OR c IS NULL will produce Scan operation instead of Search) + program.emit_insn(Insn::Null { + dest: start_reg, + dest_end: None, + }); + program.emit_insn(Insn::SeekGT { + is_index, + cursor_id: seek_cursor_id, + start_reg, + num_regs: 1, + target_pc: loop_end, + }); + } } IterationDirection::Backwards => { program.emit_insn(Insn::Last { From f1597dea903c82fb4e5d5d2a13fd6adeb66b541a Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 17:57:03 +0400 Subject: [PATCH 08/12] fix all combinations of iteration direction and index order to properly handle nulls --- core/translate/main_loop.rs | 240 +++++++++++++++++++++--------------- 1 file changed, 138 insertions(+), 102 deletions(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index e283b7cef..d64fd8afa 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -551,90 +551,89 @@ pub fn open_loop( ); // Open the loop for the index search. // Rowid equality point lookups are handled with a SeekRowid instruction which does not loop, since it is a single row lookup. - if let Search::RowidEq { cmp_expr } = search { - let src_reg = program.alloc_register(); - translate_expr( - program, - Some(table_references), - cmp_expr, - src_reg, - &t_ctx.resolver, - )?; - program.emit_insn(Insn::SeekRowid { - cursor_id: table_cursor_id - .expect("Search::RowidEq requires a table cursor"), - src_reg, - target_pc: next, - }); - } else { - // Otherwise, it's an index/rowid scan, i.e. first a seek is performed and then a scan until the comparison expression is not satisfied anymore. - if let Search::Seek { - index: Some(index), .. - } = search - { - if index.ephemeral { - let table_has_rowid = if let Table::BTree(btree) = &table.table { - btree.has_rowid - } else { - false - }; - Some(emit_autoindex( - program, - index, - table_cursor_id - .expect("an ephemeral index must have a source table cursor"), - index_cursor_id - .expect("an ephemeral index must have an index cursor"), - table_has_rowid, - )?) - } else { - index_cursor_id + match search { + Search::RowidEq { cmp_expr } => { + let src_reg = program.alloc_register(); + translate_expr( + program, + Some(table_references), + cmp_expr, + src_reg, + &t_ctx.resolver, + )?; + program.emit_insn(Insn::SeekRowid { + cursor_id: table_cursor_id + .expect("Search::RowidEq requires a table cursor"), + src_reg, + target_pc: next, + }); + } + Search::Seek { index, seek_def } => { + // Otherwise, it's an index/rowid scan, i.e. first a seek is performed and then a scan until the comparison expression is not satisfied anymore. + if let Some(index) = index { + if index.ephemeral { + let table_has_rowid = if let Table::BTree(btree) = &table.table { + btree.has_rowid + } else { + false + }; + Some(emit_autoindex( + program, + index, + table_cursor_id.expect( + "an ephemeral index must have a source table cursor", + ), + index_cursor_id + .expect("an ephemeral index must have an index cursor"), + table_has_rowid, + )?); + } } - } else { - index_cursor_id - }; - let is_index = index_cursor_id.is_some(); - let seek_cursor_id = temp_cursor_id.unwrap_or_else(|| { - index_cursor_id.unwrap_or_else(|| { - table_cursor_id - .expect("Either ephemeral or index or table cursor must be opened") - }) - }); - let Search::Seek { seek_def, .. } = search else { - unreachable!("Rowid equality point lookup should have been handled above"); - }; + let seek_cursor_id = temp_cursor_id.unwrap_or_else(|| { + index_cursor_id.unwrap_or_else(|| { + table_cursor_id.expect( + "Either ephemeral or index or table cursor must be opened", + ) + }) + }); + let Search::Seek { seek_def, .. } = search else { + unreachable!( + "Rowid equality point lookup should have been handled above" + ); + }; - let start_reg = program.alloc_registers(seek_def.key.len()); - emit_seek( - program, - table_references, - seek_def, - t_ctx, - seek_cursor_id, - start_reg, - loop_end, - is_index, - )?; - emit_seek_termination( - program, - table_references, - seek_def, - t_ctx, - seek_cursor_id, - start_reg, - loop_start, - loop_end, - is_index, - )?; + let start_reg = program.alloc_registers(seek_def.key.len()); + emit_seek( + program, + table_references, + seek_def, + t_ctx, + seek_cursor_id, + start_reg, + loop_end, + index.as_ref(), + )?; + emit_seek_termination( + program, + table_references, + seek_def, + t_ctx, + seek_cursor_id, + start_reg, + loop_start, + loop_end, + index.as_ref(), + )?; - if let Some(index_cursor_id) = index_cursor_id { - if let Some(table_cursor_id) = table_cursor_id { - // Don't do a btree table seek until it's actually necessary to read from the table. - program.emit_insn(Insn::DeferredSeek { - index_cursor_id, - table_cursor_id, - }); + if let Some(index_cursor_id) = index_cursor_id { + if let Some(table_cursor_id) = table_cursor_id { + // Don't do a btree table seek until it's actually necessary to read from the table. + program.emit_insn(Insn::DeferredSeek { + index_cursor_id, + table_cursor_id, + }); + } } } } @@ -1141,29 +1140,23 @@ fn emit_seek( seek_cursor_id: usize, start_reg: usize, loop_end: BranchOffset, - is_index: bool, + seek_index: Option<&Arc>, ) -> Result<()> { + let is_index = seek_index.is_some(); let Some(seek) = seek_def.seek.as_ref() else { // If there is no seek key, we start from the first or last row of the index, // depending on the iteration direction. + // + // Also, if we will encounter NULLs in the index at the beginning of iteration (Forward + Asc OR Backward + Desc) + // then, we must explicitly skip them as seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) + // + // note that table seek has some rules to convert seek key values to the integer affinity + it has some logic to check for explicit NULL searches + // so, we emit simple Rewind/Last in case of search over table BTree + // (this is safe as table BTree keys are always non-null single integer) match seek_def.iter_dir { IterationDirection::Forwards => { - // table seek has some rules to convert seek key values to the integer affinity + it has some logic to check for explicit NULL searches - // so, we emit simple Rewind in case of search over table BTree - // (this is safe as table BTree keys are always non-null single integer) - if !is_index { - program.emit_insn(Insn::Rewind { - cursor_id: seek_cursor_id, - pc_if_empty: loop_end, - }); - } else { - // the seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) - // so, NULL always must be filtered out - // (note, that complex filters like c < ? OR c IS NULL will produce Scan operation instead of Search) - program.emit_insn(Insn::Null { - dest: start_reg, - dest_end: None, - }); + if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Asc { + program.emit_null(start_reg, None); program.emit_insn(Insn::SeekGT { is_index, cursor_id: seek_cursor_id, @@ -1171,13 +1164,29 @@ fn emit_seek( num_regs: 1, target_pc: loop_end, }); + } else { + program.emit_insn(Insn::Rewind { + cursor_id: seek_cursor_id, + pc_if_empty: loop_end, + }); } } IterationDirection::Backwards => { - program.emit_insn(Insn::Last { - cursor_id: seek_cursor_id, - pc_if_empty: loop_end, - }); + if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Desc { + program.emit_null(start_reg, None); + program.emit_insn(Insn::SeekLT { + is_index, + cursor_id: seek_cursor_id, + start_reg, + num_regs: 1, + target_pc: loop_end, + }); + } else { + program.emit_insn(Insn::Last { + cursor_id: seek_cursor_id, + pc_if_empty: loop_end, + }); + } } } return Ok(()); @@ -1274,10 +1283,37 @@ fn emit_seek_termination( start_reg: usize, loop_start: BranchOffset, loop_end: BranchOffset, - is_index: bool, + seek_index: Option<&Arc>, ) -> Result<()> { + let is_index = seek_index.is_some(); let Some(termination) = seek_def.termination.as_ref() else { program.preassign_label_to_next_insn(loop_start); + // If we will encounter NULLs in the index at the end of iteration (Forward + Desc OR Backward + Asc) + // then, we must explicitly stop before them as seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) + match seek_def.iter_dir { + IterationDirection::Forwards => { + if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Desc { + program.emit_null(start_reg, None); + program.emit_insn(Insn::IdxGE { + cursor_id: seek_cursor_id, + start_reg, + num_regs: 1, + target_pc: loop_end, + }); + } + } + IterationDirection::Backwards => { + if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Asc { + program.emit_null(start_reg, None); + program.emit_insn(Insn::IdxLE { + cursor_id: seek_cursor_id, + start_reg, + num_regs: 1, + target_pc: loop_end, + }); + } + } + } return Ok(()); }; From e5aa836ad531ee3c59b097b31d6a2aa272f28546 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 17:57:25 +0400 Subject: [PATCH 09/12] add simple test --- testing/select.test | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/testing/select.test b/testing/select.test index cfa12ce17..5b35d3eda 100755 --- a/testing/select.test +++ b/testing/select.test @@ -787,3 +787,28 @@ do_execsql_test_on_specific_db {:memory:} rowid-references { AND `oid` = 2 AND [oid] = 2; } {2|2|2|2|2|2|2|2|2|2|2|2} + +do_execsql_test_on_specific_db {:memory:} null-in-search { + CREATE TABLE t_x_asc (id INTEGER PRIMARY KEY, x); + CREATE INDEX t_x_asc_idx ON t_x_asc(x ASC); + CREATE TABLE t_x_desc (id INTEGER PRIMARY KEY, x); + CREATE INDEX t_x_desc_idx ON t_x_desc(x DESC); + INSERT INTO t_x_asc VALUES (1, NULL), (2, 2), (10, 10); + INSERT INTO t_x_desc VALUES (1, NULL), (2, 2), (10, 10); + SELECT * FROM t_x_asc WHERE x > 5 ORDER BY x ASC; + SELECT * FROM t_x_asc WHERE x > 5 ORDER BY x DESC; + SELECT * FROM t_x_asc WHERE x < 5 ORDER BY x ASC; + SELECT * FROM t_x_asc WHERE x < 5 ORDER BY x DESC; + SELECT * FROM t_x_desc WHERE x > 5 ORDER BY x ASC; + SELECT * FROM t_x_desc WHERE x > 5 ORDER BY x DESC; + SELECT * FROM t_x_desc WHERE x < 5 ORDER BY x ASC; + SELECT * FROM t_x_desc WHERE x < 5 ORDER BY x DESC; +} {10|10 +10|10 +2|2 +2|2 +10|10 +10|10 +2|2 +2|2} + From 4a9309fe31346fb9934108239bf2f6aa04c5b5ec Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 17:58:12 +0400 Subject: [PATCH 10/12] fix clippy --- core/translate/main_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index d64fd8afa..5215cbdd3 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -568,7 +568,7 @@ pub fn open_loop( target_pc: next, }); } - Search::Seek { index, seek_def } => { + Search::Seek { index, .. } => { // Otherwise, it's an index/rowid scan, i.e. first a seek is performed and then a scan until the comparison expression is not satisfied anymore. if let Some(index) = index { if index.ephemeral { From bf5567de3560d48ff33319eeb672e6acdd41b2c3 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 18:06:42 +0400 Subject: [PATCH 11/12] fix clippy - the proper fix is to nuke it actually :) --- core/translate/main_loop.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 5215cbdd3..5e422442b 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -577,7 +577,7 @@ pub fn open_loop( } else { false }; - Some(emit_autoindex( + let _ = emit_autoindex( program, index, table_cursor_id.expect( @@ -586,7 +586,7 @@ pub fn open_loop( index_cursor_id .expect("an ephemeral index must have an index cursor"), table_has_rowid, - )?); + )?; } } From c84486c4118450ffae329acfaa8213653b9f84c3 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 30 Sep 2025 18:45:00 +0400 Subject: [PATCH 12/12] clippy logged in as jussi - so I need to fix more stuff --- core/translate/main_loop.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 5e422442b..ca384b04f 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -1155,7 +1155,7 @@ fn emit_seek( // (this is safe as table BTree keys are always non-null single integer) match seek_def.iter_dir { IterationDirection::Forwards => { - if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Asc { + if seek_index.is_some_and(|index| index.columns[0].order == SortOrder::Asc) { program.emit_null(start_reg, None); program.emit_insn(Insn::SeekGT { is_index, @@ -1172,7 +1172,7 @@ fn emit_seek( } } IterationDirection::Backwards => { - if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Desc { + if seek_index.is_some_and(|index| index.columns[0].order == SortOrder::Desc) { program.emit_null(start_reg, None); program.emit_insn(Insn::SeekLT { is_index, @@ -1292,7 +1292,7 @@ fn emit_seek_termination( // then, we must explicitly stop before them as seek always has some bound condition over indexed column (e.g. c < ?, c >= ?, ...) match seek_def.iter_dir { IterationDirection::Forwards => { - if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Desc { + if seek_index.is_some_and(|index| index.columns[0].order == SortOrder::Desc) { program.emit_null(start_reg, None); program.emit_insn(Insn::IdxGE { cursor_id: seek_cursor_id, @@ -1303,7 +1303,7 @@ fn emit_seek_termination( } } IterationDirection::Backwards => { - if seek_index.is_some() && seek_index.unwrap().columns[0].order == SortOrder::Asc { + if seek_index.is_some_and(|index| index.columns[0].order == SortOrder::Asc) { program.emit_null(start_reg, None); program.emit_insn(Insn::IdxLE { cursor_id: seek_cursor_id,