Skip to content

Commit

Permalink
Fix IdxGt,IdxGe,IdxLt,IdxLe instructions
Browse files Browse the repository at this point in the history
According to SQLite documentation, the way to use these instructions
is to compare the seek key to the index key as you would with the
Compare opcode. The compare opcode states:

"Compare two vectors of registers in reg(P1)..reg(P1+P3-1)
(call this vector "A") and in reg(P2)..reg(P2+P3-1) ("B")."

In other words, we should compare the same number of columns from each,
not compare the entire keys.

This fixes a few Clickbench queries returning incorrect results, and
so closes #1009

---

Future work: support index seek keys that use multiple columns. Our
index seek is many times slower than SQLite because we're not utilizing
all the possible columns -- instead we just use the first index column
to seek.
  • Loading branch information
jussisaurio committed Feb 15, 2025
1 parent f220f9a commit e4541ed
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions core/vdbe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1399,8 +1399,8 @@ impl Program {
let record_from_regs: Record =
make_owned_record(&state.registers, start_reg, num_regs);
if let Some(ref idx_record) = *cursor.record()? {
// omit the rowid from the idx_record, which is the last value
if idx_record.get_values()[..idx_record.len() - 1]
// Compare against the same number of values
if idx_record.get_values()[..record_from_regs.len()]
>= record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
Expand All @@ -1423,8 +1423,8 @@ impl Program {
let record_from_regs: Record =
make_owned_record(&state.registers, start_reg, num_regs);
if let Some(ref idx_record) = *cursor.record()? {
// omit the rowid from the idx_record, which is the last value
if idx_record.get_values()[..idx_record.len() - 1]
// Compare against the same number of values
if idx_record.get_values()[..record_from_regs.len()]
<= record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
Expand All @@ -1447,8 +1447,8 @@ impl Program {
let record_from_regs: Record =
make_owned_record(&state.registers, start_reg, num_regs);
if let Some(ref idx_record) = *cursor.record()? {
// omit the rowid from the idx_record, which is the last value
if idx_record.get_values()[..idx_record.len() - 1]
// Compare against the same number of values
if idx_record.get_values()[..record_from_regs.len()]
> record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
Expand All @@ -1471,8 +1471,8 @@ impl Program {
let record_from_regs: Record =
make_owned_record(&state.registers, start_reg, num_regs);
if let Some(ref idx_record) = *cursor.record()? {
// omit the rowid from the idx_record, which is the last value
if idx_record.get_values()[..idx_record.len() - 1]
// Compare against the same number of values
if idx_record.get_values()[..record_from_regs.len()]
< record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
Expand Down

0 comments on commit e4541ed

Please sign in to comment.