max / audiofiles
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+72 insertions,
-6 deletions
| @@ -1729,6 +1729,30 @@ | |||
| 1729 | 1729 | CREATE INDEX IF NOT EXISTS idx_analysis_attack_time ON audio_analysis(attack_time); | |
| 1730 | 1730 | "; | |
| 1731 | 1731 | ||
| 1732 | + | const MIGRATION_037: &str = r" | |
| 1733 | + | -- Cover the browse-list sort order, which is the worst-case list load: opening | |
| 1734 | + | -- the browser with no filter applied. | |
| 1735 | + | -- | |
| 1736 | + | -- `search_global` and `search_dir` both end `ORDER BY n.node_type, n.name | |
| 1737 | + | -- LIMIT 500`, and nothing indexed that pair. SQLite therefore scanned every | |
| 1738 | + | -- vfs_nodes row, built a temp B-tree over all of them, sorted, and discarded | |
| 1739 | + | -- all but 500. SEARCH_RESULT_LIMIT bounds what comes back, never the work | |
| 1740 | + | -- underneath it, so the cost grew with the library while the result set did not. | |
| 1741 | + | -- | |
| 1742 | + | -- Measured on a 40,200-node database, unfiltered `search_global`: | |
| 1743 | + | -- before 63.04 ms SCAN n + USE TEMP B-TREE FOR ORDER BY | |
| 1744 | + | -- after 0.81 ms SCAN n USING INDEX idx_vfs_nodes_sort, no temp B-tree | |
| 1745 | + | -- The sort disappears from the plan: SQLite walks the index in order and stops | |
| 1746 | + | -- once the LIMIT is met. | |
| 1747 | + | -- | |
| 1748 | + | -- The index costs about 4.6 MB at 40k nodes, so roughly 34 MB at the 289k-node | |
| 1749 | + | -- library the extrapolation was aimed at. That is the trade, and it is the right | |
| 1750 | + | -- way round: the list load is on the path a user waits for, and disk is not. | |
| 1751 | + | -- | |
| 1752 | + | -- Additive and idempotent. | |
| 1753 | + | CREATE INDEX IF NOT EXISTS idx_vfs_nodes_sort ON vfs_nodes(node_type, name); | |
| 1754 | + | "; | |
| 1755 | + | ||
| 1732 | 1756 | /// Register `hash_row_id(salt, key) -> TEXT` as a deterministic SQLite | |
| 1733 | 1757 | /// function on the given connection. Used by the M018 sync triggers so the | |
| 1734 | 1758 | /// `sync_changelog.row_id` field never carries cleartext content (tag strings, | |
| @@ -1919,6 +1943,7 @@ | |||
| 1919 | 1943 | MIGRATION_034, | |
| 1920 | 1944 | MIGRATION_035, | |
| 1921 | 1945 | MIGRATION_036, | |
| 1946 | + | MIGRATION_037, | |
| 1922 | 1947 | ]; | |
| 1923 | 1948 | ||
| 1924 | 1949 | for (i, sql) in MIGRATIONS.iter().enumerate() { | |
| @@ -2345,7 +2370,7 @@ | |||
| 2345 | 2370 | .conn() | |
| 2346 | 2371 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2347 | 2372 | .unwrap(); | |
| 2348 | - | assert_eq!(version, 36); | |
| 2373 | + | assert_eq!(version, 37); | |
| 2349 | 2374 | } | |
| 2350 | 2375 | ||
| 2351 | 2376 | #[test] | |
| @@ -2356,7 +2381,7 @@ | |||
| 2356 | 2381 | .conn() | |
| 2357 | 2382 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2358 | 2383 | .unwrap(); | |
| 2359 | - | assert_eq!(version, 36); | |
| 2384 | + | assert_eq!(version, 37); | |
| 2360 | 2385 | } | |
| 2361 | 2386 | ||
| 2362 | 2387 | #[test] | |
| @@ -2540,7 +2565,7 @@ | |||
| 2540 | 2565 | .conn() | |
| 2541 | 2566 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2542 | 2567 | .unwrap(); | |
| 2543 | - | assert_eq!(version, 36); | |
| 2568 | + | assert_eq!(version, 37); | |
| 2544 | 2569 | } | |
| 2545 | 2570 | ||
| 2546 | 2571 | /// Simulates the worst-case recovery path: a prior partial migration left | |
| @@ -2584,7 +2609,48 @@ | |||
| 2584 | 2609 | .conn() | |
| 2585 | 2610 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2586 | 2611 | .unwrap(); | |
| 2587 | - | assert_eq!(version, 36); | |
| 2612 | + | assert_eq!(version, 37); | |
| 2613 | + | } | |
| 2614 | + | ||
| 2615 | + | /// M037 contract: the browse-list sort must not build a temp B-tree. | |
| 2616 | + | /// | |
| 2617 | + | /// Asserting the index exists would be the weaker test, because an index | |
| 2618 | + | /// SQLite declines to use buys nothing. What actually regressed here was the | |
| 2619 | + | /// PLAN: `SCAN n` plus `USE TEMP B-TREE FOR ORDER BY` sorted the whole | |
| 2620 | + | /// library to return 500 rows, which cost 63 ms at 40k nodes against 0.81 ms | |
| 2621 | + | /// once the sort could be walked from the index. So this pins the plan. | |
| 2622 | + | /// | |
| 2623 | + | /// It breaks if someone changes the ORDER BY in `search_global` without | |
| 2624 | + | /// moving the index with it, which is the failure that would silently | |
| 2625 | + | /// restore the full sort. | |
| 2626 | + | #[test] | |
| 2627 | + | fn m037_browse_sort_uses_the_index_and_not_a_temp_btree() { | |
| 2628 | + | let db = Database::open_in_memory().unwrap(); | |
| 2629 | + | let plan: Vec<String> = db | |
| 2630 | + | .conn() | |
| 2631 | + | .prepare( | |
| 2632 | + | "EXPLAIN QUERY PLAN | |
| 2633 | + | SELECT n.id, n.name FROM vfs_nodes n | |
| 2634 | + | LEFT JOIN audio_analysis a ON n.sample_hash = a.hash | |
| 2635 | + | LEFT JOIN samples s ON n.sample_hash = s.hash | |
| 2636 | + | WHERE s.deleted_at IS NULL | |
| 2637 | + | ORDER BY n.node_type ASC, n.name ASC LIMIT 500", | |
| 2638 | + | ) | |
| 2639 | + | .unwrap() | |
| 2640 | + | .query_map([], |row| row.get::<_, String>(3)) | |
| 2641 | + | .unwrap() | |
| 2642 | + | .collect::<std::result::Result<Vec<_>, _>>() | |
| 2643 | + | .unwrap(); | |
| 2644 | + | let plan = plan.join("\n"); | |
| 2645 | + | ||
| 2646 | + | assert!( | |
| 2647 | + | !plan.to_uppercase().contains("TEMP B-TREE"), | |
| 2648 | + | "browse sort fell back to a full sort:\n{plan}" | |
| 2649 | + | ); | |
| 2650 | + | assert!( | |
| 2651 | + | plan.contains("idx_vfs_nodes_sort"), | |
| 2652 | + | "browse sort is not walking the sort index:\n{plan}" | |
| 2653 | + | ); | |
| 2588 | 2654 | } | |
| 2589 | 2655 | ||
| 2590 | 2656 | /// M018 contract: the `sync_changelog.row_id` for sensitive tables must | |
| @@ -2814,7 +2880,7 @@ | |||
| 2814 | 2880 | let initial_version: i32 = conn | |
| 2815 | 2881 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2816 | 2882 | .unwrap(); | |
| 2817 | - | assert_eq!(initial_version, 36); | |
| 2883 | + | assert_eq!(initial_version, 37); | |
| 2818 | 2884 | ||
| 2819 | 2885 | let batch = format!("BEGIN;\n{bad_sql}\nPRAGMA user_version = 999;\nCOMMIT;"); | |
| 2820 | 2886 | let first_err = conn.execute_batch(&batch).unwrap_err(); | |
| @@ -2879,7 +2945,7 @@ | |||
| 2879 | 2945 | .conn() | |
| 2880 | 2946 | .query_row("PRAGMA user_version", [], |row| row.get(0)) | |
| 2881 | 2947 | .unwrap(); | |
| 2882 | - | assert_eq!(version, 36); | |
| 2948 | + | assert_eq!(version, 37); | |
| 2883 | 2949 | } | |
| 2884 | 2950 | ||
| 2885 | 2951 | #[test] |