mirrors/limbo
1
0
Fork 0
mirror of https://github.com/tursodatabase/limbo.git synced 2025-08-04 10:08:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 12

Merge 'Make record values private' from Tiago

Browse source 
This is an attempt to move towards #881. I am not sure this is the
direction you want to take. In any case, I thought I would take a crack
at converting `values` from `Record` to private and see how bad it would
be.
In the end, as you can see, it is not so bad. I think performance-wise
it shouldn't be a bad hit with Rust's zero-cost abstraction. Also,
during the process I noticed a couple improvements that could be made
here and there but I honestly wanted to start with something small
enough that wouldn't be too hard to review.
Anyway, let me know if this is really how you would like to proceed.

Closes #962
This commit is contained in:
Pekka Enberg 2025-02-10 11:23:21 +02:00
commit 604ca4085d
17 changed files with 77 additions and 62 deletions
Download patch file Download diff file Expand all files Collapse all files

2
bindings/go/rs_src/rows.rs
View file

@ -75,7 +75,7 @@ pub extern "C" fn rows_get_value(ctx: *mut c_void, col_idx: usize) -> *const c_v
let ctx = LimboRows::from_ptr(ctx);
if let Some(row) = ctx.stmt.row() {
if let Some(value) = row.values.get(col_idx) {
if let Some(value) = row.get_values().get(col_idx) {
let value = value.to_value();
return LimboValue::from_value(&value).to_ptr();
}

5
bindings/java/rs_src/limbo_statement.rs
View file

@ -102,10 +102,9 @@ fn row_to_obj_array<'local>(
env: &mut JNIEnv<'local>,
row: &limbo_core::Row,
) -> Result<JObject<'local>> {
let obj_array =
env.new_object_array(row.values.len() as i32, "java/lang/Object", JObject::null())?;
let obj_array = env.new_object_array(row.len() as i32, "java/lang/Object", JObject::null())?;
for (i, value) in row.values.iter().enumerate() {
for (i, value) in row.get_values().iter().enumerate() {
let value = value.to_value();
let obj = match value {
limbo_core::Value::Null => JObject::null(),

2
bindings/python/src/lib.rs
View file

@ -300,7 +300,7 @@ pub fn connect(path: &str) -> Result<Connection> {
fn row_to_py(py: Python, row: &limbo_core::Row) -> PyObject {
let py_values: Vec<PyObject> = row
.values
.get_values()
.iter()
.map(|value| match value.to_value() {
limbo_core::Value::Null => py.None(),

6
bindings/wasm/lib.rs
View file

@ -76,7 +76,7 @@ impl RowIterator {
Ok(limbo_core::StepResult::Row) => {
let row = stmt.row().unwrap();
let row_array = Array::new();
for value in &row.values {
for value in row.get_values() {
let value = value.to_value();
let value = to_js_value(value);
row_array.push(&value);
@ -117,7 +117,7 @@ impl Statement {
Ok(limbo_core::StepResult::Row) => {
let row = stmt.row().unwrap();
let row_array = js_sys::Array::new();
for value in &row.values {
for value in row.get_values() {
let value = value.to_value();
let value = to_js_value(value);
row_array.push(&value);
@ -140,7 +140,7 @@ impl Statement {
Ok(limbo_core::StepResult::Row) => {
let row = stmt.row().unwrap();
let row_array = js_sys::Array::new();
for value in &row.values {
for value in row.get_values() {
let value = value.to_value();
let value = to_js_value(value);
row_array.push(&value);

8
cli/app.rs
View file

@ -627,7 +627,7 @@ impl Limbo {
match rows.step() {
Ok(StepResult::Row) => {
let row = rows.row().unwrap();
for (i, value) in row.values.iter().enumerate() {
for (i, value) in row.get_values().iter().enumerate() {
let value = value.to_value();
if i > 0 {
let _ = self.writer.write(b"|");
@ -689,7 +689,7 @@ impl Limbo {
let record = rows.row().unwrap();
let mut row = Row::new();
row.max_height(1);
for value in &record.values {
for value in record.get_values() {
let (content, alignment) = match value.to_value() {
Value::Null => {
(self.opts.null_value.clone(), CellAlignment::Left)
@ -762,7 +762,7 @@ impl Limbo {
StepResult::Row => {
let row = rows.row().unwrap();
if let Some(Value::Text(schema)) =
row.values.first().map(|v| v.to_value())
row.get_values().first().map(|v| v.to_value())
{
let _ = self.write_fmt(format_args!("{};", schema));
found = true;
@ -822,7 +822,7 @@ impl Limbo {
StepResult::Row => {
let row = rows.row().unwrap();
if let Some(Value::Text(table)) =
row.values.first().map(|v| v.to_value())
row.get_values().first().map(|v| v.to_value())
{
tables.push_str(table);
tables.push(' ');

18
core/storage/btree.rs
View file

@ -381,7 +381,7 @@ impl BTreeCursor {
let record = crate::storage::sqlite3_ondisk::read_record(payload)?;
if predicate.is_none() {
let rowid = match record.values.last() {
let rowid = match record.last_value() {
Some(OwnedValue::Integer(rowid)) => *rowid as u64,
_ => unreachable!("index cells should have an integer rowid"),
};
@ -398,7 +398,7 @@ impl BTreeCursor {
SeekOp::EQ => &record == *index_key,
};
if found {
let rowid = match record.values.last() {
let rowid = match record.last_value() {
Some(OwnedValue::Integer(rowid)) => *rowid as u64,
_ => unreachable!("index cells should have an integer rowid"),
};
@ -411,7 +411,7 @@ impl BTreeCursor {
self.stack.advance();
let record = crate::storage::sqlite3_ondisk::read_record(payload)?;
if predicate.is_none() {
let rowid = match record.values.last() {
let rowid = match record.last_value() {
Some(OwnedValue::Integer(rowid)) => *rowid as u64,
_ => unreachable!("index cells should have an integer rowid"),
};
@ -427,7 +427,7 @@ impl BTreeCursor {
SeekOp::EQ => &record == *index_key,
};
if found {
let rowid = match record.values.last() {
let rowid = match record.last_value() {
Some(OwnedValue::Integer(rowid)) => *rowid as u64,
_ => unreachable!("index cells should have an integer rowid"),
};
@ -492,18 +492,20 @@ impl BTreeCursor {
let record = crate::storage::sqlite3_ondisk::read_record(payload)?;
let found = match op {
SeekOp::GT => {
&record.values[..record.values.len() - 1] > &index_key.values
record.get_values()[..record.len() - 1] > index_key.get_values()[..]
}
SeekOp::GE => {
&record.values[..record.values.len() - 1] >= &index_key.values
record.get_values()[..record.len() - 1]
>= index_key.get_values()[..]
}
SeekOp::EQ => {
record.values[..record.values.len() - 1] == index_key.values
record.get_values()[..record.len() - 1]
== index_key.get_values()[..]
}
};
self.stack.advance();
if found {
let rowid = match record.values.last() {
let rowid = match record.last_value() {
Some(OwnedValue::Integer(rowid)) => *rowid as u64,
_ => unreachable!("index cells should have an integer rowid"),
};

18
core/types.rs
View file

@ -522,7 +522,7 @@ impl<'a> FromValue<'a> for &'a str {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct Record {
pub values: Vec<OwnedValue>,
values: Vec<OwnedValue>,
}
impl Record {
@ -534,6 +534,22 @@ impl Record {
pub fn count(&self) -> usize {
self.values.len()
}
pub fn last_value(&self) -> Option<&OwnedValue> {
self.values.last()
}
pub fn get_values(&self) -> &Vec<OwnedValue> {
&self.values
}
pub fn get_value(&self, idx: usize) -> &OwnedValue {
&self.values[idx]
}
pub fn len(&self) -> usize {
self.values.len()
}
}
const I8_LOW: i64 = -128;

6
core/vdbe/explain.rs
View file

@ -798,7 +798,7 @@ pub fn insn_to_str(
} => {
let _p4 = String::new();
let to_print: Vec<String> = order
.values
.get_values()
.iter()
.map(|v| match v {
OwnedValue::Integer(i) => {
@ -816,9 +816,7 @@ pub fn insn_to_str(
*cursor_id as i32,
*columns as i32,
0,
OwnedValue::build_text(
&(format!("k({},{})", order.values.len(), to_print.join(","))),
),
OwnedValue::build_text(&(format!("k({},{})", order.len(), to_print.join(",")))),
0,
format!("cursor={}", cursor_id),
)

16
core/vdbe/mod.rs
View file

@ -1020,7 +1020,7 @@ impl Program {
state.registers[*dest] = if cursor.get_null_flag() {
OwnedValue::Null
} else {
record.values[*column].clone()
record.get_value(*column).clone()
};
} else {
state.registers[*dest] = OwnedValue::Null;
@ -1029,7 +1029,7 @@ impl Program {
CursorType::Sorter => {
let cursor = get_cursor_as_sorter_mut(&mut cursors, *cursor_id);
if let Some(record) = cursor.record() {
state.registers[*dest] = record.values[*column].clone();
state.registers[*dest] = record.get_value(*column).clone();
} else {
state.registers[*dest] = OwnedValue::Null;
}
@ -1037,7 +1037,7 @@ impl Program {
CursorType::Pseudo(_) => {
let cursor = get_cursor_as_pseudo_mut(&mut cursors, *cursor_id);
if let Some(record) = cursor.record() {
state.registers[*dest] = record.values[*column].clone();
state.registers[*dest] = record.get_value(*column).clone();
} else {
state.registers[*dest] = OwnedValue::Null;
}
@ -1403,8 +1403,8 @@ impl Program {
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.values[..idx_record.values.len() - 1]
>= *record_from_regs.values
if idx_record.get_values()[..idx_record.len() - 1]
>= record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
} else {
@ -1427,8 +1427,8 @@ impl Program {
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.values[..idx_record.values.len() - 1]
> *record_from_regs.values
if idx_record.get_values()[..idx_record.len() - 1]
> record_from_regs.get_values()[..]
{
state.pc = target_pc.to_offset_int();
} else {
@ -1742,7 +1742,7 @@ impl Program {
order,
} => {
let order = order
.values
.get_values()
.iter()
.map(|v| match v {
OwnedValue::Integer(i) => *i == 0,

6
core/vdbe/sorter.rs
View file

@ -27,8 +27,8 @@ impl Sorter {
pub fn sort(&mut self) {
self.records.sort_by(|a, b| {
let cmp_by_idx = |idx: usize, ascending: bool| {
let a = &a.values[idx];
let b = &b.values[idx];
let a = &a.get_value(idx);
let b = &b.get_value(idx);
if ascending {
a.cmp(b)
} else {
@ -56,6 +56,6 @@ impl Sorter {
}
pub fn insert(&mut self, record: &Record) {
self.records.push(Record::new(record.values.to_vec()));
self.records.push(Record::new(record.get_values().to_vec()));
}
}

2
simulator/generation/plan.rs
View file

@ -456,7 +456,7 @@ impl Interaction {
StepResult::Row => {
let row = rows.row().unwrap();
let mut r = Vec::new();
for el in &row.values {
for el in row.get_values() {
let v = el.to_value();
let v = match v {
limbo_core::Value::Null => Value::Null,

4
sqlite3/src/lib.rs
View file

@ -442,7 +442,7 @@ pub unsafe extern "C" fn sqlite3_expanded_sql(_stmt: *mut sqlite3_stmt) -> *mut
pub unsafe extern "C" fn sqlite3_data_count(stmt: *mut sqlite3_stmt) -> ffi::c_int {
let stmt = &*stmt;
let row = stmt.stmt.row().unwrap();
row.values.len() as ffi::c_int
row.len() as ffi::c_int
}
#[no_mangle]
@ -635,7 +635,7 @@ pub unsafe extern "C" fn sqlite3_column_text(
Some(row) => row,
None => return std::ptr::null(),
};
match row.values.get(idx as usize).map(|v| v.to_value()) {
match row.get_values().get(idx as usize).map(|v| v.to_value()) {
Some(limbo_core::Value::Text(text)) => text.as_bytes().as_ptr(),
_ => std::ptr::null(),
}

6
tests/integration/functions/test_function_rowid.rs
View file

@ -30,7 +30,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
if let Value::Integer(id) = row.values[0].to_value() {
if let Value::Integer(id) = row.get_value(0).to_value() {
assert_eq!(id, 1, "First insert should have rowid 1");
}
}
@ -66,7 +66,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
if let Value::Integer(id) = row.values[0].to_value() {
if let Value::Integer(id) = row.get_value(0).to_value() {
last_id = id;
}
}
@ -112,7 +112,7 @@ fn test_integer_primary_key() -> anyhow::Result<()> {
match select_query.step()? {
StepResult::Row => {
let row = select_query.row().unwrap();
if let Value::Integer(id) = row.values[0].to_value() {
if let Value::Integer(id) = row.get_value(0).to_value() {
rowids.push(id);
}
}

2
tests/integration/fuzz/mod.rs
View file

@ -56,7 +56,7 @@ mod tests {
r => panic!("unexpected result {:?}: expecting single row", r),
}
};
row.values
row.get_values()
.iter()
.map(|x| match x.to_value() {
limbo_core::Value::Null => rusqlite::types::Value::Null,

14
tests/integration/query_processing/test_read_path.rs
View file

@ -15,7 +15,7 @@ fn test_statement_reset_bind() -> anyhow::Result<()> {
match stmt.step()? {
StepResult::Row => {
let row = stmt.row().unwrap();
assert_eq!(row.values[0].to_value(), Value::Integer(1));
assert_eq!(row.get_value(0).to_value(), Value::Integer(1));
}
StepResult::IO => tmp_db.io.run_once()?,
_ => break,
@ -30,7 +30,7 @@ fn test_statement_reset_bind() -> anyhow::Result<()> {
match stmt.step()? {
StepResult::Row => {
let row = stmt.row().unwrap();
assert_eq!(row.values[0].to_value(), Value::Integer(2));
assert_eq!(row.get_value(0).to_value(), Value::Integer(2));
}
StepResult::IO => tmp_db.io.run_once()?,
_ => break,
@ -63,23 +63,23 @@ fn test_statement_bind() -> anyhow::Result<()> {
match stmt.step()? {
StepResult::Row => {
let row = stmt.row().unwrap();
if let Value::Text(s) = row.values[0].to_value() {
if let Value::Text(s) = row.get_value(0).to_value() {
assert_eq!(s, "hello")
}
if let Value::Text(s) = row.values[1].to_value() {
if let Value::Text(s) = row.get_value(1).to_value() {
assert_eq!(s, "hello")
}
if let Value::Integer(i) = row.values[2].to_value() {
if let Value::Integer(i) = row.get_value(2).to_value() {
assert_eq!(i, 42)
}
if let Value::Blob(v) = row.values[3].to_value() {
if let Value::Blob(v) = row.get_value(3).to_value() {
assert_eq!(v, &vec![0x1 as u8, 0x2, 0x3])
}
if let Value::Float(f) = row.values[4].to_value() {
if let Value::Float(f) = row.get_value(4).to_value() {
assert_eq!(f, 0.5)
}
}

20
tests/integration/query_processing/test_write_path.rs
View file

@ -43,8 +43,8 @@ fn test_simple_overflow_page() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values[0].to_value();
let text = row.values[1].to_value();
let first_value = row.get_value(0).to_value();
let text = row.get_value(1).to_value();
let id = match first_value {
Value::Integer(i) => i as i32,
Value::Float(f) => f as i32,
@ -118,8 +118,8 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values[0].to_value();
let text = row.values[1].to_value();
let first_value = row.get_value(0).to_value();
let text = row.get_value(1).to_value();
let id = match first_value {
Value::Integer(i) => i as i32,
Value::Float(f) => f as i32,
@ -190,7 +190,7 @@ fn test_sequential_write() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values.first().expect("missing id");
let first_value = row.get_values().first().expect("missing id");
let id = match first_value.to_value() {
Value::Integer(i) => i as i32,
Value::Float(f) => f as i32,
@ -256,7 +256,7 @@ fn test_regression_multi_row_insert() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values.first().expect("missing id");
let first_value = row.get_values().first().expect("missing id");
let id = match first_value.to_value() {
Value::Float(f) => f as i32,
_ => panic!("expected float"),
@ -302,7 +302,7 @@ fn test_statement_reset() -> anyhow::Result<()> {
match stmt.step()? {
StepResult::Row => {
let row = stmt.row().unwrap();
assert_eq!(row.values[0].to_value(), Value::Integer(1));
assert_eq!(row.get_value(0).to_value(), Value::Integer(1));
break;
}
StepResult::IO => tmp_db.io.run_once()?,
@ -316,7 +316,7 @@ fn test_statement_reset() -> anyhow::Result<()> {
match stmt.step()? {
StepResult::Row => {
let row = stmt.row().unwrap();
assert_eq!(row.values[0].to_value(), Value::Integer(1));
assert_eq!(row.get_value(0).to_value(), Value::Integer(1));
break;
}
StepResult::IO => tmp_db.io.run_once()?,
@ -366,7 +366,7 @@ fn test_wal_checkpoint() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values[0].to_value();
let first_value = row.get_value(0).to_value();
let id = match first_value {
Value::Integer(i) => i as i32,
Value::Float(f) => f as i32,
@ -430,7 +430,7 @@ fn test_wal_restart() -> anyhow::Result<()> {
match rows.step()? {
StepResult::Row => {
let row = rows.row().unwrap();
let first_value = row.values[0].to_value();
let first_value = row.get_value(0).to_value();
let count = match first_value {
Value::Integer(i) => i,
_ => unreachable!(),

4
tests/integration/wal/test_wal.rs
View file

@ -44,7 +44,7 @@ pub(crate) fn execute_and_get_strings(
match step_result {
StepResult::Row => {
let row = stmt.row().unwrap();
for el in &row.values {
for el in row.get_values() {
result.push(format!("{el}"));
}
}
@ -72,7 +72,7 @@ pub(crate) fn execute_and_get_ints(
match step_result {
StepResult::Row => {
let row = stmt.row().unwrap();
for value in &row.values {
for value in row.get_values() {
let value = value.to_value();
let out = match value {
Value::Integer(i) => i,