From 5740becd084e0b34940362166bd74853962afc4b Mon Sep 17 00:00:00 2001 From: Saul Shanabrook Date: Tue, 26 May 2026 16:04:51 -0700 Subject: [PATCH] Fix multi-column column index rebuild ordering --- core-relations/src/hash_index/mod.rs | 5 +++ core-relations/src/hash_index/tests.rs | 44 +++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/core-relations/src/hash_index/mod.rs b/core-relations/src/hash_index/mod.rs index dfa7b93a2..edaf44fd1 100644 --- a/core-relations/src/hash_index/mod.rs +++ b/core-relations/src/hash_index/mod.rs @@ -350,6 +350,11 @@ impl IndexBase for ColumnIndex { radix_sort_pairs_by_value(&mut pairs); if cols.len() > 1 { + // Multi-column collection is column-major, so a stable value sort + // can leave each value's rows out of order across columns. + for group in pairs.chunk_by_mut(|a, b| a.0 == b.0) { + group.sort_unstable_by_key(|&(_, row)| row); + } // Remove duplicates (same value in multiple columns of the same row). pairs.dedup(); } diff --git a/core-relations/src/hash_index/tests.rs b/core-relations/src/hash_index/tests.rs index 91cbfb833..07006c719 100644 --- a/core-relations/src/hash_index/tests.rs +++ b/core-relations/src/hash_index/tests.rs @@ -1,7 +1,8 @@ -use crate::numeric_id::NumericId; +use crate::{numeric_id::NumericId, offsets::Offsets}; use crate::{ TupleIndex, + hash_index::ColumnIndex, table_shortcuts::{fill_table, v}, table_spec::{ColumnId, WrappedTable}, }; @@ -89,3 +90,44 @@ fn basic_updates() { } } } + +#[test] +fn multi_column_column_index_rebuild_orders_each_value_by_row() { + let rows = (0..128).map(|i| { + let left = if i >= 96 { v(7) } else { v(1_000 + i) }; + let right = if i < 32 { v(7) } else { v(2_000 + i) }; + vec![v(i), left, right] + }); + let mut table = WrappedTable::new(fill_table(rows, 1, None, |old, new| { + assert_eq!(old, new, "no conflicts in this test"); + None + })); + let mut index = Index::new(vec![ColumnId::new(1), ColumnId::new(2)], ColumnIndex::new()); + index.refresh(table.as_ref()); + + empty_execution_state!(es); + let start_version = table.version().major; + while table.version().major == start_version { + table.new_buffer().stage_remove(&[v(0)]); + table.merge(&mut es); + table.new_buffer().stage_insert(&[v(0), v(1000), v(7)]); + table.merge(&mut es); + } + + index.refresh(table.as_ref()); + let key = v(7); + let subset = index.get_subset(&key).unwrap(); + let mut row_ids = Vec::new(); + subset.offsets(|row_id| row_ids.push(row_id.index())); + + let mut expected = Vec::new(); + table + .scan(table.all().as_ref()) + .iter() + .for_each(|(row_id, row)| { + if row[1] == key || row[2] == key { + expected.push(row_id.index()); + } + }); + assert_eq!(row_ids, expected); +}