Fix: always emit rowid when column is rowid alias

SQLite does not store the rowid alias column in the record at all
when it is a rowid alias, because the rowid is always stored anyway
in the record header.
This commit is contained in:
Jussi Saurio
2025-08-21 13:20:04 +03:00
parent f9ad43a3a3
commit dd2e0ea596
11 changed files with 53 additions and 29 deletions

View File

@@ -125,7 +125,7 @@ pub fn translate_alter_table(
continue;
}
program.emit_column(cursor_id, i, iter);
program.emit_column_or_rowid(cursor_id, i, iter);
iter += 1;
}
@@ -264,7 +264,7 @@ pub fn translate_alter_table(
let first_column = program.alloc_registers(sqlite_schema_column_len);
for i in 0..sqlite_schema_column_len {
program.emit_column(cursor_id, i, first_column + i);
program.emit_column_or_rowid(cursor_id, i, first_column + i);
}
program.emit_string8_new_reg(table_name.to_string());
@@ -354,7 +354,7 @@ pub fn translate_alter_table(
let first_column = program.alloc_registers(sqlite_schema_column_len);
for i in 0..sqlite_schema_column_len {
program.emit_column(cursor_id, i, first_column + i);
program.emit_column_or_rowid(cursor_id, i, first_column + i);
}
program.emit_string8_new_reg(table_name.to_string());

View File

@@ -551,7 +551,7 @@ fn emit_delete_insns(
.iter()
.enumerate()
.for_each(|(reg_offset, column_index)| {
program.emit_column(
program.emit_column_or_rowid(
main_table_cursor_id,
column_index.pos_in_table,
start_reg + reg_offset,
@@ -615,7 +615,7 @@ fn emit_delete_insns(
// Read all column values from the row to be deleted
for (i, _column) in table_reference.columns().iter().enumerate() {
program.emit_column(main_table_cursor_id, i, columns_start_reg + i);
program.emit_column_or_rowid(main_table_cursor_id, i, columns_start_reg + i);
}
// Emit RETURNING results using the values we just read
@@ -967,7 +967,11 @@ fn emit_update_insns(
}
})
.unwrap_or(&cursor_id);
program.emit_column(cursor_id, column_idx_in_index.unwrap_or(idx), target_reg);
program.emit_column_or_rowid(
cursor_id,
column_idx_in_index.unwrap_or(idx),
target_reg,
);
}
if let Some(cdc_updates_register) = cdc_updates_register {
@@ -1155,7 +1159,7 @@ fn emit_update_insns(
.iter()
.enumerate()
.for_each(|(reg_offset, column_index)| {
program.emit_column(
program.emit_column_or_rowid(
cursor_id,
column_index.pos_in_table,
start_reg + reg_offset,
@@ -1415,7 +1419,7 @@ pub fn emit_cdc_full_record(
extra_amount: 0,
});
} else {
program.emit_column(table_cursor_id, i, columns_reg + 1 + i);
program.emit_column_or_rowid(table_cursor_id, i, columns_reg + 1 + i);
}
}
program.emit_insn(Insn::MakeRecord {

View File

@@ -2012,7 +2012,7 @@ pub fn translate_expr(
*column
};
program.emit_column(read_cursor, column, target_register);
program.emit_column_or_rowid(read_cursor, column, target_register);
}
let Some(column) = table.get_column_at(*column) else {
crate::bail_parse_error!("column index out of bounds");

View File

@@ -479,7 +479,7 @@ impl<'a> GroupByAggArgumentSource<'a> {
dest_reg_start,
..
} => {
program.emit_column(*cursor_id, *col_start, dest_reg_start + arg_idx);
program.emit_column_or_rowid(*cursor_id, *col_start, dest_reg_start + arg_idx);
Ok(dest_reg_start + arg_idx)
}
GroupByAggArgumentSource::Register {
@@ -525,7 +525,7 @@ pub fn group_by_process_single_group(
for i in 0..group_by.exprs.len() {
let sorter_column_index = i;
let group_reg = groups_start_reg + i;
program.emit_column(*pseudo_cursor, sorter_column_index, group_reg);
program.emit_column_or_rowid(*pseudo_cursor, sorter_column_index, group_reg);
}
groups_start_reg
}
@@ -648,7 +648,7 @@ pub fn group_by_process_single_group(
t_ctx.non_aggregate_expressions.iter().enumerate()
{
if *in_result {
program.emit_column(*pseudo_cursor, sorter_column_index, next_reg);
program.emit_column_or_rowid(*pseudo_cursor, sorter_column_index, next_reg);
t_ctx.resolver.expr_to_reg_cache.push((expr, next_reg));
next_reg += 1;
}

View File

@@ -157,7 +157,7 @@ pub fn translate_create_index(
// Then insert the record into the sorter
let start_reg = program.alloc_registers(columns.len() + 1);
for (i, (col, _)) in columns.iter().enumerate() {
program.emit_column(table_cursor_id, col.0, start_reg + i);
program.emit_column_or_rowid(table_cursor_id, col.0, start_reg + i);
}
let rowid_reg = start_reg + columns.len();
program.emit_insn(Insn::RowId {
@@ -400,7 +400,7 @@ pub fn translate_drop_index(
// Read sqlite_schema.name into dest_reg
let dest_reg = program.alloc_register();
program.emit_column(sqlite_schema_cursor_id, 1, dest_reg);
program.emit_column_or_rowid(sqlite_schema_cursor_id, 1, dest_reg);
// if current column is not index_name then jump to Next
// skip if sqlite_schema.name != index_name_reg
@@ -415,7 +415,7 @@ pub fn translate_drop_index(
// read type of table
// skip if sqlite_schema.type != 'index' (index_str_reg)
program.emit_column(sqlite_schema_cursor_id, 0, dest_reg);
program.emit_column_or_rowid(sqlite_schema_cursor_id, 0, dest_reg);
// if current column is not index then jump to Next
program.emit_insn(Insn::Ne {
lhs: index_str_reg,

View File

@@ -830,7 +830,7 @@ fn populate_columns_multiple_rows(
column_register
};
if let Some(temp_table_ctx) = temp_table_ctx {
program.emit_column(temp_table_ctx.cursor_id, value_index, write_reg);
program.emit_column_or_rowid(temp_table_ctx.cursor_id, value_index, write_reg);
} else {
program.emit_insn(Insn::Copy {
src_reg: yield_reg + value_index,

View File

@@ -1404,7 +1404,7 @@ fn emit_autoindex(
let ephemeral_cols_start_reg = program.alloc_registers(num_regs_to_reserve);
for (i, col) in index.columns.iter().enumerate() {
let reg = ephemeral_cols_start_reg + i;
program.emit_column(table_cursor_id, col.pos_in_table, reg);
program.emit_column_or_rowid(table_cursor_id, col.pos_in_table, reg);
}
if table_has_rowid {
program.emit_insn(Insn::RowId {

View File

@@ -133,7 +133,7 @@ pub fn emit_order_by(
.get(i)
.expect("remapping must exist for all result columns")
.orderby_sorter_idx;
program.emit_column(cursor_id, column_idx, reg);
program.emit_column_or_rowid(cursor_id, column_idx, reg);
}
emit_result_row_and_limit(

View File

@@ -713,7 +713,7 @@ pub fn translate_drop_table(
program.preassign_label_to_next_insn(metadata_loop);
// start loop on schema table
program.emit_column(
program.emit_column_or_rowid(
sqlite_schema_cursor_id_0,
2,
table_name_and_root_page_register,
@@ -726,7 +726,7 @@ pub fn translate_drop_table(
flags: CmpInsFlags::default(),
collation: program.curr_collation(),
});
program.emit_column(
program.emit_column_or_rowid(
sqlite_schema_cursor_id_0,
0,
table_name_and_root_page_register,
@@ -749,7 +749,7 @@ pub fn translate_drop_table(
let skip_cdc_label = program.allocate_label();
let entry_type_reg = program.alloc_register();
program.emit_column(sqlite_schema_cursor_id_0, 0, entry_type_reg);
program.emit_column_or_rowid(sqlite_schema_cursor_id_0, 0, entry_type_reg);
program.emit_insn(Insn::Ne {
lhs: entry_type_reg,
rhs: table_type,
@@ -904,7 +904,7 @@ pub fn translate_drop_table(
});
program.preassign_label_to_next_insn(copy_schema_to_temp_table_loop);
// start loop on schema table
program.emit_column(sqlite_schema_cursor_id_1, 3, prev_root_page_register);
program.emit_column_or_rowid(sqlite_schema_cursor_id_1, 3, prev_root_page_register);
// The label and Insn::Ne are used to skip over any rows in the schema table that don't have the root page that was moved
let next_label = program.allocate_label();
program.emit_insn(Insn::Ne {
@@ -963,9 +963,9 @@ pub fn translate_drop_table(
rowid_reg: schema_row_id_register,
target_pc: next_label,
});
program.emit_column(sqlite_schema_cursor_id_1, 0, schema_column_0_register);
program.emit_column(sqlite_schema_cursor_id_1, 1, schema_column_1_register);
program.emit_column(sqlite_schema_cursor_id_1, 2, schema_column_2_register);
program.emit_column_or_rowid(sqlite_schema_cursor_id_1, 0, schema_column_0_register);
program.emit_column_or_rowid(sqlite_schema_cursor_id_1, 1, schema_column_1_register);
program.emit_column_or_rowid(sqlite_schema_cursor_id_1, 2, schema_column_2_register);
let root_page = table
.get_root_page()
.try_into()
@@ -974,7 +974,7 @@ pub fn translate_drop_table(
value: root_page,
dest: moved_to_root_page_register,
});
program.emit_column(sqlite_schema_cursor_id_1, 4, schema_column_4_register);
program.emit_column_or_rowid(sqlite_schema_cursor_id_1, 4, schema_column_4_register);
program.emit_insn(Insn::MakeRecord {
start_reg: schema_column_0_register,
count: 5,

View File

@@ -220,8 +220,8 @@ pub fn translate_drop_view(
let col0_reg = program.alloc_register();
let col1_reg = program.alloc_register();
program.emit_column(sqlite_schema_cursor_id, 0, col0_reg);
program.emit_column(sqlite_schema_cursor_id, 1, col1_reg);
program.emit_column_or_rowid(sqlite_schema_cursor_id, 0, col0_reg);
program.emit_column_or_rowid(sqlite_schema_cursor_id, 1, col1_reg);
// Check if type == 'view' and name == view_name
let skip_delete_label = program.allocate_label();

View File

@@ -836,7 +836,27 @@ impl ProgramBuilder {
self.preassign_label_to_next_insn(loop_end);
}
pub fn emit_column(&mut self, cursor_id: CursorID, column: usize, out: usize) {
pub fn emit_column_or_rowid(&mut self, cursor_id: CursorID, column: usize, out: usize) {
let (_, cursor_type) = self.cursor_ref.get(cursor_id).unwrap();
if let CursorType::BTreeTable(btree) = cursor_type {
let column_def = btree
.columns
.get(column)
.expect("column index out of bounds");
if column_def.is_rowid_alias {
self.emit_insn(Insn::RowId {
cursor_id,
dest: out,
});
} else {
self.emit_column(cursor_id, column, out);
}
} else {
self.emit_column(cursor_id, column, out);
}
}
fn emit_column(&mut self, cursor_id: CursorID, column: usize, out: usize) {
let (_, cursor_type) = self.cursor_ref.get(cursor_id).unwrap();
use crate::translate::expr::sanitize_string;