aljaz/turso
1
0
Fork 0
mirror of https://github.com/aljazceru/turso.git synced 2026-01-31 13:54:27 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge 'Fix affinity handling in MakeRecord' from Pekka Enberg

Browse Source 
Closes #2966
This commit is contained in:
Preston Thorpe
2025-09-08 12:14:33 -04:00
committed by GitHub
parent 71a812ce55 86baa06600
commit 8d05336522
19 changed files with 228 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
core/translate/aggregation.rs
View File

@@ -115,6 +115,7 @@ pub fn handle_distinct(program: &mut ProgramBuilder, agg: &Aggregate, agg_arg_re
count: num_regs,
dest_reg: record_reg,
index_name: Some(distinct_ctx.ephemeral_index_name.to_string()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: distinct_ctx.cursor_id,

9
core/translate/alter.rs
View File

@@ -141,11 +141,18 @@ pub fn translate_alter_table(
let record = program.alloc_register();
let affinity_str = btree
.columns
.iter()
.map(|col| col.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: first_column,
count: column_count,
dest_reg: record,
index_name: None,
affinity_str: Some(affinity_str),
});
program.emit_insn(Insn::Insert {
@@ -301,6 +308,7 @@ pub fn translate_alter_table(
count: sqlite_schema_column_len,
dest_reg: record,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::Insert {
@@ -442,6 +450,7 @@ pub fn translate_alter_table(
count: sqlite_schema_column_len,
dest_reg: record,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::Insert {

1
core/translate/analyze.rs
View File

@@ -212,6 +212,7 @@ pub fn translate_analyze(
count: 3,
dest_reg: record_reg,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::NewRowid {
cursor: stat_cursor,

1
core/translate/compound_select.rs
View File

@@ -524,6 +524,7 @@ fn read_intersect_rows(
count: column_count,
dest_reg: row_content_reg,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: target_cursor_id,

24
core/translate/emitter.rs
View File

@@ -1038,6 +1038,7 @@ fn emit_update_insns(
count: num_cols + 1,
dest_reg: *record_reg,
index_name: Some(index.name.clone()),
affinity_str: None,
});
if !index.unique {
@@ -1138,11 +1139,19 @@ fn emit_update_insns(
}
let record_reg = program.alloc_register();
let affinity_str = table_ref
.columns()
.iter()
.map(|col| col.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: start,
count: table_ref.columns().len(),
dest_reg: record_reg,
index_name: None,
affinity_str: Some(affinity_str),
});
if has_user_provided_rowid {
@@ -1282,6 +1291,7 @@ fn emit_update_insns(
count: 2 * table_ref.columns().len(),
dest_reg: record_reg,
index_name: None,
affinity_str: None,
});
Some(record_reg)
} else {
@@ -1398,11 +1408,18 @@ pub fn emit_cdc_patch_record(
dst_reg: columns_reg + rowid_alias_position,
extra_amount: 0,
});
let affinity_str = table
.columns()
.iter()
.map(|col| col.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: columns_reg,
count: table.columns().len(),
dest_reg: record_reg,
index_name: None,
affinity_str: Some(affinity_str),
});
record_reg
} else {
@@ -1428,11 +1445,17 @@ pub fn emit_cdc_full_record(
program.emit_column_or_rowid(table_cursor_id, i, columns_reg + 1 + i);
}
}
let affinity_str = columns
.iter()
.map(|col| col.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: columns_reg + 1,
count: columns.len(),
dest_reg: columns_reg,
index_name: None,
affinity_str: Some(affinity_str),
});
columns_reg
}
@@ -1535,6 +1558,7 @@ pub fn emit_cdc_insns(
count: 8,
dest_reg: record_reg,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::Insert {

1
core/translate/index.rs
View File

@@ -174,6 +174,7 @@ pub fn translate_create_index(
count: columns.len() + 1,
dest_reg: record_reg,
index_name: Some(idx_name.clone()),
affinity_str: None,
});
program.emit_insn(Insn::SorterInsert {
cursor_id: sorter_cursor_id,

32
core/translate/insert.rs
View File

@@ -253,11 +253,35 @@ pub fn translate_insert(
end_offset: yield_label,
});
let record_reg = program.alloc_register();
let affinity_str = if columns.is_empty() {
btree_table
.columns
.iter()
.filter(|col| !col.hidden)
.map(|col| col.affinity().aff_mask())
.collect::<String>()
} else {
columns
.iter()
.map(|col_name| {
let column_name = normalize_ident(col_name.as_str());
table
.get_column_by_name(&column_name)
.unwrap()
.1
.affinity()
.aff_mask()
})
.collect::<String>()
};
program.emit_insn(Insn::MakeRecord {
start_reg: yield_reg + 1,
count: result.num_result_cols,
dest_reg: record_reg,
index_name: None,
affinity_str: Some(affinity_str),
});
let rowid_reg = program.alloc_register();
@@ -507,6 +531,7 @@ pub fn translate_insert(
count: num_cols + 1,
dest_reg: record_reg,
index_name: Some(index.name.clone()),
affinity_str: None,
});
if index.unique {
@@ -627,11 +652,18 @@ pub fn translate_insert(
});
}
// Create and insert the record
let affinity_str = insertion
.col_mappings
.iter()
.map(|col_mapping| col_mapping.column.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: insertion.first_col_register(),
count: insertion.col_mappings.len(),
dest_reg: insertion.record_register(),
index_name: None,
affinity_str: Some(affinity_str),
});
program.emit_insn(Insn::Insert {
cursor: cursor_id,

1
core/translate/main_loop.rs
View File

@@ -1429,6 +1429,7 @@ fn emit_autoindex(
count: num_regs_to_reserve,
dest_reg: record_reg,
index_name: Some(index.name.clone()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: index_cursor_id,

1
core/translate/order_by.rs
View File

@@ -307,6 +307,7 @@ pub fn sorter_insert(
count: column_count,
dest_reg: record_reg,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::SorterInsert {
cursor_id,

1
core/translate/plan.rs
View File

@@ -264,6 +264,7 @@ impl DistinctCtx {
count: num_regs,
dest_reg: record_reg,
index_name: Some(self.ephemeral_index_name.to_string()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: self.cursor_id,

2
core/translate/result_row.rs
View File

@@ -103,6 +103,7 @@ pub fn emit_result_row_and_limit(
count: plan.result_columns.len(),
dest_reg: record_reg,
index_name: Some(dedupe_index.name.clone()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: *index_cursor_id,
@@ -124,6 +125,7 @@ pub fn emit_result_row_and_limit(
count: plan.result_columns.len() - 1,
dest_reg: record_reg,
index_name: Some(table.name.clone()),
affinity_str: None,
});
}
program.emit_insn(Insn::Insert {

3
core/translate/schema.rs
View File

@@ -257,6 +257,7 @@ pub fn emit_schema_entry(
count: 5,
dest_reg: record_reg,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::Insert {
@@ -614,6 +615,7 @@ pub fn translate_create_virtual_table(
count: args_vec.len(),
dest_reg: args_record_reg,
index_name: None,
affinity_str: None,
});
Some(args_record_reg)
} else {
@@ -998,6 +1000,7 @@ pub fn translate_drop_table(
count: 5,
dest_reg: new_record_register,
index_name: None,
affinity_str: None,
});
program.emit_insn(Insn::Delete {
cursor_id: sqlite_schema_cursor_id_1,

9
core/translate/upsert.rs
View File

@@ -379,6 +379,7 @@ pub fn emit_upsert(
count: k + 1,
dest_reg: rec,
index_name: Some((*idx_name).clone()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: *idx_cid,
@@ -392,11 +393,19 @@ pub fn emit_upsert(
// Write table row (same rowid, new payload)
let rec = program.alloc_register();
let affinity_str = table
.columns()
.iter()
.map(|col| col.affinity().aff_mask())
.collect::<String>();
program.emit_insn(Insn::MakeRecord {
start_reg: new_start,
count: num_cols,
dest_reg: rec,
index_name: None,
affinity_str: Some(affinity_str),
});
program.emit_insn(Insn::Insert {
cursor: tbl_cursor_id,

1
core/translate/values.rs
View File

@@ -199,6 +199,7 @@ fn emit_values_to_index(
count: row_len,
dest_reg: record_reg,
index_name: Some(index.name.clone()),
affinity_str: None,
});
program.emit_insn(Insn::IdxInsert {
cursor_id: *cursor_id,

27
core/vdbe/execute.rs
View File

@@ -1864,10 +1864,27 @@ pub fn op_make_record(
start_reg,
count,
dest_reg,
affinity_str,
..
},
insn
);
if let Some(affinity_str) = affinity_str {
if affinity_str.len() != *count {
return Err(LimboError::InternalError(format!(
"MakeRecord: the length of affinity string ({}) does not match the count ({})",
affinity_str.len(),
*count
)));
}
for (i, affinity_ch) in affinity_str.chars().enumerate().take(*count) {
let reg_index = *start_reg + i;
let affinity = Affinity::from_char(affinity_ch);
apply_affinity_char(&mut state.registers[reg_index], affinity);
}
}
let record = make_record(&state.registers, start_reg, count);
state.registers[*dest_reg] = Register::Record(record);
state.pc += 1;
@@ -8502,15 +8519,19 @@ fn apply_affinity_char(target: &mut Register, affinity: Affinity) -> bool {
}
if let Value::Text(t) = value {
let text = t.as_str();
let text = t.as_str().trim();
// Handle hex numbers - they shouldn't be converted
if text.starts_with("0x") {
return false;
}
// Try to parse as number (similar to applyNumericAffinity)
let Ok(num) = checked_cast_text_to_numeric(text) else {
// For affinity conversion, only convert strings that are entirely numeric
let num = if let Ok(i) = text.parse::<i64>() {
Value::Integer(i)
} else if let Ok(f) = text.parse::<f64>() {
Value::Float(f)
} else {
return false;
};

1
core/vdbe/explain.rs
View File

@@ -584,6 +584,7 @@ pub fn insn_to_str(
count,
dest_reg,
index_name,
affinity_str: _,
} => {
let for_index = index_name.as_ref().map(|name| format!("; for {name}"));
(

1
core/vdbe/insn.rs
View File

@@ -430,6 +430,7 @@ pub enum Insn {
count: usize, // P2
dest_reg: usize, // P3
index_name: Option<String>,
affinity_str: Option<String>,
},
/// Emit a row of results.

14
testing/affinity.test Executable file
View File

@@ -0,0 +1,14 @@
#!/usr/bin/env tclsh
set testdir [file dirname $argv0]
source $testdir/tester.tcl
do_execsql_test_on_specific_db {:memory:} affinity {
CREATE TABLE t1 (c INTEGER);
INSERT INTO t1 VALUES ('1');
INSERT INTO t1 VALUES ('1a');
SELECT c, typeof(c) FROM t1;
} {
{1|integer}
{1a|text}
}

104
tests/integration/fuzz/mod.rs
View File

@@ -1381,9 +1381,6 @@ mod tests {
}
#[test]
#[ignore]
/// Ignored because of https://github.com/tursodatabase/turso/issues/2040, https://github.com/tursodatabase/turso/issues/2041
/// TODO: add fuzzing for other aggregate functions
pub fn min_max_agg_fuzz() {
let _ = env_logger::try_init();
@@ -1438,6 +1435,107 @@ mod tests {
}
}
}
#[test]
pub fn affinity_fuzz() {
let _ = env_logger::try_init();
let (mut rng, seed) = rng_from_time();
log::info!("affinity_fuzz seed: {seed}");
for iteration in 0..500 {
let db = TempDatabase::new_empty(false);
let limbo_conn = db.connect_limbo();
let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap();
// Test different column affinities - cover all SQLite affinity types
let affinities = [
"INTEGER",
"TEXT",
"REAL",
"NUMERIC",
"BLOB",
"INT",
"TINYINT",
"SMALLINT",
"MEDIUMINT",
"BIGINT",
"UNSIGNED BIG INT",
"INT2",
"INT8",
"CHARACTER(20)",
"VARCHAR(255)",
"VARYING CHARACTER(255)",
"NCHAR(55)",
"NATIVE CHARACTER(70)",
"NVARCHAR(100)",
"CLOB",
"DOUBLE",
"DOUBLE PRECISION",
"FLOAT",
"DECIMAL(10,5)",
"BOOLEAN",
"DATE",
"DATETIME",
];
let affinity = affinities[rng.random_range(0..affinities.len())];
let create_table = format!("CREATE TABLE t (x {affinity})");
limbo_exec_rows(&db, &limbo_conn, &create_table);
sqlite_exec_rows(&sqlite_conn, &create_table);
// Insert various values that test affinity conversion rules
let mut values = Vec::new();
for _ in 0..20 {
let value = match rng.random_range(0..9) {
0 => format!("'{}'", rng.random_range(-10000..10000)), // Pure integer as text
1 => format!(
"'{}.{}'",
rng.random_range(-1000..1000),
rng.random_range(1..999) // Ensure non-zero decimal part
), // Float as text with decimal
2 => format!("'a{}'", rng.random_range(0..1000)), // Text with integer suffix
3 => format!("' {} '", rng.random_range(-100..100)), // Integer with whitespace
4 => format!("'-{}'", rng.random_range(1..1000)), // Negative integer as text
5 => format!("{}", rng.random_range(-10000..10000)), // Direct integer
6 => format!(
"{}.{}",
rng.random_range(-100..100),
rng.random_range(1..999) // Ensure non-zero decimal part
), // Direct float
7 => "'text_value'".to_string(), // Pure text that won't convert
8 => "NULL".to_string(), // NULL value
_ => unreachable!(),
};
values.push(format!("({value})"));
}
let insert = format!("INSERT INTO t VALUES {}", values.join(","));
limbo_exec_rows(&db, &limbo_conn, &insert);
sqlite_exec_rows(&sqlite_conn, &insert);
// Query values and their types to verify affinity rules are applied correctly
let query = "SELECT x, typeof(x) FROM t";
let limbo_result = limbo_exec_rows(&db, &limbo_conn, query);
let sqlite_result = sqlite_exec_rows(&sqlite_conn, query);
assert_eq!(
limbo_result, sqlite_result,
"iteration: {iteration}, seed: {seed}, affinity: {affinity}, values: {values:?}"
);
// Also test with ORDER BY to ensure affinity affects sorting
let query_ordered = "SELECT x FROM t ORDER BY x";
let limbo_ordered = limbo_exec_rows(&db, &limbo_conn, query_ordered);
let sqlite_ordered = sqlite_exec_rows(&sqlite_conn, query_ordered);
assert_eq!(
limbo_ordered, sqlite_ordered,
"ORDER BY failed - iteration: {iteration}, seed: {seed}, affinity: {affinity}"
);
}
}
#[test]
// Simple fuzz test for SUM with floats
pub fn sum_agg_fuzz_floats() {