From 4c76191fec91602380718fd40757296ceebcda5a Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 15 Aug 2025 10:48:42 +0300 Subject: [PATCH 1/2] fix/sim: fix incorrect implementation of compound select generation Problem: sim was generating compound selects like this: - pick a random `table` - create a random single SELECT - create `n` random compound SELECTs (UNION ALL etc) assign a WHERE clause that always has a condition based on `table` This can result in e.g. ``` SELECT a FROM foo WHERE bar.x = 'baz' ``` Solution: Don't base the WHERE clause on a table that might not be referenced in the query. Again, the only reason this wasn't caught before was because `FaultyQuery` and `FsyncNoWait` are the only paths where this is actually tested with an assertion, and those are both disabled --- simulator/generation/query.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/simulator/generation/query.rs b/simulator/generation/query.rs index fa0932e46..c0b3ce7cc 100644 --- a/simulator/generation/query.rs +++ b/simulator/generation/query.rs @@ -177,8 +177,6 @@ impl ArbitraryFrom<&SimulatorEnv> for SelectFree { impl ArbitraryFrom<&SimulatorEnv> for Select { fn arbitrary_from(rng: &mut R, env: &SimulatorEnv) -> Self { - let table = pick(&env.tables, rng); - // Generate a number of selects based on the query size // If experimental indexes are enabled, we can have selects with compounds // Otherwise, we just have a single select with no compounds @@ -196,11 +194,7 @@ impl ArbitraryFrom<&SimulatorEnv> for Select { let first = SelectInner::arbitrary_from(rng, env); let rest: Vec = (0..num_compound_selects) - .map(|_| { - let mut select = first.clone(); - select.where_clause = Predicate::arbitrary_from(rng, table); - select - }) + .map(|_| SelectInner::arbitrary_from(rng, env)) .collect(); Self { From 96072509f5bec315da8ca3cba7ef23e861c3ca23 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Fri, 15 Aug 2025 11:07:33 +0300 Subject: [PATCH 2/2] sim: add Property::TableHasExpectedContent --- simulator/generation/property.rs | 81 ++++++++++++++++++++++++++++++++ simulator/shrink/plan.rs | 4 +- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/simulator/generation/property.rs b/simulator/generation/property.rs index a3142f3b0..b06a92e04 100644 --- a/simulator/generation/property.rs +++ b/simulator/generation/property.rs @@ -71,6 +71,17 @@ pub(crate) enum Property { update: Update, select: Select, }, + /// TableHasExpectedContent is a property in which the table + /// must have the expected content, i.e. all the insertions and + /// updates and deletions should have been persisted in the way + /// we think they were. + /// The execution of the property is as follows + /// SELECT * FROM + /// ASSERT + TableHasExpectedContent { + table: String, + expected_content: Vec>, + }, /// Double Create Failure is a property in which creating /// the same table twice leads to an error. /// The execution of the property is as follows @@ -196,6 +207,7 @@ impl Property { match self { Property::InsertValuesSelect { .. } => "Insert-Values-Select", Property::ReadYourUpdatesBack { .. } => "Read-Your-Updates-Back", + Property::TableHasExpectedContent { .. } => "Table-Has-Expected-Content", Property::DoubleCreateFailure { .. } => "Double-Create-Failure", Property::SelectLimit { .. } => "Select-Limit", Property::DeleteSelect { .. } => "Delete-Select", @@ -212,6 +224,59 @@ impl Property { /// and `interaction` cannot be serialized directly. pub(crate) fn interactions(&self) -> Vec { match self { + Property::TableHasExpectedContent { + table, + expected_content, + } => { + let table = table.to_string(); + let table_name = table.clone(); + let expected_content = expected_content.clone(); + let assumption = Interaction::Assumption(Assertion { + name: format!("table {} exists", table.clone()), + func: Box::new(move |_: &Vec, env: &mut SimulatorEnv| { + if env.tables.iter().any(|t| t.name == table_name) { + Ok(Ok(())) + } else { + Ok(Err(format!("table {table_name} does not exist"))) + } + }), + }); + + let select_interaction = Interaction::Query(Query::Select(Select::simple( + table.clone(), + Predicate::true_(), + ))); + + let assertion = Interaction::Assertion(Assertion { + name: format!("table {} should have the expected content", table.clone()), + func: Box::new(move |stack: &Vec, _| { + let rows = stack.last().unwrap(); + let Ok(rows) = rows else { + return Ok(Err(format!("expected rows but got error: {rows:?}"))); + }; + if rows.len() != expected_content.len() { + return Ok(Err(format!( + "expected {} rows but got {} for table {}", + expected_content.len(), + rows.len(), + table.clone() + ))); + } + for expected_row in expected_content.iter() { + if !rows.contains(expected_row) { + return Ok(Err(format!( + "expected row {:?} not found in table {}", + expected_row, + table.clone() + ))); + } + } + Ok(Ok(())) + }), + }); + + vec![assumption, select_interaction, assertion] + } Property::ReadYourUpdatesBack { update, select } => { let table = update.table().to_string(); let assumption = Interaction::Assumption(Assertion { @@ -1119,6 +1184,18 @@ fn property_read_your_updates_back(rng: &mut R, env: &SimulatorEnv Property::ReadYourUpdatesBack { update, select } } + +fn property_table_has_expected_content(rng: &mut R, env: &SimulatorEnv) -> Property { + // Get a random table + let table = pick(&env.tables, rng); + // Generate rows to insert + let rows = table.rows.clone(); + Property::TableHasExpectedContent { + table: table.name.clone(), + expected_content: rows, + } +} + fn property_select_limit(rng: &mut R, env: &SimulatorEnv) -> Property { // Get a random table let table = pick(&env.tables, rng); @@ -1356,6 +1433,10 @@ impl ArbitraryFrom<(&SimulatorEnv, &InteractionStats)> for Property { }, Box::new(|rng: &mut R| property_insert_values_select(rng, env, &remaining_)), ), + ( + remaining_.read, + Box::new(|rng: &mut R| property_table_has_expected_content(rng, env)), + ), ( f64::min(remaining_.read, remaining_.write), Box::new(|rng: &mut R| property_read_your_updates_back(rng, env)), diff --git a/simulator/shrink/plan.rs b/simulator/shrink/plan.rs index 0fe329748..29935aa0d 100644 --- a/simulator/shrink/plan.rs +++ b/simulator/shrink/plan.rs @@ -79,6 +79,7 @@ impl InteractionPlan { | Property::UNIONAllPreservesCardinality { .. } | Property::FsyncNoWait { .. } | Property::ReadYourUpdatesBack { .. } + | Property::TableHasExpectedContent { .. } | Property::FaultyQuery { .. } => {} } } @@ -201,7 +202,8 @@ impl InteractionPlan { | Property::SelectSelectOptimizer { .. } | Property::FaultyQuery { .. } | Property::FsyncNoWait { .. } - | Property::ReadYourUpdatesBack { .. } => {} + | Property::ReadYourUpdatesBack { .. } + | Property::TableHasExpectedContent { .. } => {} } } }