From 7e76970035c92c377c46e684c1adbe5977b5c727 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Tue, 12 Aug 2025 20:34:57 -0500 Subject: [PATCH] fix: Handle fresh INSERTs in materialized view incremental maintenance The op_insert function was incorrectly trying to capture an "old record" for fresh INSERT operations when a table had dependent materialized views. This caused a "Cannot delete: no current row" error because the cursor wasn't positioned on any row for new inserts. The issue was introduced in commit f38333b3 which refactored the state machine for incremental view handling but didn't properly distinguish between: - Fresh INSERT operations (no old record exists) - UPDATE operations without rowid change (old record should be captured) - UPDATE operations with rowid change (already handled by DELETE) This fix checks if cursor.rowid() returns a value before attempting to capture the old record. If no row exists (fresh INSERT), we correctly set old_record to None instead of erroring out. I am also including tests to make sure this doesn't break. The reason I didn't include tests earlier is that I didn't know it was possible to run the tests under a flag. But in here, I am just adding the flag to the execution script. --- Makefile | 6 +- core/vdbe/execute.rs | 38 ++-- scripts/limbo-sqlite3 | 2 +- testing/materialized_views.test | 370 ++++++++++++++++++++++++++++++++ 4 files changed, 396 insertions(+), 20 deletions(-) create mode 100755 testing/materialized_views.test diff --git a/Makefile b/Makefile index fa8ffb274..1fa102991 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,7 @@ uv-sync-test: uv sync --all-extras --dev --package turso_test .PHONE: uv-sync -test: limbo uv-sync-test test-compat test-vector test-sqlite3 test-shell test-memory test-write test-update test-constraint test-collate test-extensions test-mvcc +test: limbo uv-sync-test test-compat test-vector test-sqlite3 test-shell test-memory test-write test-update test-constraint test-collate test-extensions test-mvcc test-matviews .PHONY: test test-extensions: limbo uv-sync-test @@ -78,6 +78,10 @@ test-time: RUST_LOG=$(RUST_LOG) SQLITE_EXEC=$(SQLITE_EXEC) ./testing/time.test .PHONY: test-time +test-matviews: + RUST_LOG=$(RUST_LOG) SQLITE_EXEC=$(SQLITE_EXEC) ./testing/materialized_views.test +.PHONY: test-matviews + reset-db: ./scripts/clone_test_db.sh .PHONY: reset-db diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index e68246770..939250087 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -5112,30 +5112,32 @@ pub fn op_insert( let old_record = { let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_btree_mut(); - // Get the current key + // Get the current key - for INSERT operations, there may not be a current row let maybe_key = return_if_io!(cursor.rowid()); - let key = maybe_key.ok_or_else(|| { - LimboError::InternalError("Cannot delete: no current row".to_string()) - })?; - // Get the current record before deletion and extract values - let maybe_record = return_if_io!(cursor.record()); - if let Some(record) = maybe_record { - let mut values = record - .get_values() - .into_iter() - .map(|v| v.to_owned()) - .collect::>(); + if let Some(key) = maybe_key { + // Get the current record before deletion and extract values + let maybe_record = return_if_io!(cursor.record()); + if let Some(record) = maybe_record { + let mut values = record + .get_values() + .into_iter() + .map(|v| v.to_owned()) + .collect::>(); - // Fix rowid alias columns: replace Null with actual rowid value - if let Some(table) = schema.get_table(table_name) { - for (i, col) in table.columns().iter().enumerate() { - if col.is_rowid_alias && i < values.len() { - values[i] = Value::Integer(key); + // Fix rowid alias columns: replace Null with actual rowid value + if let Some(table) = schema.get_table(table_name) { + for (i, col) in table.columns().iter().enumerate() { + if col.is_rowid_alias && i < values.len() { + values[i] = Value::Integer(key); + } } } + Some((key, values)) + } else { + None } - Some((key, values)) } else { + // No current row - this is a fresh INSERT, not an UPDATE None } }; diff --git a/scripts/limbo-sqlite3 b/scripts/limbo-sqlite3 index ce742f795..9054eca05 100755 --- a/scripts/limbo-sqlite3 +++ b/scripts/limbo-sqlite3 @@ -7,7 +7,7 @@ PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" TURSODB="$PROJECT_ROOT/target/debug/tursodb" # Add experimental features for testing -EXPERIMENTAL_FLAGS="" +EXPERIMENTAL_FLAGS="--experimental-views" # if RUST_LOG is non-empty, enable tracing output if [ -n "$RUST_LOG" ]; then diff --git a/testing/materialized_views.test b/testing/materialized_views.test new file mode 100755 index 000000000..36cf63e52 --- /dev/null +++ b/testing/materialized_views.test @@ -0,0 +1,370 @@ +#!/usr/bin/env tclsh + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +do_execsql_test_on_specific_db {:memory:} matview-basic-filter-population { + CREATE TABLE products(id INTEGER, name TEXT, price INTEGER, category TEXT); + INSERT INTO products VALUES + (1, 'Laptop', 1200, 'Electronics'), + (2, 'Mouse', 25, 'Electronics'), + (3, 'Desk', 350, 'Furniture'), + (4, 'Chair', 150, 'Furniture'), + (5, 'Monitor', 400, 'Electronics'), + (6, 'Keyboard', 75, 'Electronics'); + + CREATE MATERIALIZED VIEW expensive_items AS + SELECT * FROM products WHERE price > 200; + + SELECT id, name, price FROM expensive_items ORDER BY id; +} {1|Laptop|1200 +3|Desk|350 +5|Monitor|400} + +do_execsql_test_on_specific_db {:memory:} matview-aggregation-population { + CREATE TABLE sales(product_id INTEGER, quantity INTEGER, day INTEGER); + INSERT INTO sales VALUES + (1, 2, 1), + (2, 5, 1), + (1, 1, 2), + (3, 1, 2), + (2, 3, 3), + (1, 1, 3); + + CREATE MATERIALIZED VIEW daily_totals AS + SELECT day, SUM(quantity) as total, COUNT(*) as transactions + FROM sales + GROUP BY day; + + SELECT * FROM daily_totals ORDER BY day; +} {1|7|2 +2|2|2 +3|4|2} + +do_execsql_test_on_specific_db {:memory:} matview-filter-with-groupby { + CREATE TABLE t(a INTEGER, b INTEGER); + INSERT INTO t(a,b) VALUES (2,2), (3,3), (6,6), (7,7); + + CREATE MATERIALIZED VIEW v AS + SELECT b as yourb, SUM(a) as mysum, COUNT(a) as mycount + FROM t + WHERE b > 2 + GROUP BY b; + + SELECT * FROM v ORDER BY yourb; +} {3|3|1 +6|6|1 +7|7|1} + +do_execsql_test_on_specific_db {:memory:} matview-insert-maintenance { + CREATE TABLE t(a INTEGER, b INTEGER); + CREATE MATERIALIZED VIEW v AS + SELECT b, SUM(a) as total, COUNT(*) as cnt + FROM t + WHERE b > 2 + GROUP BY b; + + INSERT INTO t VALUES (3,3), (6,6); + SELECT * FROM v ORDER BY b; + + INSERT INTO t VALUES (4,3), (5,6); + SELECT * FROM v ORDER BY b; + + INSERT INTO t VALUES (1,1), (2,2); + SELECT * FROM v ORDER BY b; +} {3|3|1 +6|6|1 +3|7|2 +6|11|2 +3|7|2 +6|11|2} + +do_execsql_test_on_specific_db {:memory:} matview-delete-maintenance { + CREATE TABLE items(id INTEGER, category TEXT, amount INTEGER); + INSERT INTO items VALUES + (1, 'A', 10), + (2, 'B', 20), + (3, 'A', 30), + (4, 'B', 40), + (5, 'A', 50); + + CREATE MATERIALIZED VIEW category_sums AS + SELECT category, SUM(amount) as total, COUNT(*) as cnt + FROM items + GROUP BY category; + + SELECT * FROM category_sums ORDER BY category; + + DELETE FROM items WHERE id = 3; + SELECT * FROM category_sums ORDER BY category; + + DELETE FROM items WHERE category = 'B'; + SELECT * FROM category_sums ORDER BY category; +} {A|90|3 +B|60|2 +A|60|2 +B|60|2 +A|60|2} + +do_execsql_test_on_specific_db {:memory:} matview-update-maintenance { + CREATE TABLE records(id INTEGER, value INTEGER, status INTEGER); + INSERT INTO records VALUES + (1, 100, 1), + (2, 200, 2), + (3, 300, 1), + (4, 400, 2); + + CREATE MATERIALIZED VIEW status_totals AS + SELECT status, SUM(value) as total, COUNT(*) as cnt + FROM records + GROUP BY status; + + SELECT * FROM status_totals ORDER BY status; + + UPDATE records SET value = 150 WHERE id = 1; + SELECT * FROM status_totals ORDER BY status; + + UPDATE records SET status = 2 WHERE id = 3; + SELECT * FROM status_totals ORDER BY status; +} {1|400|2 +2|600|2 +1|450|2 +2|600|2 +1|150|1 +2|900|3} + +do_execsql_test_on_specific_db {:memory:} matview-integer-primary-key-basic { + CREATE TABLE t(a INTEGER PRIMARY KEY, b INTEGER); + INSERT INTO t(a,b) VALUES (2,2), (3,3), (6,6), (7,7); + + CREATE MATERIALIZED VIEW v AS + SELECT * FROM t WHERE b > 2; + + SELECT * FROM v ORDER BY a; +} {3|3 +6|6 +7|7} + +do_execsql_test_on_specific_db {:memory:} matview-integer-primary-key-update-rowid { + CREATE TABLE t(a INTEGER PRIMARY KEY, b INTEGER); + INSERT INTO t(a,b) VALUES (2,2), (3,3), (6,6), (7,7); + + CREATE MATERIALIZED VIEW v AS + SELECT * FROM t WHERE b > 2; + + SELECT * FROM v ORDER BY a; + + UPDATE t SET a = 1 WHERE b = 3; + SELECT * FROM v ORDER BY a; + + UPDATE t SET a = 10 WHERE a = 6; + SELECT * FROM v ORDER BY a; +} {3|3 +6|6 +7|7 +1|3 +6|6 +7|7 +1|3 +7|7 +10|6} + +do_execsql_test_on_specific_db {:memory:} matview-integer-primary-key-update-value { + CREATE TABLE t(a INTEGER PRIMARY KEY, b INTEGER); + INSERT INTO t(a,b) VALUES (2,2), (3,3), (6,6), (7,7); + + CREATE MATERIALIZED VIEW v AS + SELECT * FROM t WHERE b > 2; + + SELECT * FROM v ORDER BY a; + + UPDATE t SET b = 1 WHERE a = 6; + SELECT * FROM v ORDER BY a; + + UPDATE t SET b = 5 WHERE a = 2; + SELECT * FROM v ORDER BY a; +} {3|3 +6|6 +7|7 +3|3 +7|7 +2|5 +3|3 +7|7} + +do_execsql_test_on_specific_db {:memory:} matview-integer-primary-key-with-aggregation { + CREATE TABLE t(a INTEGER PRIMARY KEY, b INTEGER, c INTEGER); + INSERT INTO t VALUES + (1, 10, 100), + (2, 10, 200), + (3, 20, 300), + (4, 20, 400), + (5, 10, 500); + + CREATE MATERIALIZED VIEW v AS + SELECT b, SUM(c) as total, COUNT(*) as cnt + FROM t + WHERE a > 2 + GROUP BY b; + + SELECT * FROM v ORDER BY b; + + UPDATE t SET a = 6 WHERE a = 1; + SELECT * FROM v ORDER BY b; + + DELETE FROM t WHERE a = 3; + SELECT * FROM v ORDER BY b; +} {10|500|1 +20|700|2 +10|600|2 +20|700|2 +10|600|2 +20|400|1} + +do_execsql_test_on_specific_db {:memory:} matview-complex-filter-aggregation { + CREATE TABLE transactions( + id INTEGER, + account INTEGER, + amount INTEGER, + type INTEGER + ); + + INSERT INTO transactions VALUES + (1, 100, 50, 1), + (2, 100, 30, 2), + (3, 200, 100, 1), + (4, 100, 20, 1), + (5, 200, 40, 2), + (6, 300, 60, 1); + + CREATE MATERIALIZED VIEW account_deposits AS + SELECT account, SUM(amount) as total_deposits, COUNT(*) as deposit_count + FROM transactions + WHERE type = 1 + GROUP BY account; + + SELECT * FROM account_deposits ORDER BY account; + + INSERT INTO transactions VALUES (7, 100, 25, 1); + SELECT * FROM account_deposits ORDER BY account; + + UPDATE transactions SET amount = 80 WHERE id = 1; + SELECT * FROM account_deposits ORDER BY account; + + DELETE FROM transactions WHERE id = 3; + SELECT * FROM account_deposits ORDER BY account; +} {100|70|2 +200|100|1 +300|60|1 +100|95|3 +200|100|1 +300|60|1 +100|125|3 +200|100|1 +300|60|1 +100|125|3 +300|60|1} + +do_execsql_test_on_specific_db {:memory:} matview-sum-count-only { + CREATE TABLE data(id INTEGER, value INTEGER, category INTEGER); + INSERT INTO data VALUES + (1, 10, 1), + (2, 20, 1), + (3, 30, 2), + (4, 40, 2), + (5, 50, 1); + + CREATE MATERIALIZED VIEW category_stats AS + SELECT category, + SUM(value) as sum_val, + COUNT(*) as cnt + FROM data + GROUP BY category; + + SELECT * FROM category_stats ORDER BY category; + + INSERT INTO data VALUES (6, 5, 1); + SELECT * FROM category_stats ORDER BY category; + + UPDATE data SET value = 35 WHERE id = 3; + SELECT * FROM category_stats ORDER BY category; +} {1|80|3 +2|70|2 +1|85|4 +2|70|2 +1|85|4 +2|75|2} + +do_execsql_test_on_specific_db {:memory:} matview-empty-table-population { + CREATE TABLE t(a INTEGER, b INTEGER); + CREATE MATERIALIZED VIEW v AS + SELECT b, SUM(a) as total, COUNT(*) as cnt + FROM t + WHERE b > 5 + GROUP BY b; + + SELECT COUNT(*) FROM v; + + INSERT INTO t VALUES (1, 3), (2, 7), (3, 9); + SELECT * FROM v ORDER BY b; +} {0 +7|2|1 +9|3|1} + +do_execsql_test_on_specific_db {:memory:} matview-all-rows-filtered { + CREATE TABLE t(a INTEGER, b INTEGER); + INSERT INTO t VALUES (1, 1), (2, 2), (3, 3); + + CREATE MATERIALIZED VIEW v AS + SELECT * FROM t WHERE b > 10; + + SELECT COUNT(*) FROM v; + + INSERT INTO t VALUES (11, 11); + SELECT * FROM v; + + UPDATE t SET b = 1 WHERE a = 11; + SELECT COUNT(*) FROM v; +} {0 +11|11 +0} + +do_execsql_test_on_specific_db {:memory:} matview-mixed-operations-sequence { + CREATE TABLE orders( + order_id INTEGER PRIMARY KEY, + customer_id INTEGER, + amount INTEGER + ); + + INSERT INTO orders VALUES (1, 100, 50); + INSERT INTO orders VALUES (2, 200, 75); + + CREATE MATERIALIZED VIEW customer_totals AS + SELECT customer_id, SUM(amount) as total, COUNT(*) as order_count + FROM orders + GROUP BY customer_id; + + SELECT * FROM customer_totals ORDER BY customer_id; + + INSERT INTO orders VALUES (3, 100, 25); + SELECT * FROM customer_totals ORDER BY customer_id; + + UPDATE orders SET amount = 100 WHERE order_id = 2; + SELECT * FROM customer_totals ORDER BY customer_id; + + DELETE FROM orders WHERE order_id = 1; + SELECT * FROM customer_totals ORDER BY customer_id; + + INSERT INTO orders VALUES (4, 300, 150); + SELECT * FROM customer_totals ORDER BY customer_id; +} {100|50|1 +200|75|1 +100|75|2 +200|75|1 +100|75|2 +200|100|1 +100|25|1 +200|100|1 +100|25|1 +200|100|1 +300|150|1} \ No newline at end of file